From b951e6312877e3f2796c89f2093d57eb567307b2 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Sat, 11 Apr 2026 23:02:26 -0500 Subject: [PATCH 1/8] docs: add design spec and implementation plan for cmd/up.go refactor --- .../plans/2026-04-11-cmd-up-refactor.md | 1571 +++++++++++++++++ .../2026-04-11-cmd-up-refactor-design.md | 182 ++ 2 files changed, 1753 insertions(+) create mode 100644 docs/superpowers/plans/2026-04-11-cmd-up-refactor.md create mode 100644 docs/superpowers/specs/2026-04-11-cmd-up-refactor-design.md diff --git a/docs/superpowers/plans/2026-04-11-cmd-up-refactor.md b/docs/superpowers/plans/2026-04-11-cmd-up-refactor.md new file mode 100644 index 000000000..cb365a536 --- /dev/null +++ b/docs/superpowers/plans/2026-04-11-cmd-up-refactor.md @@ -0,0 +1,1571 @@ +# cmd/up.go Refactor Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Extract the 1839-line `cmd/up.go` into focused `pkg/` packages with unit tests, leaving ~350-400 lines of command orchestration. + +**Architecture:** Six commits, each extracting one responsibility area into a `pkg/` package (or extending an existing one), adding tests, and updating `cmd/up.go` to call the new package. Pure refactor -- no behavior changes. + +**Tech Stack:** Go, cobra, testify/assert, table-driven tests + +**Spec:** `docs/superpowers/specs/2026-04-11-cmd-up-refactor-design.md` + +--- + +## File Structure + +| Action | Path | Responsibility | +|--------|------|----------------| +| Create | `pkg/dotfiles/dotfiles.go` | Dotfiles setup orchestration | +| Create | `pkg/dotfiles/dotfiles_test.go` | Tests for pure helper functions | +| Create | `pkg/ide/opener/opener.go` | IDE launch dispatch and helpers | +| Create | `pkg/ide/opener/opener_test.go` | Tests for parseAddressAndPort, dispatch routing | +| Create | `pkg/gpg/forward.go` | GPG agent forwarding | +| Create | `pkg/gpg/forward_test.go` | Tests for command construction | +| Modify | `pkg/tunnel/browser.go` (create) | Browser tunnel + backhaul | +| Create | `pkg/tunnel/browser_test.go` | Tests for SSH command construction | +| Modify | `pkg/workspace/provider.go` | Add CheckProviderUpdate, GetProInstance | +| Create | `pkg/workspace/provider_update_test.go` | Tests for version comparison | +| Modify | `cmd/up.go` | Slim down to orchestration only | + +--- + +### Task 1: Extract `pkg/dotfiles/` + +**Files:** +- Create: `pkg/dotfiles/dotfiles.go` +- Create: `pkg/dotfiles/dotfiles_test.go` +- Modify: `cmd/up.go:1384-1553` (remove moved functions) + +- [ ] **Step 1: Write failing tests for pure helper functions** + +Create `pkg/dotfiles/dotfiles_test.go`: + +```go +package dotfiles + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestExtractKeysFromEnvKeyValuePairs(t *testing.T) { + tests := []struct { + name string + input []string + want []string + }{ + { + name: "empty input", + input: []string{}, + want: []string{}, + }, + { + name: "single pair", + input: []string{"FOO=bar"}, + want: []string{"FOO"}, + }, + { + name: "multiple pairs", + input: []string{"FOO=bar", "BAZ=qux"}, + want: []string{"FOO", "BAZ"}, + }, + { + name: "value with equals sign", + input: []string{"FOO=bar=baz"}, + want: []string{"FOO"}, + }, + { + name: "no equals sign skipped", + input: []string{"INVALID"}, + want: []string{}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := extractKeysFromEnvKeyValuePairs(tt.input) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestCollectDotfilesScriptEnvKeyValuePairs(t *testing.T) { + t.Run("empty file list", func(t *testing.T) { + got, err := collectDotfilesScriptEnvKeyValuePairs(nil) + assert.NoError(t, err) + assert.Empty(t, got) + }) + + t.Run("reads key-value pairs from file", func(t *testing.T) { + dir := t.TempDir() + envFile := dir + "/test.env" + err := os.WriteFile(envFile, []byte("FOO=bar\nBAZ=qux\n"), 0o600) + assert.NoError(t, err) + + got, err := collectDotfilesScriptEnvKeyValuePairs([]string{envFile}) + assert.NoError(t, err) + assert.Contains(t, got, "FOO=bar") + assert.Contains(t, got, "BAZ=qux") + }) + + t.Run("nonexistent file returns error", func(t *testing.T) { + _, err := collectDotfilesScriptEnvKeyValuePairs([]string{"/nonexistent/file"}) + assert.Error(t, err) + }) +} + +func TestBuildDotCmdAgentArguments(t *testing.T) { + tests := []struct { + name string + repo string + script string + strict bool + debug bool + wantArgs []string + }{ + { + name: "basic repo only", + repo: "https://github.com/user/dots", + wantArgs: []string{ + "agent", "workspace", "install-dotfiles", + "--repository", "https://github.com/user/dots", + }, + }, + { + name: "with script", + repo: "https://github.com/user/dots", + script: "install.sh", + wantArgs: []string{ + "agent", "workspace", "install-dotfiles", + "--repository", "https://github.com/user/dots", + "--install-script", "install.sh", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := buildDotCmdAgentArguments(tt.repo, tt.script, tt.strict, tt.debug) + assert.Equal(t, tt.wantArgs, got) + }) + } +} +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `cd pkg/dotfiles && go test -v ./...` +Expected: FAIL -- package doesn't exist yet + +- [ ] **Step 3: Create `pkg/dotfiles/dotfiles.go`** + +Move the following functions from `cmd/up.go` lines 1384-1553 into `pkg/dotfiles/dotfiles.go`: +- `setupDotfiles` -> exported as `Setup` +- `buildDotCmd` -> unexported +- `buildDotCmdAgentArguments` -> unexported (refactored to accept booleans instead of config/logger) +- `extractKeysFromEnvKeyValuePairs` -> unexported +- `collectDotfilesScriptEnvKeyvaluePairs` -> unexported (renamed to `collectDotfilesScriptEnvKeyValuePairs`) + +```go +package dotfiles + +import ( + "fmt" + "os" + "os/exec" + "slices" + "strings" + + "github.com/sirupsen/logrus" + "github.com/skevetter/devpod/pkg/agent" + client2 "github.com/skevetter/devpod/pkg/client" + "github.com/skevetter/devpod/pkg/config" + config2 "github.com/skevetter/devpod/pkg/devcontainer/config" + devssh "github.com/skevetter/devpod/pkg/ssh" + "github.com/skevetter/log" +) + +// SetupParams holds all parameters for dotfiles setup. +type SetupParams struct { + Source string + Script string + EnvFiles []string + EnvKeyValues []string + Client client2.BaseWorkspaceClient + DevPodConfig *config.Config + Log log.Logger +} + +// Setup clones and installs dotfiles into the devcontainer. +func Setup(params SetupParams) error { + dotfilesRepo := params.DevPodConfig.ContextOption(config.ContextOptionDotfilesURL) + if params.Source != "" { + dotfilesRepo = params.Source + } + + dotfilesScript := params.DevPodConfig.ContextOption(config.ContextOptionDotfilesScript) + if params.Script != "" { + dotfilesScript = params.Script + } + + if dotfilesRepo == "" { + params.Log.Debug("No dotfiles repo specified, skipping") + return nil + } + + params.Log.Infof("Dotfiles Git repository %s specified", dotfilesRepo) + params.Log.Debug("Cloning dotfiles into the devcontainer...") + + strictHostKey := params.DevPodConfig.ContextOption( + config.ContextOptionSSHStrictHostKeyChecking, + ) == config.BoolTrue + + dotCmd, err := buildDotCmd( + dotfilesRepo, + dotfilesScript, + params.EnvFiles, + params.EnvKeyValues, + params.Client, + strictHostKey, + params.Log, + ) + if err != nil { + return err + } + if params.Log.GetLevel() == logrus.DebugLevel { + dotCmd.Args = append(dotCmd.Args, "--debug") + } + + params.Log.Debugf("Running dotfiles setup command: %v", dotCmd.Args) + + writer := params.Log.Writer(logrus.InfoLevel, false) + + dotCmd.Stdout = writer + dotCmd.Stderr = writer + + err = dotCmd.Run() + if err != nil { + return err + } + + params.Log.Infof("Done setting up dotfiles into the devcontainer") + + return nil +} + +func buildDotCmdAgentArguments( + dotfilesRepo, dotfilesScript string, + strictHostKey, debug bool, +) []string { + agentArguments := []string{ + "agent", + "workspace", + "install-dotfiles", + "--repository", + dotfilesRepo, + } + + if strictHostKey { + agentArguments = append(agentArguments, "--strict-host-key-checking") + } + + if debug { + agentArguments = append(agentArguments, "--debug") + } + + if dotfilesScript != "" { + agentArguments = append(agentArguments, "--install-script", dotfilesScript) + } + + return agentArguments +} + +func buildDotCmd( + dotfilesRepo, dotfilesScript string, + envFiles, envKeyValuePairs []string, + client client2.BaseWorkspaceClient, + strictHostKey bool, + logger log.Logger, +) (*exec.Cmd, error) { + sshCmd := []string{ + "ssh", + "--agent-forwarding=true", + "--start-services=true", + } + + envFilesKeyValuePairs, err := collectDotfilesScriptEnvKeyValuePairs(envFiles) + if err != nil { + return nil, err + } + + allEnvKeyValuesPairs := slices.Concat(envFilesKeyValuePairs, envKeyValuePairs) + allEnvKeys := extractKeysFromEnvKeyValuePairs(allEnvKeyValuesPairs) + for _, envKey := range allEnvKeys { + sshCmd = append(sshCmd, "--send-env", envKey) + } + + remoteUser, err := devssh.GetUser( + client.WorkspaceConfig().ID, + client.WorkspaceConfig().SSHConfigPath, + client.WorkspaceConfig().SSHConfigIncludePath, + ) + if err != nil { + remoteUser = "root" + } + + agentArguments := buildDotCmdAgentArguments( + dotfilesRepo, dotfilesScript, + strictHostKey, + logger.GetLevel() == logrus.DebugLevel, + ) + sshCmd = append(sshCmd, + "--user", + remoteUser, + "--context", + client.Context(), + client.Workspace(), + "--log-output=raw", + "--command", + agent.ContainerDevPodHelperLocation+" "+strings.Join(agentArguments, " "), + ) + execPath, err := os.Executable() + if err != nil { + return nil, err + } + + dotCmd := exec.Command( + execPath, + sshCmd..., + ) + + dotCmd.Env = append(dotCmd.Environ(), allEnvKeyValuesPairs...) + return dotCmd, nil +} + +func extractKeysFromEnvKeyValuePairs(envKeyValuePairs []string) []string { + keys := []string{} + for _, env := range envKeyValuePairs { + keyValue := strings.SplitN(env, "=", 2) + if len(keyValue) == 2 { + keys = append(keys, keyValue[0]) + } + } + return keys +} + +func collectDotfilesScriptEnvKeyValuePairs(envFiles []string) ([]string, error) { + keyValues := []string{} + for _, file := range envFiles { + envFromFile, err := config2.ParseKeyValueFile(file) + if err != nil { + return nil, err + } + keyValues = append(keyValues, envFromFile...) + } + return keyValues, nil +} +``` + +- [ ] **Step 4: Add `import "os"` to test file and run tests** + +Run: `cd pkg/dotfiles && go test -v ./...` +Expected: PASS for all helper function tests + +- [ ] **Step 5: Update `cmd/up.go` to use `pkg/dotfiles`** + +Replace the `setupDotfiles` call in `cmd/up.go` `configureWorkspace` method (around line 385): + +```go +// Old: +if err := setupDotfiles( + cmd.DotfilesSource, + cmd.DotfilesScript, + cmd.DotfilesScriptEnvFile, + cmd.DotfilesScriptEnv, + client, + devPodConfig, + log, +); err != nil { + return err +} + +// New: +if err := dotfiles.Setup(dotfiles.SetupParams{ + Source: cmd.DotfilesSource, + Script: cmd.DotfilesScript, + EnvFiles: cmd.DotfilesScriptEnvFile, + EnvKeyValues: cmd.DotfilesScriptEnv, + Client: client, + DevPodConfig: devPodConfig, + Log: log, +}); err != nil { + return err +} +``` + +Remove the following functions from `cmd/up.go`: `setupDotfiles`, `buildDotCmd`, `buildDotCmdAgentArguments`, `extractKeysFromEnvKeyValuePairs`, `collectDotfilesScriptEnvKeyvaluePairs`. + +Add import: `"github.com/skevetter/devpod/pkg/dotfiles"` + +Remove now-unused imports from `cmd/up.go`: `"slices"` (if no other uses remain). + +- [ ] **Step 6: Verify build and existing tests pass** + +Run: `go build ./... && go test ./cmd/... ./pkg/dotfiles/...` +Expected: BUILD OK, PASS + +- [ ] **Step 7: Commit** + +```bash +git add pkg/dotfiles/ cmd/up.go +git commit -m "$(cat <<'EOF' +refactor: extract pkg/dotfiles for dotfiles setup + +Move dotfiles cloning and installation logic from cmd/up.go into a +dedicated pkg/dotfiles package. Add unit tests for pure helper functions. +EOF +)" +``` + +--- + +### Task 2: Extract `pkg/ide/opener/` + +**Files:** +- Create: `pkg/ide/opener/opener.go` +- Create: `pkg/ide/opener/opener_test.go` +- Modify: `cmd/up.go:400-583, 840-1056` (remove moved functions) + +- [ ] **Step 1: Write failing tests** + +Create `pkg/ide/opener/opener_test.go`: + +```go +package opener + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestParseAddressAndPort(t *testing.T) { + tests := []struct { + name string + bindAddr string + defaultPort int + wantAddr string + wantPort int + wantErr bool + }{ + { + name: "empty uses default port", + bindAddr: "", + defaultPort: 8080, + wantAddr: "", // will be the found port as string + wantPort: 0, // will be >= defaultPort + wantErr: false, + }, + { + name: "explicit host:port", + bindAddr: "127.0.0.1:9090", + defaultPort: 8080, + wantAddr: "127.0.0.1:9090", + wantPort: 9090, + wantErr: false, + }, + { + name: "missing port returns error", + bindAddr: "127.0.0.1:", + defaultPort: 8080, + wantErr: true, + }, + { + name: "invalid format returns error", + bindAddr: "not-a-host-port", + defaultPort: 8080, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + addr, port, err := ParseAddressAndPort(tt.bindAddr, tt.defaultPort) + if tt.wantErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + if tt.bindAddr == "" { + // When empty, a free port is found + assert.Greater(t, port, 0) + } else { + assert.Equal(t, tt.wantAddr, addr) + assert.Equal(t, tt.wantPort, port) + } + }) + } +} +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `cd pkg/ide/opener && go test -v ./...` +Expected: FAIL -- package doesn't exist yet + +- [ ] **Step 3: Create `pkg/ide/opener/opener.go`** + +Move the following from `cmd/up.go`: +- `ideOpener` struct and methods -> restructured as package-level `Open` function +- `openVSCodeFlavor` -> unexported +- `openJetBrains` -> unexported +- `startVSCodeInBrowser` -> unexported +- `startJupyterNotebookInBrowser` -> unexported +- `startRStudioInBrowser` -> unexported +- `startFleet` -> unexported +- `parseAddressAndPort` -> exported as `ParseAddressAndPort` (used by tests, useful utility) + +```go +package opener + +import ( + "bytes" + "context" + "fmt" + "net" + "os" + "os/exec" + "strconv" + "strings" + + "github.com/sirupsen/logrus" + client2 "github.com/skevetter/devpod/pkg/client" + "github.com/skevetter/devpod/pkg/command" + "github.com/skevetter/devpod/pkg/config" + config2 "github.com/skevetter/devpod/pkg/devcontainer/config" + "github.com/skevetter/devpod/pkg/ide/fleet" + "github.com/skevetter/devpod/pkg/ide/jetbrains" + "github.com/skevetter/devpod/pkg/ide/jupyter" + "github.com/skevetter/devpod/pkg/ide/openvscode" + "github.com/skevetter/devpod/pkg/ide/rstudio" + "github.com/skevetter/devpod/pkg/ide/vscode" + "github.com/skevetter/devpod/pkg/ide/zed" + open2 "github.com/skevetter/devpod/pkg/open" + "github.com/skevetter/devpod/pkg/port" + "github.com/skevetter/log" + "github.com/skratchdot/open-golang/open" +) + +// Params holds everything the IDE opener needs from the command layer. +type Params struct { + GPGAgentForwarding bool + SSHAuthSockID string + GitSSHSigningKey string + DevPodConfig *config.Config + Client client2.BaseWorkspaceClient + User string + Result *config2.Result + Log log.Logger +} + +// Open launches the appropriate IDE based on ideName. +func Open( + ctx context.Context, + ideName string, + ideOptions map[string]config.OptionValue, + params Params, +) error { + folder := params.Result.SubstitutionContext.ContainerWorkspaceFolder + workspace := params.Client.Workspace() + user := params.User + + switch ideName { + case string(config.IDEVSCode), string(config.IDEVSCodeInsiders), string(config.IDECursor), + string(config.IDECodium), string(config.IDEPositron), string(config.IDEWindsurf), + string(config.IDEAntigravity), string(config.IDEBob): + return openVSCodeFlavor(ctx, ideName, folder, workspace, ideOptions, params.Log) + + case string(config.IDERustRover), string(config.IDEGoland), string(config.IDEPyCharm), + string(config.IDEPhpStorm), string(config.IDEIntellij), string(config.IDECLion), + string(config.IDERider), string(config.IDERubyMine), string(config.IDEWebStorm), + string(config.IDEDataSpell): + return openJetBrains(ideName, folder, workspace, user, ideOptions, params.Log) + + case string(config.IDEOpenVSCode): + return startVSCodeInBrowser( + params.GPGAgentForwarding, ctx, params.DevPodConfig, params.Client, + folder, user, ideOptions, params.SSHAuthSockID, params.GitSSHSigningKey, params.Log, + ) + + case string(config.IDEFleet): + return startFleet(ctx, params.Client, params.Log) + + case string(config.IDEZed): + return zed.Open(ctx, ideOptions, user, folder, workspace, params.Log) + + case string(config.IDEJupyterNotebook): + return startJupyterNotebookInBrowser( + params.GPGAgentForwarding, ctx, params.DevPodConfig, params.Client, + user, ideOptions, params.SSHAuthSockID, params.GitSSHSigningKey, params.Log, + ) + + case string(config.IDERStudio): + return startRStudioInBrowser( + params.GPGAgentForwarding, ctx, params.DevPodConfig, params.Client, + user, ideOptions, params.SSHAuthSockID, params.GitSSHSigningKey, params.Log, + ) + + default: + return nil + } +} + +// ParseAddressAndPort parses a bind address option into address and port. +// If bindAddressOption is empty, finds an available port starting from defaultPort. +func ParseAddressAndPort(bindAddressOption string, defaultPort int) (string, int, error) { + if bindAddressOption == "" { + portName, err := port.FindAvailablePort(defaultPort) + if err != nil { + return "", 0, err + } + return fmt.Sprintf("%d", portName), portName, nil + } + + address := bindAddressOption + _, portStr, err := net.SplitHostPort(address) + if err != nil { + return "", 0, fmt.Errorf("parse host:port: %w", err) + } + if portStr == "" { + return "", 0, fmt.Errorf("parse ADDRESS: expected host:port, got %s", address) + } + + portName, err := strconv.Atoi(portStr) + if err != nil { + return "", 0, fmt.Errorf("parse host:port: %w", err) + } + + return address, portName, nil +} + +func openVSCodeFlavor( + ctx context.Context, + ideName, folder, workspace string, + ideOptions map[string]config.OptionValue, + logger log.Logger, +) error { + flavorMap := map[string]vscode.Flavor{ + string(config.IDEVSCode): vscode.FlavorStable, + string(config.IDEVSCodeInsiders): vscode.FlavorInsiders, + string(config.IDECursor): vscode.FlavorCursor, + string(config.IDECodium): vscode.FlavorCodium, + string(config.IDEPositron): vscode.FlavorPositron, + string(config.IDEWindsurf): vscode.FlavorWindsurf, + string(config.IDEAntigravity): vscode.FlavorAntigravity, + string(config.IDEBob): vscode.FlavorBob, + } + + params := vscode.OpenParams{ + Workspace: workspace, + Folder: folder, + NewWindow: vscode.Options.GetValue(ideOptions, vscode.OpenNewWindow) == config.BoolTrue, + Flavor: flavorMap[ideName], + Log: logger, + } + + return vscode.Open(ctx, params) +} + +func openJetBrains( + ideName, folder, workspace, user string, + ideOptions map[string]config.OptionValue, + logger log.Logger, +) error { + type jetbrainsFactory func() interface{ OpenGateway(string, string) error } + + jetbrainsMap := map[string]jetbrainsFactory{ + string(config.IDERustRover): func() interface{ OpenGateway(string, string) error } { + return jetbrains.NewRustRoverServer(user, ideOptions, logger) + }, + string(config.IDEGoland): func() interface{ OpenGateway(string, string) error } { + return jetbrains.NewGolandServer(user, ideOptions, logger) + }, + string(config.IDEPyCharm): func() interface{ OpenGateway(string, string) error } { + return jetbrains.NewPyCharmServer(user, ideOptions, logger) + }, + string(config.IDEPhpStorm): func() interface{ OpenGateway(string, string) error } { + return jetbrains.NewPhpStorm(user, ideOptions, logger) + }, + string(config.IDEIntellij): func() interface{ OpenGateway(string, string) error } { + return jetbrains.NewIntellij(user, ideOptions, logger) + }, + string(config.IDECLion): func() interface{ OpenGateway(string, string) error } { + return jetbrains.NewCLionServer(user, ideOptions, logger) + }, + string(config.IDERider): func() interface{ OpenGateway(string, string) error } { + return jetbrains.NewRiderServer(user, ideOptions, logger) + }, + string(config.IDERubyMine): func() interface{ OpenGateway(string, string) error } { + return jetbrains.NewRubyMineServer(user, ideOptions, logger) + }, + string(config.IDEWebStorm): func() interface{ OpenGateway(string, string) error } { + return jetbrains.NewWebStormServer(user, ideOptions, logger) + }, + string(config.IDEDataSpell): func() interface{ OpenGateway(string, string) error } { + return jetbrains.NewDataSpellServer(user, ideOptions, logger) + }, + } + + if factory, ok := jetbrainsMap[ideName]; ok { + return factory().OpenGateway(folder, workspace) + } + return fmt.Errorf("unknown JetBrains IDE: %s", ideName) +} + +// startVSCodeInBrowser, startJupyterNotebookInBrowser, startRStudioInBrowser, startFleet +// are moved here verbatim from cmd/up.go. They call into pkg/tunnel for browser tunnels +// and pkg/gpg for GPG forwarding (after those packages are extracted in Tasks 3-4). +// Until then, they import the functions directly. The full code for these functions +// is identical to cmd/up.go lines 840-993 and 995-1056. +``` + +**Note for implementer:** The `startVSCodeInBrowser`, `startJupyterNotebookInBrowser`, `startRStudioInBrowser`, and `startFleet` functions should be copied verbatim from `cmd/up.go` into `pkg/ide/opener/opener.go`. This temporarily duplicates `performGpgForwarding`, `startBrowserTunnel`, and `createSSHCommand` — they exist in both `cmd/up.go` (still used by other callers) and `pkg/ide/opener/`. Tasks 3-4 resolve this: `performGpgForwarding` moves to `pkg/gpg`, `startBrowserTunnel` and `createSSHCommand` move to `pkg/tunnel`, and `pkg/ide/opener` is updated to import from those packages. The duplication lives for exactly 2 commits. + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `cd pkg/ide/opener && go test -v ./...` +Expected: PASS + +- [ ] **Step 5: Update `cmd/up.go` to use `pkg/ide/opener`** + +Replace the `openIDE` method in `cmd/up.go`: + +```go +// Old: +func (cmd *UpCmd) openIDE( + ctx context.Context, + devPodConfig *config.Config, + client client2.BaseWorkspaceClient, + wctx *workspaceContext, + log log.Logger, +) error { + if !cmd.OpenIDE { + return nil + } + ideConfig := client.WorkspaceConfig().IDE + o := newIDEOpener(cmd, devPodConfig, client, wctx, log) + return o.open(ctx, ideConfig.Name, ideConfig.Options) +} + +// New: +func (cmd *UpCmd) openIDE( + ctx context.Context, + devPodConfig *config.Config, + client client2.BaseWorkspaceClient, + wctx *workspaceContext, + log log.Logger, +) error { + if !cmd.OpenIDE { + return nil + } + ideConfig := client.WorkspaceConfig().IDE + return opener.Open(ctx, ideConfig.Name, ideConfig.Options, opener.Params{ + GPGAgentForwarding: cmd.GPGAgentForwarding, + SSHAuthSockID: cmd.SSHAuthSockID, + GitSSHSigningKey: cmd.GitSSHSigningKey, + DevPodConfig: devPodConfig, + Client: client, + User: wctx.user, + Result: wctx.result, + Log: log, + }) +} +``` + +Remove from `cmd/up.go`: `ideOpener` struct, `newIDEOpener`, `ideOpener.open`, `ideOpener.openVSCodeFlavor`, `ideOpener.openJetBrains`, `startVSCodeInBrowser`, `startJupyterNotebookInBrowser`, `startRStudioInBrowser`, `startFleet`, `parseAddressAndPort`. + +Add import: `"github.com/skevetter/devpod/pkg/ide/opener"` + +Remove now-unused imports: IDE-specific packages (`vscode`, `jetbrains`, `jupyter`, `openvscode`, `rstudio`, `fleet`, `zed`), `"net"`, `open2`, `"github.com/skratchdot/open-golang/open"`. + +- [ ] **Step 6: Verify build and tests pass** + +Run: `go build ./... && go test ./cmd/... ./pkg/ide/opener/...` +Expected: BUILD OK, PASS + +- [ ] **Step 7: Commit** + +```bash +git add pkg/ide/opener/ cmd/up.go +git commit -m "$(cat <<'EOF' +refactor: extract pkg/ide/opener for IDE launch dispatch + +Move IDE opening logic (VSCode, JetBrains, browser-based IDEs, Fleet, Zed) +from cmd/up.go into a dedicated pkg/ide/opener package with unit tests. +EOF +)" +``` + +--- + +### Task 3: Extract `pkg/gpg/` + +**Files:** +- Create: `pkg/gpg/forward.go` +- Create: `pkg/gpg/forward_test.go` +- Modify: `pkg/ide/opener/opener.go` (update imports to use `pkg/gpg`) + +- [ ] **Step 1: Write failing test** + +Create `pkg/gpg/forward_test.go`: + +```go +package gpg + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestBuildForwardCommand(t *testing.T) { + args := buildForwardArgs("/usr/bin/devpod", "root", "test-context", "test-workspace") + assert.Contains(t, args, "ssh") + assert.Contains(t, args, "--gpg-agent-forwarding=true") + assert.Contains(t, args, "--agent-forwarding=true") + assert.Contains(t, args, "--user") + assert.Contains(t, args, "root") + assert.Contains(t, args, "--context") + assert.Contains(t, args, "test-context") + assert.Contains(t, args, "test-workspace") + assert.Contains(t, args, "--command") + assert.Contains(t, args, "sleep infinity") +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cd pkg/gpg && go test -v ./...` +Expected: FAIL + +- [ ] **Step 3: Create `pkg/gpg/forward.go`** + +```go +package gpg + +import ( + "os" + "os/exec" + + client2 "github.com/skevetter/devpod/pkg/client" + devssh "github.com/skevetter/devpod/pkg/ssh" + "github.com/skevetter/log" +) + +// ForwardAgent starts a background SSH connection that forwards the local +// GPG agent to the remote workspace. +func ForwardAgent(client client2.BaseWorkspaceClient, logger log.Logger) error { + logger.Debug("gpg forwarding enabled, performing immediately") + + execPath, err := os.Executable() + if err != nil { + return err + } + + remoteUser, err := devssh.GetUser( + client.WorkspaceConfig().ID, + client.WorkspaceConfig().SSHConfigPath, + client.WorkspaceConfig().SSHConfigIncludePath, + ) + if err != nil { + remoteUser = "root" + } + + logger.Info("forwarding gpg-agent") + + args := buildForwardArgs(execPath, remoteUser, client.Context(), client.Workspace()) + + go func() { + err = exec.Command(execPath, args...).Run() + if err != nil { + logger.Error("failure in forwarding gpg-agent") + } + }() + + return nil +} + +func buildForwardArgs(execPath, user, context, workspace string) []string { + return []string{ + "ssh", + "--gpg-agent-forwarding=true", + "--agent-forwarding=true", + "--start-services=true", + "--user", + user, + "--context", + context, + workspace, + "--log-output=raw", + "--command", "sleep infinity", + } +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `cd pkg/gpg && go test -v ./...` +Expected: PASS + +- [ ] **Step 5: Update `pkg/ide/opener/opener.go` to use `pkg/gpg`** + +Replace inline `performGpgForwarding` calls in the browser IDE functions with `gpg.ForwardAgent`. Add import `"github.com/skevetter/devpod/pkg/gpg"`. Remove `performGpgForwarding` from wherever it currently lives. + +- [ ] **Step 6: Remove `performGpgForwarding` from `cmd/up.go`** + +Delete the function (originally lines 1555-1600). Update imports. + +- [ ] **Step 7: Verify build and tests pass** + +Run: `go build ./... && go test ./pkg/gpg/... ./pkg/ide/opener/... ./cmd/...` +Expected: BUILD OK, PASS + +- [ ] **Step 8: Commit** + +```bash +git add pkg/gpg/ pkg/ide/opener/ cmd/up.go +git commit -m "$(cat <<'EOF' +refactor: extract pkg/gpg for GPG agent forwarding + +Move GPG agent forwarding logic into a dedicated pkg/gpg package. +Update pkg/ide/opener to use it for browser-based IDEs. +EOF +)" +``` + +--- + +### Task 4: Move browser tunnel and backhaul into `pkg/tunnel/` + +**Files:** +- Create: `pkg/tunnel/browser.go` +- Create: `pkg/tunnel/browser_test.go` +- Modify: `pkg/ide/opener/opener.go` (update to call `tunnel.StartBrowserTunnel`) +- Modify: `cmd/up.go` (remove `startBrowserTunnel`, `setupBackhaul`, `createSSHCommand`) + +- [ ] **Step 1: Write failing test** + +Create `pkg/tunnel/browser_test.go`: + +```go +package tunnel + +import ( + "testing" + + "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" +) + +func TestCreateSSHCommandArgs(t *testing.T) { + tests := []struct { + name string + context string + workspace string + debug bool + extraArgs []string + wantArgs []string + }{ + { + name: "basic command", + context: "default", + workspace: "my-workspace", + debug: false, + extraArgs: []string{"--stdio"}, + wantArgs: []string{ + "ssh", + "--user=root", + "--agent-forwarding=false", + "--start-services=false", + "--context", + "default", + "my-workspace", + "--stdio", + }, + }, + { + name: "with debug", + context: "default", + workspace: "my-workspace", + debug: true, + extraArgs: nil, + wantArgs: []string{ + "ssh", + "--user=root", + "--agent-forwarding=false", + "--start-services=false", + "--context", + "default", + "my-workspace", + "--debug", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + args := buildSSHCommandArgs(tt.context, tt.workspace, tt.debug, tt.extraArgs) + assert.Equal(t, tt.wantArgs, args) + }) + } +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cd pkg/tunnel && go test -v -run TestCreateSSHCommandArgs` +Expected: FAIL -- `buildSSHCommandArgs` undefined + +- [ ] **Step 3: Create `pkg/tunnel/browser.go`** + +Move from `cmd/up.go`: +- `startBrowserTunnel` -> exported as `StartBrowserTunnel` +- `setupBackhaul` -> exported as `SetupBackhaul` +- `createSSHCommand` -> exported as `CreateSSHCommand` (used by `pkg/ide/opener` for Fleet) +- Extract `buildSSHCommandArgs` as a testable pure function + +```go +package tunnel + +import ( + "context" + "fmt" + "io" + "os" + "os/exec" + + "github.com/sirupsen/logrus" + client2 "github.com/skevetter/devpod/pkg/client" + "github.com/skevetter/devpod/pkg/client/clientimplementation" + "github.com/skevetter/devpod/pkg/config" + devssh "github.com/skevetter/devpod/pkg/ssh" + "github.com/skevetter/log" + "golang.org/x/crypto/ssh" +) + +// BrowserTunnelParams holds parameters for starting a browser-based IDE tunnel. +type BrowserTunnelParams struct { + DevPodConfig *config.Config + Client client2.BaseWorkspaceClient + User string + TargetURL string + ForwardPorts bool + ExtraPorts []string + AuthSockID string + GitSSHSigningKey string + Log log.Logger +} + +// StartBrowserTunnel starts an SSH tunnel for browser-based IDEs. +func StartBrowserTunnel(ctx context.Context, params BrowserTunnelParams) error { + // Setup backhaul if authSockID is set + if params.AuthSockID != "" { + go func() { + if err := SetupBackhaul(params.Client, params.AuthSockID, params.Log); err != nil { + params.Log.Error("Failed to setup backhaul SSH connection: ", err) + } + }() + } + + // handle daemon client directly + daemonClient, ok := params.Client.(client2.DaemonClient) + if ok { + toolClient, _, err := daemonClient.SSHClients(ctx, params.User) + if err != nil { + return err + } + defer func() { _ = toolClient.Close() }() + + err = clientimplementation.StartServicesDaemon( + ctx, + clientimplementation.StartServicesDaemonOptions{ + DevPodConfig: params.DevPodConfig, + Client: daemonClient, + SSHClient: toolClient, + User: params.User, + Log: params.Log, + ForwardPorts: params.ForwardPorts, + ExtraPorts: params.ExtraPorts, + }, + ) + if err != nil { + return err + } + <-ctx.Done() + return nil + } + + err := NewTunnel( + ctx, + func(ctx context.Context, stdin io.Reader, stdout io.Writer) error { + writer := params.Log.Writer(logrus.DebugLevel, false) + defer func() { _ = writer.Close() }() + + cmd, err := CreateSSHCommand(ctx, params.Client, params.Log, []string{ + "--log-output=raw", + fmt.Sprintf("--reuse-ssh-auth-sock=%s", params.AuthSockID), + "--stdio", + }) + if err != nil { + return err + } + cmd.Stdout = stdout + cmd.Stdin = stdin + cmd.Stderr = writer + return cmd.Run() + }, + func(ctx context.Context, containerClient *ssh.Client) error { + streamLogger, ok := params.Log.(*log.StreamLogger) + if ok { + streamLogger.JSON(logrus.InfoLevel, map[string]string{ + "url": params.TargetURL, + "done": "true", + }) + } + + configureDockerCredentials := params.DevPodConfig.ContextOption( + config.ContextOptionSSHInjectDockerCredentials, + ) == config.BoolTrue + configureGitCredentials := params.DevPodConfig.ContextOption( + config.ContextOptionSSHInjectGitCredentials, + ) == config.BoolTrue + configureGitSSHSignatureHelper := params.DevPodConfig.ContextOption( + config.ContextOptionGitSSHSignatureForwarding, + ) == config.BoolTrue + + err := RunServices( + ctx, + RunServicesOptions{ + DevPodConfig: params.DevPodConfig, + ContainerClient: containerClient, + User: params.User, + ForwardPorts: params.ForwardPorts, + ExtraPorts: params.ExtraPorts, + PlatformOptions: nil, + Workspace: params.Client.WorkspaceConfig(), + ConfigureDockerCredentials: configureDockerCredentials, + ConfigureGitCredentials: configureGitCredentials, + ConfigureGitSSHSignatureHelper: configureGitSSHSignatureHelper, + GitSSHSigningKey: params.GitSSHSigningKey, + Log: params.Log, + }, + ) + if err != nil { + return fmt.Errorf("run credentials server in browser tunnel: %w", err) + } + + <-ctx.Done() + return nil + }, + ) + return err +} + +// SetupBackhaul sets up a long-running SSH connection to keep an AUTH_SOCK alive. +func SetupBackhaul(client client2.BaseWorkspaceClient, authSockID string, logger log.Logger) error { + execPath, err := os.Executable() + if err != nil { + return err + } + + remoteUser, err := devssh.GetUser( + client.WorkspaceConfig().ID, + client.WorkspaceConfig().SSHConfigPath, + client.WorkspaceConfig().SSHConfigIncludePath, + ) + if err != nil { + remoteUser = "root" + } + + args := []string{ + "ssh", + "--agent-forwarding=true", + fmt.Sprintf("--reuse-ssh-auth-sock=%s", authSockID), + "--start-services=false", + "--user", + remoteUser, + "--context", + client.Context(), + client.Workspace(), + "--log-output=raw", + "--command", + "while true; do sleep 6000000; done", + } + + if logger.GetLevel() == logrus.DebugLevel { + args = append(args, "--debug") + } + + logger.Info("Setting up backhaul SSH connection") + + writer := logger.Writer(logrus.InfoLevel, false) + dotCmd := exec.Command(execPath, args...) + dotCmd.Stdout = writer + dotCmd.Stderr = writer + + if err := dotCmd.Run(); err != nil { + return err + } + + logger.Infof("Done setting up backhaul") + return nil +} + +// CreateSSHCommand builds an exec.Cmd for SSH into the workspace. +func CreateSSHCommand( + ctx context.Context, + client client2.BaseWorkspaceClient, + logger log.Logger, + extraArgs []string, +) (*exec.Cmd, error) { + execPath, err := os.Executable() + if err != nil { + return nil, err + } + + args := buildSSHCommandArgs( + client.Context(), + client.Workspace(), + logger.GetLevel() == logrus.DebugLevel, + extraArgs, + ) + + return exec.CommandContext(ctx, execPath, args...), nil +} + +func buildSSHCommandArgs(context, workspace string, debug bool, extraArgs []string) []string { + args := []string{ + "ssh", + "--user=root", + "--agent-forwarding=false", + "--start-services=false", + "--context", + context, + workspace, + } + if debug { + args = append(args, "--debug") + } + args = append(args, extraArgs...) + return args +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `cd pkg/tunnel && go test -v -run TestCreateSSHCommandArgs` +Expected: PASS + +- [ ] **Step 5: Update `pkg/ide/opener/opener.go`** + +Replace inline `startBrowserTunnel` calls with `tunnel.StartBrowserTunnel`. Replace `createSSHCommand` calls (in `startFleet`) with `tunnel.CreateSSHCommand`. Add import `"github.com/skevetter/devpod/pkg/tunnel"`. + +- [ ] **Step 6: Remove from `cmd/up.go`** + +Delete: `startBrowserTunnel`, `setupBackhaul`, `createSSHCommand`. +Remove now-unused imports: `"io"`, `"golang.org/x/crypto/ssh"`. + +- [ ] **Step 7: Verify build and tests pass** + +Run: `go build ./... && go test ./pkg/tunnel/... ./pkg/ide/opener/... ./cmd/...` +Expected: BUILD OK, PASS + +- [ ] **Step 8: Commit** + +```bash +git add pkg/tunnel/browser.go pkg/tunnel/browser_test.go pkg/ide/opener/ cmd/up.go +git commit -m "$(cat <<'EOF' +refactor: move browser tunnel and backhaul into pkg/tunnel + +Move startBrowserTunnel, setupBackhaul, and createSSHCommand from cmd/up.go +into pkg/tunnel where they naturally belong alongside existing tunnel logic. +EOF +)" +``` + +--- + +### Task 5: Move provider update check into `pkg/workspace/` + +**Files:** +- Modify: `pkg/workspace/provider.go` (add CheckProviderUpdate, GetProInstance) +- Create: `pkg/workspace/provider_update_test.go` +- Modify: `cmd/up.go` (remove `checkProviderUpdate`, `getProInstance`) + +- [ ] **Step 1: Write failing tests** + +Create `pkg/workspace/provider_update_test.go`: + +```go +package workspace + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestShouldSkipProviderUpdate(t *testing.T) { + tests := []struct { + name string + currentVersion string + isDevVersion bool + isInternal bool + wantSkip bool + }{ + { + name: "dev version skips", + isDevVersion: true, + wantSkip: true, + }, + { + name: "internal source skips", + isInternal: true, + wantSkip: true, + }, + { + name: "same version skips", + currentVersion: "v0.5.0", + wantSkip: false, // handled by semver comparison, not this function + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := shouldSkipProviderUpdate(tt.isDevVersion, tt.isInternal) + assert.Equal(t, tt.wantSkip, got) + }) + } +} + +func TestGetProInstance_NilWhenNoInstances(t *testing.T) { + // GetProInstance with nil config should return nil gracefully + result := GetProInstance(nil, "some-provider", nil) + assert.Nil(t, result) +} +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `cd pkg/workspace && go test -v -run "TestShouldSkip|TestGetPro"` +Expected: FAIL -- functions not defined + +- [ ] **Step 3: Add functions to `pkg/workspace/provider.go`** + +Append to existing `pkg/workspace/provider.go`: + +```go +// CheckProviderUpdate ensures the provider version matches the pro instance version. +func CheckProviderUpdate( + devPodConfig *config.Config, + proInstance *provider2.ProInstance, + log log.Logger, +) error { + if version.GetVersion() == version.DevVersion { + log.Debugf("skipping provider upgrade check during development") + return nil + } + if proInstance == nil { + log.Debug("no pro instance available, skipping provider upgrade check") + return nil + } + + newVersion, err := platform.GetProInstanceDevPodVersion(proInstance) + if err != nil { + return fmt.Errorf("version for pro instance %s: %w", proInstance.Host, err) + } + + p, err := FindProvider(devPodConfig, proInstance.Provider, log) + if err != nil { + return fmt.Errorf("get provider config for pro provider %s: %w", proInstance.Provider, err) + } + if shouldSkipProviderUpdate( + p.Config.Version == version.DevVersion, + p.Config.Source.Internal, + ) { + return nil + } + + v1, err := semver.Parse(strings.TrimPrefix(newVersion, "v")) + if err != nil { + return fmt.Errorf("parse version %s: %w", newVersion, err) + } + v2, err := semver.Parse(strings.TrimPrefix(p.Config.Version, "v")) + if err != nil { + return fmt.Errorf("parse version %s: %w", p.Config.Version, err) + } + if v1.Compare(v2) == 0 { + return nil + } + log.Infof( + "New provider version available, attempting to update %s from %s to %s", + proInstance.Provider, + p.Config.Version, + newVersion, + ) + + providerSource, err := ResolveProviderSource(devPodConfig, proInstance.Provider, log) + if err != nil { + return fmt.Errorf("resolve provider source %s: %w", proInstance.Provider, err) + } + + splitted := strings.Split(providerSource, "@") + if len(splitted) == 0 { + return fmt.Errorf("no provider source found %s", providerSource) + } + providerSource = splitted[0] + "@" + newVersion + + _, err = UpdateProvider(devPodConfig, proInstance.Provider, providerSource, log) + if err != nil { + return fmt.Errorf("update provider %s: %w", proInstance.Provider, err) + } + + log.WithFields(logrus.Fields{ + "provider": proInstance.Provider, + }).Done("updated provider") + return nil +} + +func shouldSkipProviderUpdate(isDevVersion, isInternal bool) bool { + return isDevVersion || isInternal +} + +// GetProInstance finds the pro instance associated with a provider. +func GetProInstance( + devPodConfig *config.Config, + providerName string, + log log.Logger, +) *provider2.ProInstance { + if devPodConfig == nil { + return nil + } + + proInstances, err := ListProInstances(devPodConfig, log) + if err != nil { + return nil + } + if len(proInstances) == 0 { + return nil + } + + proInstance, ok := FindProviderProInstance(proInstances, providerName) + if !ok { + return nil + } + + return proInstance +} +``` + +Add necessary imports to `pkg/workspace/provider.go`: `"github.com/blang/semver/v4"`, `"github.com/sirupsen/logrus"`, `"github.com/skevetter/devpod/pkg/platform"`, `"github.com/skevetter/devpod/pkg/version"`. + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `cd pkg/workspace && go test -v -run "TestShouldSkip|TestGetPro"` +Expected: PASS + +- [ ] **Step 5: Update `cmd/up.go`** + +In `prepareClient` method, replace: + +```go +// Old: +proInstance := getProInstance(devPodConfig, client.Provider(), logger) +err = checkProviderUpdate(devPodConfig, proInstance, logger) + +// New: +proInstance := workspace2.GetProInstance(devPodConfig, client.Provider(), logger) +err = workspace2.CheckProviderUpdate(devPodConfig, proInstance, logger) +``` + +Remove from `cmd/up.go`: `checkProviderUpdate`, `getProInstance` functions. +Remove now-unused imports: `"github.com/blang/semver/v4"`, `"github.com/skevetter/devpod/pkg/platform"`, `"github.com/skevetter/devpod/pkg/version"` (if no other uses). + +- [ ] **Step 6: Verify build and tests pass** + +Run: `go build ./... && go test ./pkg/workspace/... ./cmd/...` +Expected: BUILD OK, PASS + +- [ ] **Step 7: Commit** + +```bash +git add pkg/workspace/provider.go pkg/workspace/provider_update_test.go cmd/up.go +git commit -m "$(cat <<'EOF' +refactor: move provider update check into pkg/workspace + +Move checkProviderUpdate and getProInstance from cmd/up.go into +pkg/workspace where they naturally belong alongside FindProvider +and UpdateProvider. +EOF +)" +``` + +--- + +### Task 6: Final cleanup of `cmd/up.go` + +**Files:** +- Modify: `cmd/up.go` + +- [ ] **Step 1: Remove all dead imports** + +Run: `go build ./cmd/...` + +Fix any unused import errors. The following imports should now be removable from `cmd/up.go` (verify each): +- `"bytes"` (was used by `startFleet`) +- `"io"` (was used by `startBrowserTunnel`) +- `"net"` (was used by `parseAddressAndPort`) +- `"slices"` (was used by dotfiles) +- `"github.com/blang/semver/v4"` (was used by `checkProviderUpdate`) +- Various IDE sub-packages (`fleet`, `jetbrains`, `jupyter`, `openvscode`, `rstudio`, `vscode`, `zed`) +- `"github.com/skevetter/devpod/pkg/command"` (was used by `startFleet`) +- `open2 "github.com/skevetter/devpod/pkg/open"` (was used by browser IDEs) +- `"github.com/skevetter/devpod/pkg/platform"` (was used by `checkProviderUpdate`) +- `"github.com/skevetter/devpod/pkg/port"` (was used by `parseAddressAndPort`) +- `"github.com/skevetter/devpod/pkg/version"` (was used by `checkProviderUpdate`) +- `devssh "github.com/skevetter/devpod/pkg/ssh"` (was used by `createSSHCommand` and `setupDotfiles`) +- `"github.com/skratchdot/open-golang/open"` (was used by `startFleet`) +- `"golang.org/x/crypto/ssh"` (was used by `startBrowserTunnel`) + +- [ ] **Step 2: Verify the file is ~350-400 lines** + +Run: `wc -l cmd/up.go` +Expected: approximately 350-450 lines + +- [ ] **Step 3: Verify full build and all tests pass** + +Run: `go build ./... && go test ./...` +Expected: BUILD OK, all PASS + +- [ ] **Step 4: Commit** + +```bash +git add cmd/up.go +git commit -m "$(cat <<'EOF' +refactor: clean up cmd/up.go after extractions + +Remove dead imports and unused code after extracting dotfiles, IDE opener, +GPG forwarding, browser tunnel, and provider update logic into pkg/. +EOF +)" +``` + +--- + +## Verification Checklist + +After all 6 commits: + +- [ ] `go build ./...` passes +- [ ] `go test ./...` passes +- [ ] `go vet ./...` passes +- [ ] `cmd/up.go` is under 450 lines +- [ ] No circular imports (cmd/ -> pkg/, pkg/ -> pkg/, never pkg/ -> cmd/) +- [ ] All new packages have test files diff --git a/docs/superpowers/specs/2026-04-11-cmd-up-refactor-design.md b/docs/superpowers/specs/2026-04-11-cmd-up-refactor-design.md new file mode 100644 index 000000000..d664c3f32 --- /dev/null +++ b/docs/superpowers/specs/2026-04-11-cmd-up-refactor-design.md @@ -0,0 +1,182 @@ +# Design: Extract cmd/up.go into pkg/ packages + +**Date:** 2026-04-11 +**Status:** Approved +**Approach:** Single PR, one commit per extracted package (Approach C) + +## Problem + +`cmd/up.go` is 1839 lines with 6 distinct responsibility areas packed into one file: +command definition, client preparation, devPodUp orchestration, IDE opening, +SSH/tunnel/credentials setup, and dotfiles/utilities. This makes it hard to +navigate, test, and modify. + +## Goal + +Break `cmd/up.go` into focused `pkg/` packages, each with unit tests. The file +should shrink to ~350-400 lines of command-level orchestration. All behavior must +be preserved. + +## Package Extractions + +### 1. `pkg/dotfiles/` (new) + +**Source lines:** 1384-1553 +**Functions moved:** `setupDotfiles`, `buildDotCmd`, `buildDotCmdAgentArguments`, +`extractKeysFromEnvKeyValuePairs`, `collectDotfilesScriptEnvKeyvaluePairs` + +**Exported API:** + +```go +package dotfiles + +type SetupParams struct { + Source string + Script string + EnvFiles []string + EnvKeyValues []string + Client client.BaseWorkspaceClient + DevPodConfig *config.Config + Log log.Logger +} + +func Setup(params SetupParams) error +``` + +**Tests:** Table-driven tests for `extractKeysFromEnvKeyValuePairs`, +`collectDotfilesScriptEnvKeyvaluePairs`, and `buildDotCmdAgentArguments` +(pure functions). Integration-style test for `Setup` with a mock client. + +### 2. `pkg/ide/opener/` (new) + +**Source lines:** 400-583, 840-1056 +**Functions moved:** `ideOpener` struct and methods, `openVSCodeFlavor`, +`openJetBrains`, `startVSCodeInBrowser`, `startJupyterNotebookInBrowser`, +`startRStudioInBrowser`, `startFleet`, `parseAddressAndPort` + +**Exported API:** + +```go +package opener + +type Params struct { + GPGAgentForwarding bool + SSHAuthSockID string + GitSSHSigningKey string + DevPodConfig *config.Config + Client client.BaseWorkspaceClient + User string + Workdir string + Result *config2.Result + Log log.Logger +} + +func Open(ctx context.Context, ideName string, ideOptions map[string]config.OptionValue, params Params) error +``` + +**Tests:** `parseAddressAndPort` table-driven tests. IDE dispatch routing test +(verify correct branch taken per IDE name). + +### 3. `pkg/gpg/` (new) + +**Source lines:** 1555-1600 +**Functions moved:** `performGpgForwarding` + +**Exported API:** + +```go +package gpg + +func ForwardAgent(client client.BaseWorkspaceClient, log log.Logger) error +``` + +**Tests:** Verify command construction arguments. + +### 4. Extend `pkg/tunnel/` (browser tunnel + backhaul) + +**Source lines:** 1089-1263 +**Functions moved:** `startBrowserTunnel`, `setupBackhaul`, `createSSHCommand` +(internal helper) + +**New exports:** + +```go +// In pkg/tunnel/ + +type BrowserTunnelParams struct { + DevPodConfig *config.Config + Client client.BaseWorkspaceClient + User string + TargetURL string + ForwardPorts bool + ExtraPorts []string + AuthSockID string + GitSSHSigningKey string + Log log.Logger +} + +func StartBrowserTunnel(ctx context.Context, params BrowserTunnelParams) error + +func SetupBackhaul(client client.BaseWorkspaceClient, authSockID string, log log.Logger) error +``` + +**Tests:** Unit test for SSH command argument construction. + +### 5. Move provider update check to `pkg/workspace/` + +**Source lines:** 1602-1693 +**Functions moved:** `checkProviderUpdate`, `getProInstance` + +**New exports:** + +```go +// In pkg/workspace/ + +func CheckProviderUpdate(devPodConfig *config.Config, proInstance *provider2.ProInstance, log log.Logger) error + +func GetProInstance(devPodConfig *config.Config, providerName string, log log.Logger) *provider2.ProInstance +``` + +**Tests:** Table-driven test for version comparison logic (dev version skip, +same version skip, upgrade path). + +## What stays in cmd/up.go (~350-400 lines) + +- `UpCmd` struct, `NewUpCmd`, flag registration methods (~180 lines) +- `execute`, `validate`, `prepareClient` (command orchestration) +- `Run`, `prepareWorkspace`, `executeDevPodUp`, `configureWorkspace`, `openIDE` + (thin orchestration calling into pkg/) +- `devPodUp`, `devPodUpMachine`, `devPodUpProxy`, `devPodUpDaemon` (client + dispatch, tightly coupled to cobra context) +- `WithSignals`, `validatePodmanFlags`, `isValidMapping` (command utilities) +- `mergeDevPodUpOptions`, `mergeEnvFromFiles` (option merging) + +## Commit Sequence + +1. `refactor: extract pkg/dotfiles for dotfiles setup` +2. `refactor: extract pkg/ide/opener for IDE launch dispatch` +3. `refactor: extract pkg/gpg for GPG agent forwarding` +4. `refactor: move browser tunnel and backhaul into pkg/tunnel` +5. `refactor: move provider update check into pkg/workspace` +6. `refactor: clean up cmd/up.go after extractions` + +Each commit moves a responsibility area out, adds tests, and updates +`cmd/up.go` to call the new package. No intermediate broken state. + +## Cross-cutting Dependencies + +`createSSHCommand` is used by `startBrowserTunnel`, `setupBackhaul`, and +`startFleet` (IDE opener). It moves to `pkg/tunnel/` as an internal helper, +and `pkg/ide/opener/` imports `pkg/tunnel/` to reuse it (or it gets promoted +to an exported function in `pkg/tunnel/`). Direction is still pkg/ -> pkg/, +no circular risk. + +## Risks and Mitigations + +- **Circular imports:** The extracted packages depend on `pkg/client`, + `pkg/config`, `pkg/ssh` etc. but not on `cmd/`. Direction is always + cmd/ -> pkg/, never reverse. No circular risk. +- **Behavior regression:** Pure refactor with no logic changes. Existing + integration tests plus new unit tests provide coverage. +- **Large PR:** Mitigated by clean per-commit structure. Each commit is + independently reviewable. From a66b86f0671a7d57cf6aec0f55185e1a1ce010a6 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Sat, 11 Apr 2026 23:06:45 -0500 Subject: [PATCH 2/8] refactor: extract pkg/dotfiles for dotfiles setup --- cmd/up.go | 190 ++----------------------------- pkg/dotfiles/dotfiles.go | 205 ++++++++++++++++++++++++++++++++++ pkg/dotfiles/dotfiles_test.go | 121 ++++++++++++++++++++ 3 files changed, 336 insertions(+), 180 deletions(-) create mode 100644 pkg/dotfiles/dotfiles.go create mode 100644 pkg/dotfiles/dotfiles_test.go diff --git a/cmd/up.go b/cmd/up.go index 53edc923a..edadcab38 100644 --- a/cmd/up.go +++ b/cmd/up.go @@ -10,7 +10,6 @@ import ( "os/exec" "os/signal" "path/filepath" - "slices" "strconv" "strings" "syscall" @@ -26,6 +25,7 @@ import ( "github.com/skevetter/devpod/pkg/config" config2 "github.com/skevetter/devpod/pkg/devcontainer/config" "github.com/skevetter/devpod/pkg/devcontainer/sshtunnel" + "github.com/skevetter/devpod/pkg/dotfiles" "github.com/skevetter/devpod/pkg/ide" "github.com/skevetter/devpod/pkg/ide/fleet" "github.com/skevetter/devpod/pkg/ide/jetbrains" @@ -382,15 +382,15 @@ func (cmd *UpCmd) configureWorkspace( log.Info("SSH configuration completed in workspace") } - if err := setupDotfiles( - cmd.DotfilesSource, - cmd.DotfilesScript, - cmd.DotfilesScriptEnvFile, - cmd.DotfilesScriptEnv, - client, - devPodConfig, - log, - ); err != nil { + if err := dotfiles.Setup(dotfiles.SetupParams{ + Source: cmd.DotfilesSource, + Script: cmd.DotfilesScript, + EnvFiles: cmd.DotfilesScriptEnvFile, + EnvKeyValues: cmd.DotfilesScriptEnv, + Client: client, + DevPodConfig: devPodConfig, + Log: log, + }); err != nil { return err } @@ -1382,176 +1382,6 @@ func createSSHCommand( return exec.CommandContext(ctx, execPath, args...), nil } -func setupDotfiles( - dotfiles, script string, - envFiles, envKeyValuePairs []string, - client client2.BaseWorkspaceClient, - devPodConfig *config.Config, - log log.Logger, -) error { - dotfilesRepo := devPodConfig.ContextOption(config.ContextOptionDotfilesURL) - if dotfiles != "" { - dotfilesRepo = dotfiles - } - - dotfilesScript := devPodConfig.ContextOption(config.ContextOptionDotfilesScript) - if script != "" { - dotfilesScript = script - } - - if dotfilesRepo == "" { - log.Debug("No dotfiles repo specified, skipping") - return nil - } - - log.Infof("Dotfiles Git repository %s specified", dotfilesRepo) - log.Debug("Cloning dotfiles into the devcontainer...") - - dotCmd, err := buildDotCmd( - devPodConfig, - dotfilesRepo, - dotfilesScript, - envFiles, - envKeyValuePairs, - client, - log, - ) - if err != nil { - return err - } - if log.GetLevel() == logrus.DebugLevel { - dotCmd.Args = append(dotCmd.Args, "--debug") - } - - log.Debugf("Running dotfiles setup command: %v", dotCmd.Args) - - writer := log.Writer(logrus.InfoLevel, false) - - dotCmd.Stdout = writer - dotCmd.Stderr = writer - - err = dotCmd.Run() - if err != nil { - return err - } - - log.Infof("Done setting up dotfiles into the devcontainer") - - return nil -} - -func buildDotCmdAgentArguments( - devPodConfig *config.Config, - dotfilesRepo, dotfilesScript string, - log log.Logger, -) []string { - agentArguments := []string{ - "agent", - "workspace", - "install-dotfiles", - "--repository", - dotfilesRepo, - } - - if devPodConfig.ContextOption(config.ContextOptionSSHStrictHostKeyChecking) == config.BoolTrue { - agentArguments = append(agentArguments, "--strict-host-key-checking") - } - - if log.GetLevel() == logrus.DebugLevel { - agentArguments = append(agentArguments, "--debug") - } - - if dotfilesScript != "" { - log.Infof("Dotfiles script %s specified", dotfilesScript) - agentArguments = append(agentArguments, "--install-script", dotfilesScript) - } - - return agentArguments -} - -func buildDotCmd( - devPodConfig *config.Config, - dotfilesRepo, dotfilesScript string, - envFiles, envKeyValuePairs []string, - client client2.BaseWorkspaceClient, - log log.Logger, -) (*exec.Cmd, error) { - sshCmd := []string{ - "ssh", - "--agent-forwarding=true", - "--start-services=true", - } - - envFilesKeyValuePairs, err := collectDotfilesScriptEnvKeyvaluePairs(envFiles) - if err != nil { - return nil, err - } - - // Collect file-based and CLI options env variables names (aka keys) and - // configure ssh env var passthrough with send-env - allEnvKeyValuesPairs := slices.Concat(envFilesKeyValuePairs, envKeyValuePairs) - allEnvKeys := extractKeysFromEnvKeyValuePairs(allEnvKeyValuesPairs) - for _, envKey := range allEnvKeys { - sshCmd = append(sshCmd, "--send-env", envKey) - } - - remoteUser, err := devssh.GetUser( - client.WorkspaceConfig().ID, - client.WorkspaceConfig().SSHConfigPath, - client.WorkspaceConfig().SSHConfigIncludePath, - ) - if err != nil { - remoteUser = "root" - } - - agentArguments := buildDotCmdAgentArguments(devPodConfig, dotfilesRepo, dotfilesScript, log) - sshCmd = append(sshCmd, - "--user", - remoteUser, - "--context", - client.Context(), - client.Workspace(), - "--log-output=raw", - "--command", - agent.ContainerDevPodHelperLocation+" "+strings.Join(agentArguments, " "), - ) - execPath, err := os.Executable() - if err != nil { - return nil, err - } - - dotCmd := exec.Command( - execPath, - sshCmd..., - ) - - dotCmd.Env = append(dotCmd.Environ(), allEnvKeyValuesPairs...) - return dotCmd, nil -} - -func extractKeysFromEnvKeyValuePairs(envKeyValuePairs []string) []string { - keys := []string{} - for _, env := range envKeyValuePairs { - keyValue := strings.SplitN(env, "=", 2) - if len(keyValue) == 2 { - keys = append(keys, keyValue[0]) - } - } - return keys -} - -func collectDotfilesScriptEnvKeyvaluePairs(envFiles []string) ([]string, error) { - keyValues := []string{} - for _, file := range envFiles { - envFromFile, err := config2.ParseKeyValueFile(file) - if err != nil { - return nil, err - } - keyValues = append(keyValues, envFromFile...) - } - return keyValues, nil -} - func performGpgForwarding( client client2.BaseWorkspaceClient, log log.Logger, diff --git a/pkg/dotfiles/dotfiles.go b/pkg/dotfiles/dotfiles.go new file mode 100644 index 000000000..28bf552ba --- /dev/null +++ b/pkg/dotfiles/dotfiles.go @@ -0,0 +1,205 @@ +package dotfiles + +import ( + "os" + "os/exec" + "slices" + "strings" + + "github.com/sirupsen/logrus" + "github.com/skevetter/devpod/pkg/agent" + client2 "github.com/skevetter/devpod/pkg/client" + "github.com/skevetter/devpod/pkg/config" + config2 "github.com/skevetter/devpod/pkg/devcontainer/config" + devssh "github.com/skevetter/devpod/pkg/ssh" + "github.com/skevetter/log" +) + +// SetupParams holds all parameters needed for dotfiles setup. +type SetupParams struct { + Source string + Script string + EnvFiles []string + EnvKeyValues []string + Client client2.BaseWorkspaceClient + DevPodConfig *config.Config + Log log.Logger +} + +// Setup clones and installs dotfiles into the devcontainer. +func Setup(p SetupParams) error { + dotfilesRepo := p.DevPodConfig.ContextOption(config.ContextOptionDotfilesURL) + if p.Source != "" { + dotfilesRepo = p.Source + } + + dotfilesScript := p.DevPodConfig.ContextOption(config.ContextOptionDotfilesScript) + if p.Script != "" { + dotfilesScript = p.Script + } + + if dotfilesRepo == "" { + p.Log.Debug("No dotfiles repo specified, skipping") + return nil + } + + p.Log.Infof("Dotfiles Git repository %s specified", dotfilesRepo) + p.Log.Debug("Cloning dotfiles into the devcontainer...") + + dotCmd, err := buildDotCmd(buildDotCmdParams{ + devPodConfig: p.DevPodConfig, + dotfilesRepo: dotfilesRepo, + dotfilesScript: dotfilesScript, + envFiles: p.EnvFiles, + envKeyValuePairs: p.EnvKeyValues, + client: p.Client, + log: p.Log, + }) + if err != nil { + return err + } + if p.Log.GetLevel() == logrus.DebugLevel { + dotCmd.Args = append(dotCmd.Args, "--debug") + } + + p.Log.Debugf("Running dotfiles setup command: %v", dotCmd.Args) + + writer := p.Log.Writer(logrus.InfoLevel, false) + + dotCmd.Stdout = writer + dotCmd.Stderr = writer + + err = dotCmd.Run() + if err != nil { + return err + } + + p.Log.Infof("Done setting up dotfiles into the devcontainer") + + return nil +} + +func buildDotCmdAgentArguments( + dotfilesRepo, dotfilesScript string, + strictHostKey, debug bool, +) []string { + agentArguments := []string{ + "agent", + "workspace", + "install-dotfiles", + "--repository", + dotfilesRepo, + } + + if strictHostKey { + agentArguments = append(agentArguments, "--strict-host-key-checking") + } + + if debug { + agentArguments = append(agentArguments, "--debug") + } + + if dotfilesScript != "" { + agentArguments = append(agentArguments, "--install-script", dotfilesScript) + } + + return agentArguments +} + +type buildDotCmdParams struct { + devPodConfig *config.Config + dotfilesRepo string + dotfilesScript string + envFiles []string + envKeyValuePairs []string + client client2.BaseWorkspaceClient + log log.Logger +} + +func buildDotCmd(p buildDotCmdParams) (*exec.Cmd, error) { + sshCmd := []string{ + "ssh", + "--agent-forwarding=true", + "--start-services=true", + } + + envFilesKeyValuePairs, err := collectDotfilesScriptEnvKeyValuePairs(p.envFiles) + if err != nil { + return nil, err + } + + // Collect file-based and CLI options env variables names (aka keys) and + // configure ssh env var passthrough with send-env + allEnvKeyValuesPairs := slices.Concat(envFilesKeyValuePairs, p.envKeyValuePairs) + allEnvKeys := extractKeysFromEnvKeyValuePairs(allEnvKeyValuesPairs) + for _, envKey := range allEnvKeys { + sshCmd = append(sshCmd, "--send-env", envKey) + } + + remoteUser, err := devssh.GetUser( + p.client.WorkspaceConfig().ID, + p.client.WorkspaceConfig().SSHConfigPath, + p.client.WorkspaceConfig().SSHConfigIncludePath, + ) + if err != nil { + remoteUser = "root" + } + + strictHostKey := p.devPodConfig.ContextOption( + config.ContextOptionSSHStrictHostKeyChecking, + ) == config.BoolTrue + debug := p.log.GetLevel() == logrus.DebugLevel + agentArguments := buildDotCmdAgentArguments( + p.dotfilesRepo, p.dotfilesScript, strictHostKey, debug, + ) + + if p.dotfilesScript != "" { + p.log.Infof("Dotfiles script %s specified", p.dotfilesScript) + } + + sshCmd = append(sshCmd, + "--user", + remoteUser, + "--context", + p.client.Context(), + p.client.Workspace(), + "--log-output=raw", + "--command", + agent.ContainerDevPodHelperLocation+" "+strings.Join(agentArguments, " "), + ) + execPath, err := os.Executable() + if err != nil { + return nil, err + } + + dotCmd := exec.Command( //nolint:gosec + execPath, + sshCmd..., + ) + + dotCmd.Env = append(dotCmd.Environ(), allEnvKeyValuesPairs...) + return dotCmd, nil +} + +func extractKeysFromEnvKeyValuePairs(envKeyValuePairs []string) []string { + keys := []string{} + for _, env := range envKeyValuePairs { + keyValue := strings.SplitN(env, "=", 2) + if len(keyValue) == 2 { + keys = append(keys, keyValue[0]) + } + } + return keys +} + +func collectDotfilesScriptEnvKeyValuePairs(envFiles []string) ([]string, error) { + keyValues := []string{} + for _, file := range envFiles { + envFromFile, err := config2.ParseKeyValueFile(file) + if err != nil { + return nil, err + } + keyValues = append(keyValues, envFromFile...) + } + return keyValues, nil +} diff --git a/pkg/dotfiles/dotfiles_test.go b/pkg/dotfiles/dotfiles_test.go new file mode 100644 index 000000000..d95a2302e --- /dev/null +++ b/pkg/dotfiles/dotfiles_test.go @@ -0,0 +1,121 @@ +package dotfiles + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestExtractKeysFromEnvKeyValuePairs(t *testing.T) { + tests := []struct { + name string + input []string + expected []string + }{ + { + name: "empty input", + input: []string{}, + expected: []string{}, + }, + { + name: "single key-value pair", + input: []string{"FOO=bar"}, + expected: []string{"FOO"}, + }, + { + name: "multiple key-value pairs", + input: []string{"FOO=bar", "BAZ=qux"}, + expected: []string{"FOO", "BAZ"}, + }, + { + name: "value contains equals sign", + input: []string{"FOO=bar=baz"}, + expected: []string{"FOO"}, + }, + { + name: "entry without equals sign is skipped", + input: []string{"NOEQ", "FOO=bar"}, + expected: []string{"FOO"}, + }, + { + name: "empty value", + input: []string{"FOO="}, + expected: []string{"FOO"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := extractKeysFromEnvKeyValuePairs(tt.input) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestCollectDotfilesScriptEnvKeyValuePairs(t *testing.T) { + t.Run("empty file list", func(t *testing.T) { + result, err := collectDotfilesScriptEnvKeyValuePairs([]string{}) + assert.NoError(t, err) + assert.Equal(t, []string{}, result) + }) + + t.Run("nonexistent file returns error", func(t *testing.T) { + _, err := collectDotfilesScriptEnvKeyValuePairs([]string{"/nonexistent/file"}) + assert.Error(t, err) + }) +} + +func TestBuildDotCmdAgentArguments(t *testing.T) { + tests := []struct { + name string + dotfilesRepo string + dotfilesScript string + strictHostKey bool + debug bool + expected []string + }{ + { + name: "basic repo only", + dotfilesRepo: "https://github.com/user/dotfiles", + expected: []string{ + "agent", "workspace", "install-dotfiles", + "--repository", "https://github.com/user/dotfiles", + }, + }, + { + name: "with script", + dotfilesRepo: "https://github.com/user/dotfiles", + dotfilesScript: "install.sh", + expected: []string{ + "agent", "workspace", "install-dotfiles", + "--repository", "https://github.com/user/dotfiles", + "--install-script", "install.sh", + }, + }, + { + name: "all options enabled", + dotfilesRepo: "https://github.com/user/dotfiles", + dotfilesScript: "setup.sh", + strictHostKey: true, + debug: true, + expected: []string{ + "agent", "workspace", "install-dotfiles", + "--repository", "https://github.com/user/dotfiles", + "--strict-host-key-checking", "--debug", + "--install-script", "setup.sh", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := buildDotCmdAgentArguments( + tt.dotfilesRepo, + tt.dotfilesScript, + tt.strictHostKey, + tt.debug, + ) + assert.Equal(t, tt.expected, result) + }) + } +} From fcc5979260aefb03c9a2c616e228bfa510d52648 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Sat, 11 Apr 2026 23:16:29 -0500 Subject: [PATCH 3/8] refactor: extract pkg/ide/opener for IDE launch dispatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move IDE opening logic (VSCode flavors, JetBrains, browser-based IDEs, Fleet) from cmd/up.go into a dedicated pkg/ide/opener package with an exported Open() function and Params struct. Helper functions (performGpgForwarding, startBrowserTunnel, setupBackhaul, createSSHCommand) are temporarily duplicated — they will move to pkg/gpg/ and pkg/tunnel/ in subsequent tasks. --- cmd/up.go | 443 +---------------------- pkg/ide/opener/opener.go | 656 ++++++++++++++++++++++++++++++++++ pkg/ide/opener/opener_test.go | 64 ++++ 3 files changed, 731 insertions(+), 432 deletions(-) create mode 100644 pkg/ide/opener/opener.go create mode 100644 pkg/ide/opener/opener_test.go diff --git a/cmd/up.go b/cmd/up.go index edadcab38..da02286ed 100644 --- a/cmd/up.go +++ b/cmd/up.go @@ -1,11 +1,9 @@ package cmd import ( - "bytes" "context" "fmt" "io" - "net" "os" "os/exec" "os/signal" @@ -21,23 +19,14 @@ import ( "github.com/skevetter/devpod/pkg/agent/tunnelserver" client2 "github.com/skevetter/devpod/pkg/client" "github.com/skevetter/devpod/pkg/client/clientimplementation" - "github.com/skevetter/devpod/pkg/command" "github.com/skevetter/devpod/pkg/config" config2 "github.com/skevetter/devpod/pkg/devcontainer/config" "github.com/skevetter/devpod/pkg/devcontainer/sshtunnel" "github.com/skevetter/devpod/pkg/dotfiles" "github.com/skevetter/devpod/pkg/ide" - "github.com/skevetter/devpod/pkg/ide/fleet" - "github.com/skevetter/devpod/pkg/ide/jetbrains" - "github.com/skevetter/devpod/pkg/ide/jupyter" - "github.com/skevetter/devpod/pkg/ide/openvscode" - "github.com/skevetter/devpod/pkg/ide/rstudio" - "github.com/skevetter/devpod/pkg/ide/vscode" - "github.com/skevetter/devpod/pkg/ide/zed" - open2 "github.com/skevetter/devpod/pkg/open" + "github.com/skevetter/devpod/pkg/ide/opener" options2 "github.com/skevetter/devpod/pkg/options" "github.com/skevetter/devpod/pkg/platform" - "github.com/skevetter/devpod/pkg/port" provider2 "github.com/skevetter/devpod/pkg/provider" devssh "github.com/skevetter/devpod/pkg/ssh" "github.com/skevetter/devpod/pkg/telemetry" @@ -46,7 +35,6 @@ import ( "github.com/skevetter/devpod/pkg/version" workspace2 "github.com/skevetter/devpod/pkg/workspace" "github.com/skevetter/log" - "github.com/skratchdot/open-golang/open" "github.com/spf13/cobra" "golang.org/x/crypto/ssh" ) @@ -410,176 +398,16 @@ func (cmd *UpCmd) openIDE( } ideConfig := client.WorkspaceConfig().IDE - opener := newIDEOpener(cmd, devPodConfig, client, wctx, log) - return opener.open(ctx, ideConfig.Name, ideConfig.Options) -} - -// ideOpener handles opening different IDE types. -type ideOpener struct { - cmd *UpCmd - devPodConfig *config.Config - client client2.BaseWorkspaceClient - wctx *workspaceContext - log log.Logger -} - -func newIDEOpener( - cmd *UpCmd, - devPodConfig *config.Config, - client client2.BaseWorkspaceClient, - wctx *workspaceContext, - log log.Logger, -) *ideOpener { - return &ideOpener{ - cmd: cmd, - devPodConfig: devPodConfig, - client: client, - wctx: wctx, - log: log, - } -} - -func (o *ideOpener) open( - ctx context.Context, - ideName string, - ideOptions map[string]config.OptionValue, -) error { - folder := o.wctx.result.SubstitutionContext.ContainerWorkspaceFolder - workspace := o.client.Workspace() - user := o.wctx.user - - switch ideName { - case string(config.IDEVSCode), string(config.IDEVSCodeInsiders), string(config.IDECursor), - string(config.IDECodium), string(config.IDEPositron), string(config.IDEWindsurf), - string(config.IDEAntigravity), string(config.IDEBob): - return o.openVSCodeFlavor(ctx, ideName, folder, ideOptions) - - case string(config.IDERustRover), string(config.IDEGoland), string(config.IDEPyCharm), - string(config.IDEPhpStorm), string(config.IDEIntellij), string(config.IDECLion), - string(config.IDERider), string(config.IDERubyMine), string(config.IDEWebStorm), string(config.IDEDataSpell): - return o.openJetBrains(ideName, folder, workspace, user, ideOptions) - - case string(config.IDEOpenVSCode): - return startVSCodeInBrowser( - o.cmd.GPGAgentForwarding, - ctx, - o.devPodConfig, - o.client, - folder, - user, - ideOptions, - o.cmd.SSHAuthSockID, - o.cmd.GitSSHSigningKey, - o.log, - ) - - case string(config.IDEFleet): - return startFleet(ctx, o.client, o.log) - - case string(config.IDEZed): - return zed.Open(ctx, ideOptions, user, folder, workspace, o.log) - - case string(config.IDEJupyterNotebook): - return startJupyterNotebookInBrowser( - o.cmd.GPGAgentForwarding, - ctx, - o.devPodConfig, - o.client, - user, - ideOptions, - o.cmd.SSHAuthSockID, - o.cmd.GitSSHSigningKey, - o.log, - ) - - case string(config.IDERStudio): - return startRStudioInBrowser( - o.cmd.GPGAgentForwarding, - ctx, - o.devPodConfig, - o.client, - user, - ideOptions, - o.cmd.SSHAuthSockID, - o.cmd.GitSSHSigningKey, - o.log, - ) - - default: - return nil - } -} - -func (o *ideOpener) openVSCodeFlavor( - ctx context.Context, - ideName, folder string, - ideOptions map[string]config.OptionValue, -) error { - flavorMap := map[string]vscode.Flavor{ - string(config.IDEVSCode): vscode.FlavorStable, - string(config.IDEVSCodeInsiders): vscode.FlavorInsiders, - string(config.IDECursor): vscode.FlavorCursor, - string(config.IDECodium): vscode.FlavorCodium, - string(config.IDEPositron): vscode.FlavorPositron, - string(config.IDEWindsurf): vscode.FlavorWindsurf, - string(config.IDEAntigravity): vscode.FlavorAntigravity, - string(config.IDEBob): vscode.FlavorBob, - } - - params := vscode.OpenParams{ - Workspace: o.client.Workspace(), - Folder: folder, - NewWindow: vscode.Options.GetValue(ideOptions, vscode.OpenNewWindow) == config.BoolTrue, - Flavor: flavorMap[ideName], - Log: o.log, - } - - return vscode.Open(ctx, params) -} - -func (o *ideOpener) openJetBrains( - ideName, folder, workspace, user string, - ideOptions map[string]config.OptionValue, -) error { - type jetbrainsFactory func() interface{ OpenGateway(string, string) error } - - jetbrainsMap := map[string]jetbrainsFactory{ - string(config.IDERustRover): func() interface{ OpenGateway(string, string) error } { - return jetbrains.NewRustRoverServer(user, ideOptions, o.log) - }, - string(config.IDEGoland): func() interface{ OpenGateway(string, string) error } { - return jetbrains.NewGolandServer(user, ideOptions, o.log) - }, - string(config.IDEPyCharm): func() interface{ OpenGateway(string, string) error } { - return jetbrains.NewPyCharmServer(user, ideOptions, o.log) - }, - string(config.IDEPhpStorm): func() interface{ OpenGateway(string, string) error } { - return jetbrains.NewPhpStorm(user, ideOptions, o.log) - }, - string(config.IDEIntellij): func() interface{ OpenGateway(string, string) error } { - return jetbrains.NewIntellij(user, ideOptions, o.log) - }, - string(config.IDECLion): func() interface{ OpenGateway(string, string) error } { - return jetbrains.NewCLionServer(user, ideOptions, o.log) - }, - string(config.IDERider): func() interface{ OpenGateway(string, string) error } { - return jetbrains.NewRiderServer(user, ideOptions, o.log) - }, - string(config.IDERubyMine): func() interface{ OpenGateway(string, string) error } { - return jetbrains.NewRubyMineServer(user, ideOptions, o.log) - }, - string(config.IDEWebStorm): func() interface{ OpenGateway(string, string) error } { - return jetbrains.NewWebStormServer(user, ideOptions, o.log) - }, - string(config.IDEDataSpell): func() interface{ OpenGateway(string, string) error } { - return jetbrains.NewDataSpellServer(user, ideOptions, o.log) - }, - } - - if factory, ok := jetbrainsMap[ideName]; ok { - return factory().OpenGateway(folder, workspace) - } - return fmt.Errorf("unknown JetBrains IDE: %s", ideName) + return opener.Open(ctx, ideConfig.Name, ideConfig.Options, opener.Params{ + GPGAgentForwarding: cmd.GPGAgentForwarding, + SSHAuthSockID: cmd.SSHAuthSockID, + GitSSHSigningKey: cmd.GitSSHSigningKey, + DevPodConfig: devPodConfig, + Client: client, + User: wctx.user, + Result: wctx.result, + Log: log, + }) } func (cmd *UpCmd) devPodUp( @@ -837,255 +665,6 @@ func (cmd *UpCmd) devPodUpMachine( }) } -func startJupyterNotebookInBrowser( - forwardGpg bool, - ctx context.Context, - devPodConfig *config.Config, - client client2.BaseWorkspaceClient, - user string, - ideOptions map[string]config.OptionValue, - authSockID string, - gitSSHSigningKey string, - logger log.Logger, -) error { - if forwardGpg { - err := performGpgForwarding(client, logger) - if err != nil { - return err - } - } - - // determine port - jupyterAddress, jupyterPort, err := parseAddressAndPort( - jupyter.Options.GetValue(ideOptions, jupyter.BindAddressOption), - jupyter.DefaultServerPort, - ) - if err != nil { - return err - } - - // wait until reachable then open browser - targetURL := fmt.Sprintf("http://localhost:%d/lab", jupyterPort) - if jupyter.Options.GetValue(ideOptions, jupyter.OpenOption) == config.BoolTrue { - go func() { - err = open2.Open(ctx, targetURL, logger) - if err != nil { - logger.WithFields(logrus.Fields{"error": err}). - Error("error opening jupyter notebook") - } - - logger.Info( - "started jupyter notebook in browser mode. Please keep this terminal open as long as you use Jupyter Notebook", - ) - }() - } - - // start in browser - logger.Infof("Starting jupyter notebook in browser mode at %s", targetURL) - extraPorts := []string{fmt.Sprintf("%s:%d", jupyterAddress, jupyter.DefaultServerPort)} - return startBrowserTunnel( - ctx, - devPodConfig, - client, - user, - targetURL, - false, - extraPorts, - authSockID, - gitSSHSigningKey, - logger, - ) -} - -func startRStudioInBrowser( - forwardGpg bool, - ctx context.Context, - devPodConfig *config.Config, - client client2.BaseWorkspaceClient, - user string, - ideOptions map[string]config.OptionValue, - authSockID string, - gitSSHSigningKey string, - logger log.Logger, -) error { - if forwardGpg { - err := performGpgForwarding(client, logger) - if err != nil { - return err - } - } - - // determine port - addr, port, err := parseAddressAndPort( - rstudio.Options.GetValue(ideOptions, rstudio.BindAddressOption), - rstudio.DefaultServerPort, - ) - if err != nil { - return err - } - - // wait until reachable then open browser - targetURL := fmt.Sprintf("http://localhost:%d", port) - if rstudio.Options.GetValue(ideOptions, rstudio.OpenOption) == config.BoolTrue { - go func() { - err = open2.Open(ctx, targetURL, logger) - if err != nil { - logger.Errorf("error opening rstudio: %v", err) - } - - logger.Infof( - "started RStudio Server in browser mode. Please keep this terminal open as long as you use it", - ) - }() - } - - // start in browser - logger.Infof("Starting RStudio server in browser mode at %s", targetURL) - extraPorts := []string{fmt.Sprintf("%s:%d", addr, rstudio.DefaultServerPort)} - return startBrowserTunnel( - ctx, - devPodConfig, - client, - user, - targetURL, - false, - extraPorts, - authSockID, - gitSSHSigningKey, - logger, - ) -} - -func startFleet(ctx context.Context, client client2.BaseWorkspaceClient, logger log.Logger) error { - // create ssh command - stdout := &bytes.Buffer{} - cmd, err := createSSHCommand( - ctx, - client, - logger, - []string{"--command", "cat " + fleet.FleetURLFileName}, - ) - if err != nil { - return err - } - cmd.Stdout = stdout - err = cmd.Run() - if err != nil { - return command.WrapCommandError(stdout.Bytes(), err) - } - - url := strings.TrimSpace(stdout.String()) - if len(url) == 0 { - return fmt.Errorf("seems like fleet is not running within the container") - } - - logger.Warnf( - "Fleet is exposed at a publicly reachable URL, please make sure to not disclose this URL " + - "to anyone as they will be able to reach your workspace from that", - ) - logger.Infof("Starting Fleet at %s ...", url) - err = open.Run(url) - if err != nil { - return err - } - - return nil -} - -func startVSCodeInBrowser( - forwardGpg bool, - ctx context.Context, - devPodConfig *config.Config, - client client2.BaseWorkspaceClient, - workspaceFolder, user string, - ideOptions map[string]config.OptionValue, - authSockID string, - gitSSHSigningKey string, - logger log.Logger, -) error { - if forwardGpg { - err := performGpgForwarding(client, logger) - if err != nil { - return err - } - } - - // determine port - vscodeAddress, vscodePort, err := parseAddressAndPort( - openvscode.Options.GetValue(ideOptions, openvscode.BindAddressOption), - openvscode.DefaultVSCodePort, - ) - if err != nil { - return err - } - - // wait until reachable then open browser - targetURL := fmt.Sprintf("http://localhost:%d/?folder=%s", vscodePort, workspaceFolder) - if openvscode.Options.GetValue(ideOptions, openvscode.OpenOption) == config.BoolTrue { - go func() { - err = open2.Open(ctx, targetURL, logger) - if err != nil { - logger.Errorf("error opening vscode: %v", err) - } - - logger.Infof( - "started vscode in browser mode. Please keep this terminal open as long as you use VSCode browser version", - ) - }() - } - - // start in browser - logger.Infof("Starting vscode in browser mode at %s", targetURL) - forwardPorts := openvscode.Options.GetValue( - ideOptions, - openvscode.ForwardPortsOption, - ) == config.BoolTrue - extraPorts := []string{fmt.Sprintf("%s:%d", vscodeAddress, openvscode.DefaultVSCodePort)} - return startBrowserTunnel( - ctx, - devPodConfig, - client, - user, - targetURL, - forwardPorts, - extraPorts, - authSockID, - gitSSHSigningKey, - logger, - ) -} - -func parseAddressAndPort(bindAddressOption string, defaultPort int) (string, int, error) { - var ( - err error - address string - portName int - ) - if bindAddressOption == "" { - portName, err = port.FindAvailablePort(defaultPort) - if err != nil { - return "", 0, err - } - - address = fmt.Sprintf("%d", portName) - } else { - address = bindAddressOption - _, port, err := net.SplitHostPort(address) - if err != nil { - return "", 0, fmt.Errorf("parse host:port: %w", err) - } else if port == "" { - return "", 0, fmt.Errorf("parse ADDRESS: expected host:port, got %s", address) - } - - portName, err = strconv.Atoi(port) - if err != nil { - return "", 0, fmt.Errorf("parse host:port: %w", err) - } - } - - return address, portName, nil -} - // setupBackhaul sets up a long running command in the container to ensure an SSH connection is kept alive. func setupBackhaul(client client2.BaseWorkspaceClient, authSockId string, log log.Logger) error { execPath, err := os.Executable() diff --git a/pkg/ide/opener/opener.go b/pkg/ide/opener/opener.go new file mode 100644 index 000000000..8183ea196 --- /dev/null +++ b/pkg/ide/opener/opener.go @@ -0,0 +1,656 @@ +package opener + +import ( + "bytes" + "context" + "fmt" + "io" + "net" + "os" + "os/exec" + "strconv" + "strings" + + "github.com/sirupsen/logrus" + client2 "github.com/skevetter/devpod/pkg/client" + "github.com/skevetter/devpod/pkg/client/clientimplementation" + "github.com/skevetter/devpod/pkg/command" + "github.com/skevetter/devpod/pkg/config" + config2 "github.com/skevetter/devpod/pkg/devcontainer/config" + "github.com/skevetter/devpod/pkg/ide/fleet" + "github.com/skevetter/devpod/pkg/ide/jetbrains" + "github.com/skevetter/devpod/pkg/ide/jupyter" + "github.com/skevetter/devpod/pkg/ide/openvscode" + "github.com/skevetter/devpod/pkg/ide/rstudio" + "github.com/skevetter/devpod/pkg/ide/vscode" + "github.com/skevetter/devpod/pkg/ide/zed" + open2 "github.com/skevetter/devpod/pkg/open" + "github.com/skevetter/devpod/pkg/port" + devssh "github.com/skevetter/devpod/pkg/ssh" + "github.com/skevetter/devpod/pkg/tunnel" + "github.com/skevetter/log" + "github.com/skratchdot/open-golang/open" + "golang.org/x/crypto/ssh" +) + +// Params holds the parameters needed to open an IDE. +type Params struct { + GPGAgentForwarding bool + SSHAuthSockID string + GitSSHSigningKey string + DevPodConfig *config.Config + Client client2.BaseWorkspaceClient + User string + Result *config2.Result + Log log.Logger +} + +// Open dispatches to the correct IDE opener based on ideName. +func Open( + ctx context.Context, + ideName string, + ideOptions map[string]config.OptionValue, + params Params, +) error { + if fn, ok := browserIDEOpener(ideName); ok { + return fn(ctx, ideOptions, params) + } + + return openDesktopIDE(ctx, ideName, ideOptions, params) +} + +// browserIDEOpener returns a handler for browser-based IDEs if ideName matches. +func browserIDEOpener( + ideName string, +) (func(context.Context, map[string]config.OptionValue, Params) error, bool) { + switch ideName { + case string(config.IDEOpenVSCode): + return openVSCodeBrowser, true + case string(config.IDEJupyterNotebook): + return openJupyterBrowser, true + case string(config.IDERStudio): + return openRStudioBrowser, true + default: + return nil, false + } +} + +func openDesktopIDE( + ctx context.Context, + ideName string, + ideOptions map[string]config.OptionValue, + params Params, +) error { + switch ideName { + case string(config.IDEVSCode), string(config.IDEVSCodeInsiders), string(config.IDECursor), + string(config.IDECodium), string(config.IDEPositron), string(config.IDEWindsurf), + string(config.IDEAntigravity), string(config.IDEBob): + return openVSCodeFlavor(ctx, ideName, ideOptions, params) + + case string(config.IDERustRover), string(config.IDEGoland), string(config.IDEPyCharm), + string(config.IDEPhpStorm), string(config.IDEIntellij), string(config.IDECLion), + string(config.IDERider), string(config.IDERubyMine), string(config.IDEWebStorm), + string(config.IDEDataSpell): + return openJetBrains(ideName, ideOptions, params) + + case string(config.IDEFleet): + return startFleet(ctx, params.Client, params.Log) + + case string(config.IDEZed): + return zed.Open( + ctx, ideOptions, params.User, + params.Result.SubstitutionContext.ContainerWorkspaceFolder, + params.Client.Workspace(), params.Log, + ) + + default: + return nil + } +} + +// ParseAddressAndPort parses a bind address option into host address and port. +// If bindAddressOption is empty, it finds an available port starting from defaultPort. +func ParseAddressAndPort(bindAddressOption string, defaultPort int) (string, int, error) { + if bindAddressOption == "" { + return parseDefaultPort(defaultPort) + } + + return parseExplicitAddress(bindAddressOption) +} + +func parseDefaultPort(defaultPort int) (string, int, error) { + portName, err := port.FindAvailablePort(defaultPort) + if err != nil { + return "", 0, err + } + + return fmt.Sprintf("%d", portName), portName, nil +} + +func parseExplicitAddress(address string) (string, int, error) { + _, p, err := net.SplitHostPort(address) + if err != nil { + return "", 0, fmt.Errorf("parse host:port: %w", err) + } + if p == "" { + return "", 0, fmt.Errorf("parse ADDRESS: expected host:port, got %s", address) + } + + portName, err := strconv.Atoi(p) + if err != nil { + return "", 0, fmt.Errorf("parse host:port: %w", err) + } + + return address, portName, nil +} + +var vsCodeFlavorMap = map[string]vscode.Flavor{ + string(config.IDEVSCode): vscode.FlavorStable, + string(config.IDEVSCodeInsiders): vscode.FlavorInsiders, + string(config.IDECursor): vscode.FlavorCursor, + string(config.IDECodium): vscode.FlavorCodium, + string(config.IDEPositron): vscode.FlavorPositron, + string(config.IDEWindsurf): vscode.FlavorWindsurf, + string(config.IDEAntigravity): vscode.FlavorAntigravity, + string(config.IDEBob): vscode.FlavorBob, +} + +func openVSCodeFlavor( + ctx context.Context, + ideName string, + ideOptions map[string]config.OptionValue, + params Params, +) error { + return vscode.Open(ctx, vscode.OpenParams{ + Workspace: params.Client.Workspace(), + Folder: params.Result.SubstitutionContext.ContainerWorkspaceFolder, + NewWindow: vscode.Options.GetValue(ideOptions, vscode.OpenNewWindow) == config.BoolTrue, + Flavor: vsCodeFlavorMap[ideName], + Log: params.Log, + }) +} + +func openJetBrains( + ideName string, + ideOptions map[string]config.OptionValue, + params Params, +) error { + folder := params.Result.SubstitutionContext.ContainerWorkspaceFolder + workspace := params.Client.Workspace() + user := params.User + logger := params.Log + type jetbrainsFactory func() interface{ OpenGateway(string, string) error } + + jetbrainsMap := map[string]jetbrainsFactory{ + string(config.IDERustRover): func() interface{ OpenGateway(string, string) error } { + return jetbrains.NewRustRoverServer(user, ideOptions, logger) + }, + string(config.IDEGoland): func() interface{ OpenGateway(string, string) error } { + return jetbrains.NewGolandServer(user, ideOptions, logger) + }, + string(config.IDEPyCharm): func() interface{ OpenGateway(string, string) error } { + return jetbrains.NewPyCharmServer(user, ideOptions, logger) + }, + string(config.IDEPhpStorm): func() interface{ OpenGateway(string, string) error } { + return jetbrains.NewPhpStorm(user, ideOptions, logger) + }, + string(config.IDEIntellij): func() interface{ OpenGateway(string, string) error } { + return jetbrains.NewIntellij(user, ideOptions, logger) + }, + string(config.IDECLion): func() interface{ OpenGateway(string, string) error } { + return jetbrains.NewCLionServer(user, ideOptions, logger) + }, + string(config.IDERider): func() interface{ OpenGateway(string, string) error } { + return jetbrains.NewRiderServer(user, ideOptions, logger) + }, + string(config.IDERubyMine): func() interface{ OpenGateway(string, string) error } { + return jetbrains.NewRubyMineServer(user, ideOptions, logger) + }, + string(config.IDEWebStorm): func() interface{ OpenGateway(string, string) error } { + return jetbrains.NewWebStormServer(user, ideOptions, logger) + }, + string(config.IDEDataSpell): func() interface{ OpenGateway(string, string) error } { + return jetbrains.NewDataSpellServer(user, ideOptions, logger) + }, + } + + if factory, ok := jetbrainsMap[ideName]; ok { + return factory().OpenGateway(folder, workspace) + } + return fmt.Errorf("unknown JetBrains IDE: %s", ideName) +} + +// browserTunnelParams bundles the arguments for browser-based IDE tunnels. +type browserTunnelParams struct { + ctx context.Context + devPodConfig *config.Config + client client2.BaseWorkspaceClient + user string + targetURL string + forwardPorts bool + extraPorts []string + authSockID string + gitSSHSigningKey string + logger log.Logger +} + +func openJupyterBrowser( + ctx context.Context, + ideOptions map[string]config.OptionValue, + params Params, +) error { + if params.GPGAgentForwarding { + if err := performGpgForwarding(params.Client, params.Log); err != nil { + return err + } + } + + addr, jupyterPort, err := ParseAddressAndPort( + jupyter.Options.GetValue(ideOptions, jupyter.BindAddressOption), + jupyter.DefaultServerPort, + ) + if err != nil { + return err + } + + targetURL := fmt.Sprintf("http://localhost:%d/lab", jupyterPort) + if jupyter.Options.GetValue(ideOptions, jupyter.OpenOption) == config.BoolTrue { + go func() { + if openErr := open2.Open(ctx, targetURL, params.Log); openErr != nil { + params.Log.WithFields(logrus.Fields{"error": openErr}). + Error("error opening jupyter notebook") + } + + params.Log.Info( + "started jupyter notebook in browser mode. " + + "Please keep this terminal open as long as you use Jupyter Notebook", + ) + }() + } + + params.Log.Infof("Starting jupyter notebook in browser mode at %s", targetURL) + return startBrowserTunnel(browserTunnelParams{ + ctx: ctx, + devPodConfig: params.DevPodConfig, + client: params.Client, + user: params.User, + targetURL: targetURL, + extraPorts: []string{fmt.Sprintf("%s:%d", addr, jupyter.DefaultServerPort)}, + authSockID: params.SSHAuthSockID, + gitSSHSigningKey: params.GitSSHSigningKey, + logger: params.Log, + }) +} + +func openRStudioBrowser( + ctx context.Context, + ideOptions map[string]config.OptionValue, + params Params, +) error { + if params.GPGAgentForwarding { + if err := performGpgForwarding(params.Client, params.Log); err != nil { + return err + } + } + + addr, rsPort, err := ParseAddressAndPort( + rstudio.Options.GetValue(ideOptions, rstudio.BindAddressOption), + rstudio.DefaultServerPort, + ) + if err != nil { + return err + } + + targetURL := fmt.Sprintf("http://localhost:%d", rsPort) + if rstudio.Options.GetValue(ideOptions, rstudio.OpenOption) == config.BoolTrue { + go func() { + if openErr := open2.Open(ctx, targetURL, params.Log); openErr != nil { + params.Log.Errorf("error opening rstudio: %v", openErr) + } + + params.Log.Infof( + "started RStudio Server in browser mode. Please keep this terminal open as long as you use it", + ) + }() + } + + params.Log.Infof("Starting RStudio server in browser mode at %s", targetURL) + return startBrowserTunnel(browserTunnelParams{ + ctx: ctx, + devPodConfig: params.DevPodConfig, + client: params.Client, + user: params.User, + targetURL: targetURL, + extraPorts: []string{fmt.Sprintf("%s:%d", addr, rstudio.DefaultServerPort)}, + authSockID: params.SSHAuthSockID, + gitSSHSigningKey: params.GitSSHSigningKey, + logger: params.Log, + }) +} + +func openVSCodeBrowser( + ctx context.Context, + ideOptions map[string]config.OptionValue, + params Params, +) error { + if params.GPGAgentForwarding { + if err := performGpgForwarding(params.Client, params.Log); err != nil { + return err + } + } + + folder := params.Result.SubstitutionContext.ContainerWorkspaceFolder + addr, vscodePort, err := ParseAddressAndPort( + openvscode.Options.GetValue(ideOptions, openvscode.BindAddressOption), + openvscode.DefaultVSCodePort, + ) + if err != nil { + return err + } + + targetURL := fmt.Sprintf("http://localhost:%d/?folder=%s", vscodePort, folder) + if openvscode.Options.GetValue(ideOptions, openvscode.OpenOption) == config.BoolTrue { + go func() { + if openErr := open2.Open(ctx, targetURL, params.Log); openErr != nil { + params.Log.Errorf("error opening vscode: %v", openErr) + } + + params.Log.Infof( + "started vscode in browser mode. " + + "Please keep this terminal open as long as you use VSCode browser version", + ) + }() + } + + params.Log.Infof("Starting vscode in browser mode at %s", targetURL) + forwardPorts := openvscode.Options.GetValue( + ideOptions, + openvscode.ForwardPortsOption, + ) == config.BoolTrue + return startBrowserTunnel(browserTunnelParams{ + ctx: ctx, + devPodConfig: params.DevPodConfig, + client: params.Client, + user: params.User, + targetURL: targetURL, + forwardPorts: forwardPorts, + extraPorts: []string{fmt.Sprintf("%s:%d", addr, openvscode.DefaultVSCodePort)}, + authSockID: params.SSHAuthSockID, + gitSSHSigningKey: params.GitSSHSigningKey, + logger: params.Log, + }) +} + +func startFleet(ctx context.Context, client client2.BaseWorkspaceClient, logger log.Logger) error { + stdout := &bytes.Buffer{} + sshCmd, err := createSSHCommand( + ctx, + client, + logger, + []string{"--command", "cat " + fleet.FleetURLFileName}, + ) + if err != nil { + return err + } + sshCmd.Stdout = stdout + err = sshCmd.Run() + if err != nil { + return command.WrapCommandError(stdout.Bytes(), err) + } + + url := strings.TrimSpace(stdout.String()) + if len(url) == 0 { + return fmt.Errorf("seems like fleet is not running within the container") + } + + logger.Warnf( + "Fleet is exposed at a publicly reachable URL, please make sure to not disclose this URL " + + "to anyone as they will be able to reach your workspace from that", + ) + logger.Infof("Starting Fleet at %s ...", url) + + return open.Run(url) +} + +// Duplicated helpers below — these will be moved to pkg/gpg/ and pkg/tunnel/ in Tasks 3-4. + +func performGpgForwarding( + client client2.BaseWorkspaceClient, + logger log.Logger, +) error { + logger.Debug("gpg forwarding enabled, performing immediately") + + execPath, err := os.Executable() + if err != nil { + return err + } + + remoteUser, err := devssh.GetUser( + client.WorkspaceConfig().ID, + client.WorkspaceConfig().SSHConfigPath, + client.WorkspaceConfig().SSHConfigIncludePath, + ) + if err != nil { + remoteUser = "root" + } + + logger.Info("forwarding gpg-agent") + + go func() { + //nolint:gosec // execPath is the current binary, arguments are controlled + err = exec.Command( + execPath, + "ssh", + "--gpg-agent-forwarding=true", + "--agent-forwarding=true", + "--start-services=true", + "--user", + remoteUser, + "--context", + client.Context(), + client.Workspace(), + "--log-output=raw", + "--command", "sleep infinity", + ).Run() + if err != nil { + logger.Error("failure in forwarding gpg-agent") + } + }() + + return nil +} + +func setupBackhaul(client client2.BaseWorkspaceClient, authSockID string, logger log.Logger) error { + execPath, err := os.Executable() + if err != nil { + return err + } + + remoteUser, err := devssh.GetUser( + client.WorkspaceConfig().ID, + client.WorkspaceConfig().SSHConfigPath, + client.WorkspaceConfig().SSHConfigIncludePath, + ) + if err != nil { + remoteUser = "root" + } + + //nolint:gosec // execPath is the current binary, arguments are controlled + dotCmd := exec.Command( + execPath, + "ssh", + "--agent-forwarding=true", + fmt.Sprintf("--reuse-ssh-auth-sock=%s", authSockID), + "--start-services=false", + "--user", + remoteUser, + "--context", + client.Context(), + client.Workspace(), + "--log-output=raw", + "--command", + "while true; do sleep 6000000; done", + ) + + if logger.GetLevel() == logrus.DebugLevel { + dotCmd.Args = append(dotCmd.Args, "--debug") + } + + logger.Info("Setting up backhaul SSH connection") + + writer := logger.Writer(logrus.InfoLevel, false) + + dotCmd.Stdout = writer + dotCmd.Stderr = writer + + err = dotCmd.Run() + if err != nil { + return err + } + + logger.Infof("Done setting up backhaul") + + return nil +} + +func createSSHCommand( + ctx context.Context, + client client2.BaseWorkspaceClient, + logger log.Logger, + extraArgs []string, +) (*exec.Cmd, error) { + execPath, err := os.Executable() + if err != nil { + return nil, err + } + + args := []string{ + "ssh", + "--user=root", + "--agent-forwarding=false", + "--start-services=false", + "--context", + client.Context(), + client.Workspace(), + } + if logger.GetLevel() == logrus.DebugLevel { + args = append(args, "--debug") + } + args = append(args, extraArgs...) + + //nolint:gosec // execPath is the current binary, arguments are controlled + return exec.CommandContext(ctx, execPath, args...), nil +} + +func startBrowserTunnel(p browserTunnelParams) error { + if p.authSockID != "" { + go func() { + if err := setupBackhaul(p.client, p.authSockID, p.logger); err != nil { + p.logger.Error("Failed to setup backhaul SSH connection: ", err) + } + }() + } + + daemonClient, ok := p.client.(client2.DaemonClient) + if ok { + return startBrowserTunnelDaemon(p, daemonClient) + } + + return startBrowserTunnelSSH(p) +} + +func startBrowserTunnelDaemon(p browserTunnelParams, daemonClient client2.DaemonClient) error { + toolClient, _, err := daemonClient.SSHClients(p.ctx, p.user) + if err != nil { + return err + } + defer func() { _ = toolClient.Close() }() + + err = clientimplementation.StartServicesDaemon( + p.ctx, + clientimplementation.StartServicesDaemonOptions{ + DevPodConfig: p.devPodConfig, + Client: daemonClient, + SSHClient: toolClient, + User: p.user, + Log: p.logger, + ForwardPorts: p.forwardPorts, + ExtraPorts: p.extraPorts, + }, + ) + if err != nil { + return err + } + <-p.ctx.Done() + + return nil +} + +func startBrowserTunnelSSH(p browserTunnelParams) error { + return tunnel.NewTunnel( + p.ctx, + func(ctx context.Context, stdin io.Reader, stdout io.Writer) error { + writer := p.logger.Writer(logrus.DebugLevel, false) + defer func() { _ = writer.Close() }() + + sshCmd, err := createSSHCommand(ctx, p.client, p.logger, []string{ + "--log-output=raw", + fmt.Sprintf("--reuse-ssh-auth-sock=%s", p.authSockID), + "--stdio", + }) + if err != nil { + return err + } + sshCmd.Stdout = stdout + sshCmd.Stdin = stdin + sshCmd.Stderr = writer + return sshCmd.Run() + }, + func(ctx context.Context, containerClient *ssh.Client) error { + return runBrowserTunnelServices(ctx, p, containerClient) + }, + ) +} + +func runBrowserTunnelServices( + ctx context.Context, + p browserTunnelParams, + containerClient *ssh.Client, +) error { + streamLogger, ok := p.logger.(*log.StreamLogger) + if ok { + streamLogger.JSON(logrus.InfoLevel, map[string]string{ + "url": p.targetURL, + "done": "true", + }) + } + + err := tunnel.RunServices( + ctx, + tunnel.RunServicesOptions{ + DevPodConfig: p.devPodConfig, + ContainerClient: containerClient, + User: p.user, + ForwardPorts: p.forwardPorts, + ExtraPorts: p.extraPorts, + Workspace: p.client.WorkspaceConfig(), + ConfigureDockerCredentials: p.devPodConfig.ContextOption( + config.ContextOptionSSHInjectDockerCredentials, + ) == config.BoolTrue, + ConfigureGitCredentials: p.devPodConfig.ContextOption( + config.ContextOptionSSHInjectGitCredentials, + ) == config.BoolTrue, + ConfigureGitSSHSignatureHelper: p.devPodConfig.ContextOption( + config.ContextOptionGitSSHSignatureForwarding, + ) == config.BoolTrue, + GitSSHSigningKey: p.gitSSHSigningKey, + Log: p.logger, + }, + ) + if err != nil { + return fmt.Errorf("run credentials server in browser tunnel: %w", err) + } + + <-ctx.Done() + return nil +} diff --git a/pkg/ide/opener/opener_test.go b/pkg/ide/opener/opener_test.go new file mode 100644 index 000000000..405156e7c --- /dev/null +++ b/pkg/ide/opener/opener_test.go @@ -0,0 +1,64 @@ +package opener + +import ( + "testing" +) + +func TestParseAddressAndPort_Empty(t *testing.T) { + addr, p, err := ParseAddressAndPort("", 10000) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if p < 10000 { + t.Errorf("expected port >= 10000, got %d", p) + } + if addr == "" { + t.Error("expected non-empty address") + } +} + +func TestParseAddressAndPort_Explicit(t *testing.T) { + tests := []struct { + name string + input string + wantAddr string + wantPort int + }{ + {"host:port", "127.0.0.1:8080", "127.0.0.1:8080", 8080}, + {"localhost:port", "localhost:3000", "localhost:3000", 3000}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + addr, p, err := ParseAddressAndPort(tt.input, 10000) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if addr != tt.wantAddr { + t.Errorf("addr = %q, want %q", addr, tt.wantAddr) + } + if p != tt.wantPort { + t.Errorf("port = %d, want %d", p, tt.wantPort) + } + }) + } +} + +func TestParseAddressAndPort_Errors(t *testing.T) { + tests := []struct { + name string + input string + }{ + {"missing port", "127.0.0.1"}, + {"invalid format", "not:a:valid:address"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, _, err := ParseAddressAndPort(tt.input, 10000) + if err == nil { + t.Error("expected error, got nil") + } + }) + } +} From acb7b5e488b4b9ccd9e64f06d4f926c417646df3 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Sat, 11 Apr 2026 23:21:10 -0500 Subject: [PATCH 4/8] refactor: extract pkg/gpg for GPG agent forwarding --- cmd/up.go | 47 -------------------------------- pkg/gpg/forward.go | 59 ++++++++++++++++++++++++++++++++++++++++ pkg/gpg/forward_test.go | 21 ++++++++++++++ pkg/ide/opener/opener.go | 55 +++---------------------------------- 4 files changed, 84 insertions(+), 98 deletions(-) create mode 100644 pkg/gpg/forward.go create mode 100644 pkg/gpg/forward_test.go diff --git a/cmd/up.go b/cmd/up.go index da02286ed..3ca97ad0c 100644 --- a/cmd/up.go +++ b/cmd/up.go @@ -961,53 +961,6 @@ func createSSHCommand( return exec.CommandContext(ctx, execPath, args...), nil } -func performGpgForwarding( - client client2.BaseWorkspaceClient, - log log.Logger, -) error { - log.Debug("gpg forwarding enabled, performing immediately") - - execPath, err := os.Executable() - if err != nil { - return err - } - - remoteUser, err := devssh.GetUser( - client.WorkspaceConfig().ID, - client.WorkspaceConfig().SSHConfigPath, - client.WorkspaceConfig().SSHConfigIncludePath, - ) - if err != nil { - remoteUser = "root" - } - - log.Info("forwarding gpg-agent") - - // perform in background an ssh command forwarding the - // gpg agent, in order to have it immediately take effect - go func() { - err = exec.Command( - execPath, - "ssh", - "--gpg-agent-forwarding=true", - "--agent-forwarding=true", - "--start-services=true", - "--user", - remoteUser, - "--context", - client.Context(), - client.Workspace(), - "--log-output=raw", - "--command", "sleep infinity", - ).Run() - if err != nil { - log.Error("failure in forwarding gpg-agent") - } - }() - - return nil -} - // checkProviderUpdate currently only ensures the local provider is in sync with the remote for DevPod Pro instances // Potentially auto-upgrade other providers in the future. func checkProviderUpdate( diff --git a/pkg/gpg/forward.go b/pkg/gpg/forward.go new file mode 100644 index 000000000..d919f8397 --- /dev/null +++ b/pkg/gpg/forward.go @@ -0,0 +1,59 @@ +package gpg + +import ( + "os" + "os/exec" + + client2 "github.com/skevetter/devpod/pkg/client" + devssh "github.com/skevetter/devpod/pkg/ssh" + "github.com/skevetter/log" +) + +// ForwardAgent starts a background SSH connection that forwards the local GPG agent. +func ForwardAgent(client client2.BaseWorkspaceClient, logger log.Logger) error { + logger.Debug("gpg forwarding enabled, performing immediately") + + execPath, err := os.Executable() + if err != nil { + return err + } + + remoteUser, err := devssh.GetUser( + client.WorkspaceConfig().ID, + client.WorkspaceConfig().SSHConfigPath, + client.WorkspaceConfig().SSHConfigIncludePath, + ) + if err != nil { + remoteUser = "root" + } + + logger.Info("forwarding gpg-agent") + + args := buildForwardArgs(remoteUser, client.Context(), client.Workspace()) + + go func() { + //nolint:gosec // execPath comes from os.Executable() + err = exec.Command(execPath, args...).Run() + if err != nil { + logger.Error("failure in forwarding gpg-agent") + } + }() + + return nil +} + +func buildForwardArgs(user, context, workspace string) []string { + return []string{ + "ssh", + "--gpg-agent-forwarding=true", + "--agent-forwarding=true", + "--start-services=true", + "--user", + user, + "--context", + context, + workspace, + "--log-output=raw", + "--command", "sleep infinity", + } +} diff --git a/pkg/gpg/forward_test.go b/pkg/gpg/forward_test.go new file mode 100644 index 000000000..e6559e4a9 --- /dev/null +++ b/pkg/gpg/forward_test.go @@ -0,0 +1,21 @@ +package gpg + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestBuildForwardArgs(t *testing.T) { + args := buildForwardArgs("root", "test-context", "test-workspace") + assert.Contains(t, args, "ssh") + assert.Contains(t, args, "--gpg-agent-forwarding=true") + assert.Contains(t, args, "--agent-forwarding=true") + assert.Contains(t, args, "--user") + assert.Contains(t, args, "root") + assert.Contains(t, args, "--context") + assert.Contains(t, args, "test-context") + assert.Contains(t, args, "test-workspace") + assert.Contains(t, args, "--command") + assert.Contains(t, args, "sleep infinity") +} diff --git a/pkg/ide/opener/opener.go b/pkg/ide/opener/opener.go index 8183ea196..96f1ff04c 100644 --- a/pkg/ide/opener/opener.go +++ b/pkg/ide/opener/opener.go @@ -17,6 +17,7 @@ import ( "github.com/skevetter/devpod/pkg/command" "github.com/skevetter/devpod/pkg/config" config2 "github.com/skevetter/devpod/pkg/devcontainer/config" + "github.com/skevetter/devpod/pkg/gpg" "github.com/skevetter/devpod/pkg/ide/fleet" "github.com/skevetter/devpod/pkg/ide/jetbrains" "github.com/skevetter/devpod/pkg/ide/jupyter" @@ -240,7 +241,7 @@ func openJupyterBrowser( params Params, ) error { if params.GPGAgentForwarding { - if err := performGpgForwarding(params.Client, params.Log); err != nil { + if err := gpg.ForwardAgent(params.Client, params.Log); err != nil { return err } } @@ -288,7 +289,7 @@ func openRStudioBrowser( params Params, ) error { if params.GPGAgentForwarding { - if err := performGpgForwarding(params.Client, params.Log); err != nil { + if err := gpg.ForwardAgent(params.Client, params.Log); err != nil { return err } } @@ -334,7 +335,7 @@ func openVSCodeBrowser( params Params, ) error { if params.GPGAgentForwarding { - if err := performGpgForwarding(params.Client, params.Log); err != nil { + if err := gpg.ForwardAgent(params.Client, params.Log); err != nil { return err } } @@ -412,54 +413,6 @@ func startFleet(ctx context.Context, client client2.BaseWorkspaceClient, logger return open.Run(url) } -// Duplicated helpers below — these will be moved to pkg/gpg/ and pkg/tunnel/ in Tasks 3-4. - -func performGpgForwarding( - client client2.BaseWorkspaceClient, - logger log.Logger, -) error { - logger.Debug("gpg forwarding enabled, performing immediately") - - execPath, err := os.Executable() - if err != nil { - return err - } - - remoteUser, err := devssh.GetUser( - client.WorkspaceConfig().ID, - client.WorkspaceConfig().SSHConfigPath, - client.WorkspaceConfig().SSHConfigIncludePath, - ) - if err != nil { - remoteUser = "root" - } - - logger.Info("forwarding gpg-agent") - - go func() { - //nolint:gosec // execPath is the current binary, arguments are controlled - err = exec.Command( - execPath, - "ssh", - "--gpg-agent-forwarding=true", - "--agent-forwarding=true", - "--start-services=true", - "--user", - remoteUser, - "--context", - client.Context(), - client.Workspace(), - "--log-output=raw", - "--command", "sleep infinity", - ).Run() - if err != nil { - logger.Error("failure in forwarding gpg-agent") - } - }() - - return nil -} - func setupBackhaul(client client2.BaseWorkspaceClient, authSockID string, logger log.Logger) error { execPath, err := os.Executable() if err != nil { From 1471bc37a3454adf6087281756db8d4421845bbd Mon Sep 17 00:00:00 2001 From: Samuel K Date: Sat, 11 Apr 2026 23:27:38 -0500 Subject: [PATCH 5/8] refactor: move browser tunnel and backhaul into pkg/tunnel --- cmd/up.go | 207 ------------------------ pkg/ide/opener/opener.go | 318 +++++++++---------------------------- pkg/tunnel/browser.go | 215 +++++++++++++++++++++++++ pkg/tunnel/browser_test.go | 73 +++++++++ 4 files changed, 362 insertions(+), 451 deletions(-) create mode 100644 pkg/tunnel/browser.go create mode 100644 pkg/tunnel/browser_test.go diff --git a/cmd/up.go b/cmd/up.go index 3ca97ad0c..cec891e3a 100644 --- a/cmd/up.go +++ b/cmd/up.go @@ -5,7 +5,6 @@ import ( "fmt" "io" "os" - "os/exec" "os/signal" "path/filepath" "strconv" @@ -30,13 +29,11 @@ import ( provider2 "github.com/skevetter/devpod/pkg/provider" devssh "github.com/skevetter/devpod/pkg/ssh" "github.com/skevetter/devpod/pkg/telemetry" - "github.com/skevetter/devpod/pkg/tunnel" "github.com/skevetter/devpod/pkg/util" "github.com/skevetter/devpod/pkg/version" workspace2 "github.com/skevetter/devpod/pkg/workspace" "github.com/skevetter/log" "github.com/spf13/cobra" - "golang.org/x/crypto/ssh" ) // UpCmd holds the up cmd flags. @@ -665,182 +662,6 @@ func (cmd *UpCmd) devPodUpMachine( }) } -// setupBackhaul sets up a long running command in the container to ensure an SSH connection is kept alive. -func setupBackhaul(client client2.BaseWorkspaceClient, authSockId string, log log.Logger) error { - execPath, err := os.Executable() - if err != nil { - return err - } - - remoteUser, err := devssh.GetUser( - client.WorkspaceConfig().ID, - client.WorkspaceConfig().SSHConfigPath, - client.WorkspaceConfig().SSHConfigIncludePath, - ) - if err != nil { - remoteUser = "root" - } - - dotCmd := exec.Command( - execPath, - "ssh", - "--agent-forwarding=true", - fmt.Sprintf("--reuse-ssh-auth-sock=%s", authSockId), - "--start-services=false", - "--user", - remoteUser, - "--context", - client.Context(), - client.Workspace(), - "--log-output=raw", - "--command", - "while true; do sleep 6000000; done", // sleep infinity is not available on all systems - ) - - if log.GetLevel() == logrus.DebugLevel { - dotCmd.Args = append(dotCmd.Args, "--debug") - } - - log.Info("Setting up backhaul SSH connection") - - writer := log.Writer(logrus.InfoLevel, false) - - dotCmd.Stdout = writer - dotCmd.Stderr = writer - - err = dotCmd.Run() - if err != nil { - return err - } - - log.Infof("Done setting up backhaul") - - return nil -} - -func startBrowserTunnel( - ctx context.Context, - devPodConfig *config.Config, - client client2.BaseWorkspaceClient, - user, targetURL string, - forwardPorts bool, - extraPorts []string, - authSockID string, - gitSSHSigningKey string, - logger log.Logger, -) error { - // Setup a backhaul SSH connection using the remote user so there is an AUTH SOCK to use - // With normal IDEs this would be the SSH connection made by the IDE - // authSockID is not set when in proxy mode since we cannot use the proxies ssh-agent - if authSockID != "" { - go func() { - if err := setupBackhaul(client, authSockID, logger); err != nil { - logger.Error("Failed to setup backhaul SSH connection: ", err) - } - }() - } - - // handle this directly with the daemon client - daemonClient, ok := client.(client2.DaemonClient) - if ok { - toolClient, _, err := daemonClient.SSHClients(ctx, user) - if err != nil { - return err - } - defer func() { _ = toolClient.Close() }() - - err = clientimplementation.StartServicesDaemon( - ctx, - clientimplementation.StartServicesDaemonOptions{ - DevPodConfig: devPodConfig, - Client: daemonClient, - SSHClient: toolClient, - User: user, - Log: logger, - ForwardPorts: forwardPorts, - ExtraPorts: extraPorts, - }, - ) - if err != nil { - return err - } - <-ctx.Done() - - return nil - } - - err := tunnel.NewTunnel( - ctx, - func(ctx context.Context, stdin io.Reader, stdout io.Writer) error { - writer := logger.Writer(logrus.DebugLevel, false) - defer func() { _ = writer.Close() }() - - cmd, err := createSSHCommand(ctx, client, logger, []string{ - "--log-output=raw", - fmt.Sprintf("--reuse-ssh-auth-sock=%s", authSockID), - "--stdio", - }) - if err != nil { - return err - } - cmd.Stdout = stdout - cmd.Stdin = stdin - cmd.Stderr = writer - return cmd.Run() - }, - func(ctx context.Context, containerClient *ssh.Client) error { - // print port to console - streamLogger, ok := logger.(*log.StreamLogger) - if ok { - streamLogger.JSON(logrus.InfoLevel, map[string]string{ - "url": targetURL, - "done": "true", - }) - } - - configureDockerCredentials := devPodConfig.ContextOption( - config.ContextOptionSSHInjectDockerCredentials, - ) == config.BoolTrue - configureGitCredentials := devPodConfig.ContextOption( - config.ContextOptionSSHInjectGitCredentials, - ) == config.BoolTrue - configureGitSSHSignatureHelper := devPodConfig.ContextOption( - config.ContextOptionGitSSHSignatureForwarding, - ) == config.BoolTrue - - // run in container - err := tunnel.RunServices( - ctx, - tunnel.RunServicesOptions{ - DevPodConfig: devPodConfig, - ContainerClient: containerClient, - User: user, - ForwardPorts: forwardPorts, - ExtraPorts: extraPorts, - PlatformOptions: nil, - Workspace: client.WorkspaceConfig(), - ConfigureDockerCredentials: configureDockerCredentials, - ConfigureGitCredentials: configureGitCredentials, - ConfigureGitSSHSignatureHelper: configureGitSSHSignatureHelper, - GitSSHSigningKey: gitSSHSigningKey, - Log: logger, - }, - ) - if err != nil { - return fmt.Errorf("run credentials server in browser tunnel: %w", err) - } - - <-ctx.Done() - return nil - }, - ) - if err != nil { - return err - } - - return nil -} - type configureSSHParams struct { sshConfigPath string sshConfigIncludePath string @@ -933,34 +754,6 @@ var inheritedEnvironmentVariables = []string{ "GIT_COMMITTER_DATE", } -func createSSHCommand( - ctx context.Context, - client client2.BaseWorkspaceClient, - logger log.Logger, - extraArgs []string, -) (*exec.Cmd, error) { - execPath, err := os.Executable() - if err != nil { - return nil, err - } - - args := []string{ - "ssh", - "--user=root", - "--agent-forwarding=false", - "--start-services=false", - "--context", - client.Context(), - client.Workspace(), - } - if logger.GetLevel() == logrus.DebugLevel { - args = append(args, "--debug") - } - args = append(args, extraArgs...) - - return exec.CommandContext(ctx, execPath, args...), nil -} - // checkProviderUpdate currently only ensures the local provider is in sync with the remote for DevPod Pro instances // Potentially auto-upgrade other providers in the future. func checkProviderUpdate( diff --git a/pkg/ide/opener/opener.go b/pkg/ide/opener/opener.go index 96f1ff04c..72c4056b9 100644 --- a/pkg/ide/opener/opener.go +++ b/pkg/ide/opener/opener.go @@ -4,10 +4,7 @@ import ( "bytes" "context" "fmt" - "io" "net" - "os" - "os/exec" "strconv" "strings" @@ -27,11 +24,9 @@ import ( "github.com/skevetter/devpod/pkg/ide/zed" open2 "github.com/skevetter/devpod/pkg/open" "github.com/skevetter/devpod/pkg/port" - devssh "github.com/skevetter/devpod/pkg/ssh" "github.com/skevetter/devpod/pkg/tunnel" "github.com/skevetter/log" "github.com/skratchdot/open-golang/open" - "golang.org/x/crypto/ssh" ) // Params holds the parameters needed to open an IDE. @@ -221,18 +216,42 @@ func openJetBrains( return fmt.Errorf("unknown JetBrains IDE: %s", ideName) } -// browserTunnelParams bundles the arguments for browser-based IDE tunnels. -type browserTunnelParams struct { - ctx context.Context - devPodConfig *config.Config - client client2.BaseWorkspaceClient - user string - targetURL string - forwardPorts bool - extraPorts []string - authSockID string - gitSSHSigningKey string - logger log.Logger +func makeDaemonStartFunc( + params Params, + forwardPorts bool, + extraPorts []string, +) func(ctx context.Context) error { + daemonClient, ok := params.Client.(client2.DaemonClient) + if !ok { + return nil + } + + return func(ctx context.Context) error { + toolClient, _, err := daemonClient.SSHClients(ctx, params.User) + if err != nil { + return err + } + defer func() { _ = toolClient.Close() }() + + err = clientimplementation.StartServicesDaemon( + ctx, + clientimplementation.StartServicesDaemonOptions{ + DevPodConfig: params.DevPodConfig, + Client: daemonClient, + SSHClient: toolClient, + User: params.User, + Log: params.Log, + ForwardPorts: forwardPorts, + ExtraPorts: extraPorts, + }, + ) + if err != nil { + return err + } + <-ctx.Done() + + return nil + } } func openJupyterBrowser( @@ -270,16 +289,18 @@ func openJupyterBrowser( } params.Log.Infof("Starting jupyter notebook in browser mode at %s", targetURL) - return startBrowserTunnel(browserTunnelParams{ - ctx: ctx, - devPodConfig: params.DevPodConfig, - client: params.Client, - user: params.User, - targetURL: targetURL, - extraPorts: []string{fmt.Sprintf("%s:%d", addr, jupyter.DefaultServerPort)}, - authSockID: params.SSHAuthSockID, - gitSSHSigningKey: params.GitSSHSigningKey, - logger: params.Log, + extraPorts := []string{fmt.Sprintf("%s:%d", addr, jupyter.DefaultServerPort)} + return tunnel.StartBrowserTunnel(tunnel.BrowserTunnelParams{ + Ctx: ctx, + DevPodConfig: params.DevPodConfig, + Client: params.Client, + User: params.User, + TargetURL: targetURL, + ExtraPorts: extraPorts, + AuthSockID: params.SSHAuthSockID, + GitSSHSigningKey: params.GitSSHSigningKey, + Logger: params.Log, + DaemonStartFunc: makeDaemonStartFunc(params, false, extraPorts), }) } @@ -316,16 +337,18 @@ func openRStudioBrowser( } params.Log.Infof("Starting RStudio server in browser mode at %s", targetURL) - return startBrowserTunnel(browserTunnelParams{ - ctx: ctx, - devPodConfig: params.DevPodConfig, - client: params.Client, - user: params.User, - targetURL: targetURL, - extraPorts: []string{fmt.Sprintf("%s:%d", addr, rstudio.DefaultServerPort)}, - authSockID: params.SSHAuthSockID, - gitSSHSigningKey: params.GitSSHSigningKey, - logger: params.Log, + extraPorts := []string{fmt.Sprintf("%s:%d", addr, rstudio.DefaultServerPort)} + return tunnel.StartBrowserTunnel(tunnel.BrowserTunnelParams{ + Ctx: ctx, + DevPodConfig: params.DevPodConfig, + Client: params.Client, + User: params.User, + TargetURL: targetURL, + ExtraPorts: extraPorts, + AuthSockID: params.SSHAuthSockID, + GitSSHSigningKey: params.GitSSHSigningKey, + Logger: params.Log, + DaemonStartFunc: makeDaemonStartFunc(params, false, extraPorts), }) } @@ -368,23 +391,25 @@ func openVSCodeBrowser( ideOptions, openvscode.ForwardPortsOption, ) == config.BoolTrue - return startBrowserTunnel(browserTunnelParams{ - ctx: ctx, - devPodConfig: params.DevPodConfig, - client: params.Client, - user: params.User, - targetURL: targetURL, - forwardPorts: forwardPorts, - extraPorts: []string{fmt.Sprintf("%s:%d", addr, openvscode.DefaultVSCodePort)}, - authSockID: params.SSHAuthSockID, - gitSSHSigningKey: params.GitSSHSigningKey, - logger: params.Log, + extraPorts := []string{fmt.Sprintf("%s:%d", addr, openvscode.DefaultVSCodePort)} + return tunnel.StartBrowserTunnel(tunnel.BrowserTunnelParams{ + Ctx: ctx, + DevPodConfig: params.DevPodConfig, + Client: params.Client, + User: params.User, + TargetURL: targetURL, + ForwardPorts: forwardPorts, + ExtraPorts: extraPorts, + AuthSockID: params.SSHAuthSockID, + GitSSHSigningKey: params.GitSSHSigningKey, + Logger: params.Log, + DaemonStartFunc: makeDaemonStartFunc(params, forwardPorts, extraPorts), }) } func startFleet(ctx context.Context, client client2.BaseWorkspaceClient, logger log.Logger) error { stdout := &bytes.Buffer{} - sshCmd, err := createSSHCommand( + sshCmd, err := tunnel.CreateSSHCommand( ctx, client, logger, @@ -412,198 +437,3 @@ func startFleet(ctx context.Context, client client2.BaseWorkspaceClient, logger return open.Run(url) } - -func setupBackhaul(client client2.BaseWorkspaceClient, authSockID string, logger log.Logger) error { - execPath, err := os.Executable() - if err != nil { - return err - } - - remoteUser, err := devssh.GetUser( - client.WorkspaceConfig().ID, - client.WorkspaceConfig().SSHConfigPath, - client.WorkspaceConfig().SSHConfigIncludePath, - ) - if err != nil { - remoteUser = "root" - } - - //nolint:gosec // execPath is the current binary, arguments are controlled - dotCmd := exec.Command( - execPath, - "ssh", - "--agent-forwarding=true", - fmt.Sprintf("--reuse-ssh-auth-sock=%s", authSockID), - "--start-services=false", - "--user", - remoteUser, - "--context", - client.Context(), - client.Workspace(), - "--log-output=raw", - "--command", - "while true; do sleep 6000000; done", - ) - - if logger.GetLevel() == logrus.DebugLevel { - dotCmd.Args = append(dotCmd.Args, "--debug") - } - - logger.Info("Setting up backhaul SSH connection") - - writer := logger.Writer(logrus.InfoLevel, false) - - dotCmd.Stdout = writer - dotCmd.Stderr = writer - - err = dotCmd.Run() - if err != nil { - return err - } - - logger.Infof("Done setting up backhaul") - - return nil -} - -func createSSHCommand( - ctx context.Context, - client client2.BaseWorkspaceClient, - logger log.Logger, - extraArgs []string, -) (*exec.Cmd, error) { - execPath, err := os.Executable() - if err != nil { - return nil, err - } - - args := []string{ - "ssh", - "--user=root", - "--agent-forwarding=false", - "--start-services=false", - "--context", - client.Context(), - client.Workspace(), - } - if logger.GetLevel() == logrus.DebugLevel { - args = append(args, "--debug") - } - args = append(args, extraArgs...) - - //nolint:gosec // execPath is the current binary, arguments are controlled - return exec.CommandContext(ctx, execPath, args...), nil -} - -func startBrowserTunnel(p browserTunnelParams) error { - if p.authSockID != "" { - go func() { - if err := setupBackhaul(p.client, p.authSockID, p.logger); err != nil { - p.logger.Error("Failed to setup backhaul SSH connection: ", err) - } - }() - } - - daemonClient, ok := p.client.(client2.DaemonClient) - if ok { - return startBrowserTunnelDaemon(p, daemonClient) - } - - return startBrowserTunnelSSH(p) -} - -func startBrowserTunnelDaemon(p browserTunnelParams, daemonClient client2.DaemonClient) error { - toolClient, _, err := daemonClient.SSHClients(p.ctx, p.user) - if err != nil { - return err - } - defer func() { _ = toolClient.Close() }() - - err = clientimplementation.StartServicesDaemon( - p.ctx, - clientimplementation.StartServicesDaemonOptions{ - DevPodConfig: p.devPodConfig, - Client: daemonClient, - SSHClient: toolClient, - User: p.user, - Log: p.logger, - ForwardPorts: p.forwardPorts, - ExtraPorts: p.extraPorts, - }, - ) - if err != nil { - return err - } - <-p.ctx.Done() - - return nil -} - -func startBrowserTunnelSSH(p browserTunnelParams) error { - return tunnel.NewTunnel( - p.ctx, - func(ctx context.Context, stdin io.Reader, stdout io.Writer) error { - writer := p.logger.Writer(logrus.DebugLevel, false) - defer func() { _ = writer.Close() }() - - sshCmd, err := createSSHCommand(ctx, p.client, p.logger, []string{ - "--log-output=raw", - fmt.Sprintf("--reuse-ssh-auth-sock=%s", p.authSockID), - "--stdio", - }) - if err != nil { - return err - } - sshCmd.Stdout = stdout - sshCmd.Stdin = stdin - sshCmd.Stderr = writer - return sshCmd.Run() - }, - func(ctx context.Context, containerClient *ssh.Client) error { - return runBrowserTunnelServices(ctx, p, containerClient) - }, - ) -} - -func runBrowserTunnelServices( - ctx context.Context, - p browserTunnelParams, - containerClient *ssh.Client, -) error { - streamLogger, ok := p.logger.(*log.StreamLogger) - if ok { - streamLogger.JSON(logrus.InfoLevel, map[string]string{ - "url": p.targetURL, - "done": "true", - }) - } - - err := tunnel.RunServices( - ctx, - tunnel.RunServicesOptions{ - DevPodConfig: p.devPodConfig, - ContainerClient: containerClient, - User: p.user, - ForwardPorts: p.forwardPorts, - ExtraPorts: p.extraPorts, - Workspace: p.client.WorkspaceConfig(), - ConfigureDockerCredentials: p.devPodConfig.ContextOption( - config.ContextOptionSSHInjectDockerCredentials, - ) == config.BoolTrue, - ConfigureGitCredentials: p.devPodConfig.ContextOption( - config.ContextOptionSSHInjectGitCredentials, - ) == config.BoolTrue, - ConfigureGitSSHSignatureHelper: p.devPodConfig.ContextOption( - config.ContextOptionGitSSHSignatureForwarding, - ) == config.BoolTrue, - GitSSHSigningKey: p.gitSSHSigningKey, - Log: p.logger, - }, - ) - if err != nil { - return fmt.Errorf("run credentials server in browser tunnel: %w", err) - } - - <-ctx.Done() - return nil -} diff --git a/pkg/tunnel/browser.go b/pkg/tunnel/browser.go new file mode 100644 index 000000000..a5c8221ad --- /dev/null +++ b/pkg/tunnel/browser.go @@ -0,0 +1,215 @@ +package tunnel + +import ( + "context" + "fmt" + "io" + "os" + "os/exec" + + "github.com/sirupsen/logrus" + client2 "github.com/skevetter/devpod/pkg/client" + "github.com/skevetter/devpod/pkg/config" + devssh "github.com/skevetter/devpod/pkg/ssh" + "github.com/skevetter/log" + "golang.org/x/crypto/ssh" +) + +// BrowserTunnelParams bundles the arguments for browser-based IDE tunnels. +type BrowserTunnelParams struct { + Ctx context.Context + DevPodConfig *config.Config + Client client2.BaseWorkspaceClient + User string + TargetURL string + ForwardPorts bool + ExtraPorts []string + AuthSockID string + GitSSHSigningKey string + Logger log.Logger + + // DaemonStartFunc is called when the client is a DaemonClient. + // If nil, the SSH tunnel path is always used. + DaemonStartFunc func(ctx context.Context) error +} + +// StartBrowserTunnel sets up a browser tunnel for IDE access, either via daemon or SSH. +func StartBrowserTunnel(p BrowserTunnelParams) error { + if p.AuthSockID != "" { + go func() { + if err := SetupBackhaul(p.Client, p.AuthSockID, p.Logger); err != nil { + p.Logger.Error("Failed to setup backhaul SSH connection: ", err) + } + }() + } + + if p.DaemonStartFunc != nil { + return p.DaemonStartFunc(p.Ctx) + } + + return startBrowserTunnelSSH(p) +} + +func startBrowserTunnelSSH(p BrowserTunnelParams) error { + return NewTunnel( + p.Ctx, + func(ctx context.Context, stdin io.Reader, stdout io.Writer) error { + writer := p.Logger.Writer(logrus.DebugLevel, false) + defer func() { _ = writer.Close() }() + + sshCmd, err := CreateSSHCommand(ctx, p.Client, p.Logger, []string{ + "--log-output=raw", + fmt.Sprintf("--reuse-ssh-auth-sock=%s", p.AuthSockID), + "--stdio", + }) + if err != nil { + return err + } + sshCmd.Stdout = stdout + sshCmd.Stdin = stdin + sshCmd.Stderr = writer + return sshCmd.Run() + }, + func(ctx context.Context, containerClient *ssh.Client) error { + return runBrowserTunnelServices(ctx, p, containerClient) + }, + ) +} + +func runBrowserTunnelServices( + ctx context.Context, + p BrowserTunnelParams, + containerClient *ssh.Client, +) error { + streamLogger, ok := p.Logger.(*log.StreamLogger) + if ok { + streamLogger.JSON(logrus.InfoLevel, map[string]string{ + "url": p.TargetURL, + "done": "true", + }) + } + + err := RunServices( + ctx, + RunServicesOptions{ + DevPodConfig: p.DevPodConfig, + ContainerClient: containerClient, + User: p.User, + ForwardPorts: p.ForwardPorts, + ExtraPorts: p.ExtraPorts, + Workspace: p.Client.WorkspaceConfig(), + ConfigureDockerCredentials: p.DevPodConfig.ContextOption( + config.ContextOptionSSHInjectDockerCredentials, + ) == config.BoolTrue, + ConfigureGitCredentials: p.DevPodConfig.ContextOption( + config.ContextOptionSSHInjectGitCredentials, + ) == config.BoolTrue, + ConfigureGitSSHSignatureHelper: p.DevPodConfig.ContextOption( + config.ContextOptionGitSSHSignatureForwarding, + ) == config.BoolTrue, + GitSSHSigningKey: p.GitSSHSigningKey, + Log: p.Logger, + }, + ) + if err != nil { + return fmt.Errorf("run credentials server in browser tunnel: %w", err) + } + + <-ctx.Done() + return nil +} + +// SetupBackhaul sets up a long-running SSH connection for backhaul. +func SetupBackhaul(client client2.BaseWorkspaceClient, authSockID string, logger log.Logger) error { + execPath, err := os.Executable() + if err != nil { + return err + } + + remoteUser, err := devssh.GetUser( + client.WorkspaceConfig().ID, + client.WorkspaceConfig().SSHConfigPath, + client.WorkspaceConfig().SSHConfigIncludePath, + ) + if err != nil { + remoteUser = "root" + } + + //nolint:gosec // execPath is the current binary, arguments are controlled + dotCmd := exec.Command( + execPath, + "ssh", + "--agent-forwarding=true", + fmt.Sprintf("--reuse-ssh-auth-sock=%s", authSockID), + "--start-services=false", + "--user", + remoteUser, + "--context", + client.Context(), + client.Workspace(), + "--log-output=raw", + "--command", + "while true; do sleep 6000000; done", + ) + + if logger.GetLevel() == logrus.DebugLevel { + dotCmd.Args = append(dotCmd.Args, "--debug") + } + + logger.Info("Setting up backhaul SSH connection") + + writer := logger.Writer(logrus.InfoLevel, false) + + dotCmd.Stdout = writer + dotCmd.Stderr = writer + + err = dotCmd.Run() + if err != nil { + return err + } + + logger.Infof("Done setting up backhaul") + + return nil +} + +// CreateSSHCommand builds an exec.Cmd that runs `devpod ssh` with the given arguments. +func CreateSSHCommand( + ctx context.Context, + client client2.BaseWorkspaceClient, + logger log.Logger, + extraArgs []string, +) (*exec.Cmd, error) { + execPath, err := os.Executable() + if err != nil { + return nil, err + } + + args := buildSSHCommandArgs( + client.Context(), + client.Workspace(), + logger.GetLevel() == logrus.DebugLevel, + extraArgs, + ) + + //nolint:gosec // execPath is the current binary, arguments are controlled + return exec.CommandContext(ctx, execPath, args...), nil +} + +// buildSSHCommandArgs constructs the argument list for `devpod ssh`. +func buildSSHCommandArgs(clientContext, workspace string, debug bool, extraArgs []string) []string { + args := []string{ + "ssh", + "--user=root", + "--agent-forwarding=false", + "--start-services=false", + "--context", + clientContext, + workspace, + } + if debug { + args = append(args, "--debug") + } + args = append(args, extraArgs...) + return args +} diff --git a/pkg/tunnel/browser_test.go b/pkg/tunnel/browser_test.go new file mode 100644 index 000000000..6ead5c258 --- /dev/null +++ b/pkg/tunnel/browser_test.go @@ -0,0 +1,73 @@ +package tunnel + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestBuildSSHCommandArgs(t *testing.T) { + tests := []struct { + name string + context string + workspace string + debug bool + extraArgs []string + expected []string + }{ + { + name: "basic", + context: "default", + workspace: "my-workspace", + expected: []string{ + "ssh", + "--user=root", + "--agent-forwarding=false", + "--start-services=false", + "--context", + "default", + "my-workspace", + }, + }, + { + name: "with debug", + context: "default", + workspace: "my-workspace", + debug: true, + expected: []string{ + "ssh", + "--user=root", + "--agent-forwarding=false", + "--start-services=false", + "--context", + "default", + "my-workspace", + "--debug", + }, + }, + { + name: "with extra args", + context: "prod", + workspace: "ws", + extraArgs: []string{"--stdio", "--log-output=raw"}, + expected: []string{ + "ssh", + "--user=root", + "--agent-forwarding=false", + "--start-services=false", + "--context", + "prod", + "ws", + "--stdio", + "--log-output=raw", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := buildSSHCommandArgs(tt.context, tt.workspace, tt.debug, tt.extraArgs) + assert.Equal(t, tt.expected, got) + }) + } +} From 5c5339eac2723617c1644eba90c8e064adb0a232 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Sat, 11 Apr 2026 23:32:26 -0500 Subject: [PATCH 6/8] refactor: move provider update check into pkg/workspace --- cmd/up.go | 100 +------------------ pkg/workspace/provider_update.go | 135 ++++++++++++++++++++++++++ pkg/workspace/provider_update_test.go | 57 +++++++++++ 3 files changed, 194 insertions(+), 98 deletions(-) create mode 100644 pkg/workspace/provider_update.go create mode 100644 pkg/workspace/provider_update_test.go diff --git a/cmd/up.go b/cmd/up.go index cec891e3a..bd9be3922 100644 --- a/cmd/up.go +++ b/cmd/up.go @@ -11,7 +11,6 @@ import ( "strings" "syscall" - "github.com/blang/semver/v4" "github.com/sirupsen/logrus" "github.com/skevetter/devpod/cmd/flags" "github.com/skevetter/devpod/pkg/agent" @@ -25,12 +24,10 @@ import ( "github.com/skevetter/devpod/pkg/ide" "github.com/skevetter/devpod/pkg/ide/opener" options2 "github.com/skevetter/devpod/pkg/options" - "github.com/skevetter/devpod/pkg/platform" provider2 "github.com/skevetter/devpod/pkg/provider" devssh "github.com/skevetter/devpod/pkg/ssh" "github.com/skevetter/devpod/pkg/telemetry" "github.com/skevetter/devpod/pkg/util" - "github.com/skevetter/devpod/pkg/version" workspace2 "github.com/skevetter/devpod/pkg/workspace" "github.com/skevetter/log" "github.com/spf13/cobra" @@ -754,99 +751,6 @@ var inheritedEnvironmentVariables = []string{ "GIT_COMMITTER_DATE", } -// checkProviderUpdate currently only ensures the local provider is in sync with the remote for DevPod Pro instances -// Potentially auto-upgrade other providers in the future. -func checkProviderUpdate( - devPodConfig *config.Config, - proInstance *provider2.ProInstance, - log log.Logger, -) error { - if version.GetVersion() == version.DevVersion { - log.Debugf("skipping provider upgrade check during development") - return nil - } - if proInstance == nil { - log.Debug("no pro instance available, skipping provider upgrade check") - return nil - } - - // compare versions - newVersion, err := platform.GetProInstanceDevPodVersion(proInstance) - if err != nil { - return fmt.Errorf("version for pro instance %s: %w", proInstance.Host, err) - } - - p, err := workspace2.FindProvider(devPodConfig, proInstance.Provider, log) - if err != nil { - return fmt.Errorf("get provider config for pro provider %s: %w", proInstance.Provider, err) - } - if p.Config.Version == version.DevVersion { - return nil - } - if p.Config.Source.Internal { - return nil - } - - v1, err := semver.Parse(strings.TrimPrefix(newVersion, "v")) - if err != nil { - return fmt.Errorf("parse version %s: %w", newVersion, err) - } - v2, err := semver.Parse(strings.TrimPrefix(p.Config.Version, "v")) - if err != nil { - return fmt.Errorf("parse version %s: %w", p.Config.Version, err) - } - if v1.Compare(v2) == 0 { - return nil - } - log.Infof( - "New provider version available, attempting to update %s from %s to %s", - proInstance.Provider, - p.Config.Version, - newVersion, - ) - - providerSource, err := workspace2.ResolveProviderSource(devPodConfig, proInstance.Provider, log) - if err != nil { - return fmt.Errorf("resolve provider source %s: %w", proInstance.Provider, err) - } - - splitted := strings.Split(providerSource, "@") - if len(splitted) == 0 { - return fmt.Errorf("no provider source found %s", providerSource) - } - providerSource = splitted[0] + "@" + newVersion - - _, err = workspace2.UpdateProvider(devPodConfig, proInstance.Provider, providerSource, log) - if err != nil { - return fmt.Errorf("update provider %s: %w", proInstance.Provider, err) - } - - log.WithFields(logrus.Fields{ - "provider": proInstance.Provider, - }).Done("updated provider") - return nil -} - -func getProInstance( - devPodConfig *config.Config, - providerName string, - log log.Logger, -) *provider2.ProInstance { - proInstances, err := workspace2.ListProInstances(devPodConfig, log) - if err != nil { - return nil - } else if len(proInstances) == 0 { - return nil - } - - proInstance, ok := workspace2.FindProviderProInstance(proInstances, providerName) - if !ok { - return nil - } - - return proInstance -} - func (cmd *UpCmd) prepareClient( ctx context.Context, devPodConfig *config.Config, @@ -920,8 +824,8 @@ func (cmd *UpCmd) prepareClient( } if !cmd.Platform.Enabled { - proInstance := getProInstance(devPodConfig, client.Provider(), logger) - err = checkProviderUpdate(devPodConfig, proInstance, logger) + proInstance := workspace2.GetProInstance(devPodConfig, client.Provider(), logger) + err = workspace2.CheckProviderUpdate(devPodConfig, proInstance, logger) if err != nil { return nil, logger, err } diff --git a/pkg/workspace/provider_update.go b/pkg/workspace/provider_update.go new file mode 100644 index 000000000..476ddead0 --- /dev/null +++ b/pkg/workspace/provider_update.go @@ -0,0 +1,135 @@ +package workspace + +import ( + "fmt" + "strings" + + "github.com/blang/semver/v4" + "github.com/sirupsen/logrus" + "github.com/skevetter/devpod/pkg/config" + "github.com/skevetter/devpod/pkg/platform" + provider2 "github.com/skevetter/devpod/pkg/provider" + "github.com/skevetter/devpod/pkg/version" + "github.com/skevetter/log" +) + +// CheckProviderUpdate currently only ensures the local provider is in sync with the remote for DevPod Pro instances. +// Potentially auto-upgrade other providers in the future. +func CheckProviderUpdate( + devPodConfig *config.Config, + proInstance *provider2.ProInstance, + log log.Logger, +) error { + if version.GetVersion() == version.DevVersion { + log.Debugf("skipping provider upgrade check during development") + return nil + } + if proInstance == nil { + log.Debug("no pro instance available, skipping provider upgrade check") + return nil + } + + return checkProviderUpdateForInstance(devPodConfig, proInstance, log) +} + +func checkProviderUpdateForInstance( + devPodConfig *config.Config, + proInstance *provider2.ProInstance, + log log.Logger, +) error { + newVersion, err := platform.GetProInstanceDevPodVersion(proInstance) + if err != nil { + return fmt.Errorf("version for pro instance %s: %w", proInstance.Host, err) + } + + p, err := FindProvider(devPodConfig, proInstance.Provider, log) + if err != nil { + return fmt.Errorf("get provider config for pro provider %s: %w", proInstance.Provider, err) + } + + if shouldSkipProviderUpdate(p.Config.Version == version.DevVersion, p.Config.Source.Internal) { + return nil + } + + needsUpdate, err := providerVersionNeedsUpdate(newVersion, p.Config.Version) + if err != nil { + return err + } + if !needsUpdate { + return nil + } + + log.Infof( + "New provider version available, attempting to update %s from %s to %s", + proInstance.Provider, + p.Config.Version, + newVersion, + ) + + return applyProviderUpdate(devPodConfig, proInstance.Provider, newVersion, log) +} + +func providerVersionNeedsUpdate(newVersion, currentVersion string) (bool, error) { + v1, err := semver.Parse(strings.TrimPrefix(newVersion, "v")) + if err != nil { + return false, fmt.Errorf("parse version %s: %w", newVersion, err) + } + v2, err := semver.Parse(strings.TrimPrefix(currentVersion, "v")) + if err != nil { + return false, fmt.Errorf("parse version %s: %w", currentVersion, err) + } + return v1.Compare(v2) != 0, nil +} + +func applyProviderUpdate( + devPodConfig *config.Config, + providerName, newVersion string, + log log.Logger, +) error { + providerSource, err := ResolveProviderSource(devPodConfig, providerName, log) + if err != nil { + return fmt.Errorf("resolve provider source %s: %w", providerName, err) + } + + splitted := strings.Split(providerSource, "@") + if len(splitted) == 0 { + return fmt.Errorf("no provider source found %s", providerSource) + } + providerSource = splitted[0] + "@" + newVersion + + _, err = UpdateProvider(devPodConfig, providerName, providerSource, log) + if err != nil { + return fmt.Errorf("update provider %s: %w", providerName, err) + } + + log.WithFields(logrus.Fields{ + "provider": providerName, + }).Done("updated provider") + return nil +} + +// GetProInstance returns the ProInstance associated with the given provider name, or nil if not found. +func GetProInstance( + devPodConfig *config.Config, + providerName string, + log log.Logger, +) *provider2.ProInstance { + proInstances, err := ListProInstances(devPodConfig, log) + if err != nil { + return nil + } else if len(proInstances) == 0 { + return nil + } + + proInstance, ok := FindProviderProInstance(proInstances, providerName) + if !ok { + return nil + } + + return proInstance +} + +// shouldSkipProviderUpdate returns true if the provider update check should be skipped. +func shouldSkipProviderUpdate(isDevVersion, isInternal bool) bool { + return isDevVersion || isInternal +} diff --git a/pkg/workspace/provider_update_test.go b/pkg/workspace/provider_update_test.go new file mode 100644 index 000000000..ddcd414a5 --- /dev/null +++ b/pkg/workspace/provider_update_test.go @@ -0,0 +1,57 @@ +package workspace + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestShouldSkipProviderUpdate(t *testing.T) { + tests := []struct { + name string + isDevVersion bool + isInternal bool + expected bool + }{ + { + name: "skip when dev version", + isDevVersion: true, + isInternal: false, + expected: true, + }, + { + name: "skip when internal", + isDevVersion: false, + isInternal: true, + expected: true, + }, + { + name: "skip when both dev version and internal", + isDevVersion: true, + isInternal: true, + expected: true, + }, + { + name: "do not skip for regular provider", + isDevVersion: false, + isInternal: false, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := shouldSkipProviderUpdate(tt.isDevVersion, tt.isInternal) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestGetProInstance_EmptyProviderName(t *testing.T) { + // GetProInstance returns nil when provider name doesn't match any pro instance. + // We can't easily test with a real config without disk I/O, but we can verify + // that shouldSkipProviderUpdate is the composable unit for logic testing. + // This is a placeholder for integration-level testing. + result := shouldSkipProviderUpdate(false, false) + assert.False(t, result, "regular provider should not be skipped") +} From 09a82a1fea6291ffea2c8d515626ca243c3a170c Mon Sep 17 00:00:00 2001 From: Samuel K Date: Sat, 11 Apr 2026 23:40:58 -0500 Subject: [PATCH 7/8] test: add providerVersionNeedsUpdate tests and fix naming - Add 7 table-driven tests for providerVersionNeedsUpdate covering same version, upgrades, downgrades, mixed prefixes, and invalid input - Add buildSSHCommandArgs test case for debug + extra args combined - Rename dotCmd to backhaulCmd in SetupBackhaul for clarity --- .../plans/2026-04-11-cmd-up-refactor.md | 1571 ----------------- .../2026-04-11-cmd-up-refactor-design.md | 182 -- pkg/tunnel/browser.go | 10 +- pkg/tunnel/browser_test.go | 59 +- pkg/workspace/provider_update_test.go | 32 +- 5 files changed, 49 insertions(+), 1805 deletions(-) delete mode 100644 docs/superpowers/plans/2026-04-11-cmd-up-refactor.md delete mode 100644 docs/superpowers/specs/2026-04-11-cmd-up-refactor-design.md diff --git a/docs/superpowers/plans/2026-04-11-cmd-up-refactor.md b/docs/superpowers/plans/2026-04-11-cmd-up-refactor.md deleted file mode 100644 index cb365a536..000000000 --- a/docs/superpowers/plans/2026-04-11-cmd-up-refactor.md +++ /dev/null @@ -1,1571 +0,0 @@ -# cmd/up.go Refactor Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Extract the 1839-line `cmd/up.go` into focused `pkg/` packages with unit tests, leaving ~350-400 lines of command orchestration. - -**Architecture:** Six commits, each extracting one responsibility area into a `pkg/` package (or extending an existing one), adding tests, and updating `cmd/up.go` to call the new package. Pure refactor -- no behavior changes. - -**Tech Stack:** Go, cobra, testify/assert, table-driven tests - -**Spec:** `docs/superpowers/specs/2026-04-11-cmd-up-refactor-design.md` - ---- - -## File Structure - -| Action | Path | Responsibility | -|--------|------|----------------| -| Create | `pkg/dotfiles/dotfiles.go` | Dotfiles setup orchestration | -| Create | `pkg/dotfiles/dotfiles_test.go` | Tests for pure helper functions | -| Create | `pkg/ide/opener/opener.go` | IDE launch dispatch and helpers | -| Create | `pkg/ide/opener/opener_test.go` | Tests for parseAddressAndPort, dispatch routing | -| Create | `pkg/gpg/forward.go` | GPG agent forwarding | -| Create | `pkg/gpg/forward_test.go` | Tests for command construction | -| Modify | `pkg/tunnel/browser.go` (create) | Browser tunnel + backhaul | -| Create | `pkg/tunnel/browser_test.go` | Tests for SSH command construction | -| Modify | `pkg/workspace/provider.go` | Add CheckProviderUpdate, GetProInstance | -| Create | `pkg/workspace/provider_update_test.go` | Tests for version comparison | -| Modify | `cmd/up.go` | Slim down to orchestration only | - ---- - -### Task 1: Extract `pkg/dotfiles/` - -**Files:** -- Create: `pkg/dotfiles/dotfiles.go` -- Create: `pkg/dotfiles/dotfiles_test.go` -- Modify: `cmd/up.go:1384-1553` (remove moved functions) - -- [ ] **Step 1: Write failing tests for pure helper functions** - -Create `pkg/dotfiles/dotfiles_test.go`: - -```go -package dotfiles - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestExtractKeysFromEnvKeyValuePairs(t *testing.T) { - tests := []struct { - name string - input []string - want []string - }{ - { - name: "empty input", - input: []string{}, - want: []string{}, - }, - { - name: "single pair", - input: []string{"FOO=bar"}, - want: []string{"FOO"}, - }, - { - name: "multiple pairs", - input: []string{"FOO=bar", "BAZ=qux"}, - want: []string{"FOO", "BAZ"}, - }, - { - name: "value with equals sign", - input: []string{"FOO=bar=baz"}, - want: []string{"FOO"}, - }, - { - name: "no equals sign skipped", - input: []string{"INVALID"}, - want: []string{}, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := extractKeysFromEnvKeyValuePairs(tt.input) - assert.Equal(t, tt.want, got) - }) - } -} - -func TestCollectDotfilesScriptEnvKeyValuePairs(t *testing.T) { - t.Run("empty file list", func(t *testing.T) { - got, err := collectDotfilesScriptEnvKeyValuePairs(nil) - assert.NoError(t, err) - assert.Empty(t, got) - }) - - t.Run("reads key-value pairs from file", func(t *testing.T) { - dir := t.TempDir() - envFile := dir + "/test.env" - err := os.WriteFile(envFile, []byte("FOO=bar\nBAZ=qux\n"), 0o600) - assert.NoError(t, err) - - got, err := collectDotfilesScriptEnvKeyValuePairs([]string{envFile}) - assert.NoError(t, err) - assert.Contains(t, got, "FOO=bar") - assert.Contains(t, got, "BAZ=qux") - }) - - t.Run("nonexistent file returns error", func(t *testing.T) { - _, err := collectDotfilesScriptEnvKeyValuePairs([]string{"/nonexistent/file"}) - assert.Error(t, err) - }) -} - -func TestBuildDotCmdAgentArguments(t *testing.T) { - tests := []struct { - name string - repo string - script string - strict bool - debug bool - wantArgs []string - }{ - { - name: "basic repo only", - repo: "https://github.com/user/dots", - wantArgs: []string{ - "agent", "workspace", "install-dotfiles", - "--repository", "https://github.com/user/dots", - }, - }, - { - name: "with script", - repo: "https://github.com/user/dots", - script: "install.sh", - wantArgs: []string{ - "agent", "workspace", "install-dotfiles", - "--repository", "https://github.com/user/dots", - "--install-script", "install.sh", - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := buildDotCmdAgentArguments(tt.repo, tt.script, tt.strict, tt.debug) - assert.Equal(t, tt.wantArgs, got) - }) - } -} -``` - -- [ ] **Step 2: Run tests to verify they fail** - -Run: `cd pkg/dotfiles && go test -v ./...` -Expected: FAIL -- package doesn't exist yet - -- [ ] **Step 3: Create `pkg/dotfiles/dotfiles.go`** - -Move the following functions from `cmd/up.go` lines 1384-1553 into `pkg/dotfiles/dotfiles.go`: -- `setupDotfiles` -> exported as `Setup` -- `buildDotCmd` -> unexported -- `buildDotCmdAgentArguments` -> unexported (refactored to accept booleans instead of config/logger) -- `extractKeysFromEnvKeyValuePairs` -> unexported -- `collectDotfilesScriptEnvKeyvaluePairs` -> unexported (renamed to `collectDotfilesScriptEnvKeyValuePairs`) - -```go -package dotfiles - -import ( - "fmt" - "os" - "os/exec" - "slices" - "strings" - - "github.com/sirupsen/logrus" - "github.com/skevetter/devpod/pkg/agent" - client2 "github.com/skevetter/devpod/pkg/client" - "github.com/skevetter/devpod/pkg/config" - config2 "github.com/skevetter/devpod/pkg/devcontainer/config" - devssh "github.com/skevetter/devpod/pkg/ssh" - "github.com/skevetter/log" -) - -// SetupParams holds all parameters for dotfiles setup. -type SetupParams struct { - Source string - Script string - EnvFiles []string - EnvKeyValues []string - Client client2.BaseWorkspaceClient - DevPodConfig *config.Config - Log log.Logger -} - -// Setup clones and installs dotfiles into the devcontainer. -func Setup(params SetupParams) error { - dotfilesRepo := params.DevPodConfig.ContextOption(config.ContextOptionDotfilesURL) - if params.Source != "" { - dotfilesRepo = params.Source - } - - dotfilesScript := params.DevPodConfig.ContextOption(config.ContextOptionDotfilesScript) - if params.Script != "" { - dotfilesScript = params.Script - } - - if dotfilesRepo == "" { - params.Log.Debug("No dotfiles repo specified, skipping") - return nil - } - - params.Log.Infof("Dotfiles Git repository %s specified", dotfilesRepo) - params.Log.Debug("Cloning dotfiles into the devcontainer...") - - strictHostKey := params.DevPodConfig.ContextOption( - config.ContextOptionSSHStrictHostKeyChecking, - ) == config.BoolTrue - - dotCmd, err := buildDotCmd( - dotfilesRepo, - dotfilesScript, - params.EnvFiles, - params.EnvKeyValues, - params.Client, - strictHostKey, - params.Log, - ) - if err != nil { - return err - } - if params.Log.GetLevel() == logrus.DebugLevel { - dotCmd.Args = append(dotCmd.Args, "--debug") - } - - params.Log.Debugf("Running dotfiles setup command: %v", dotCmd.Args) - - writer := params.Log.Writer(logrus.InfoLevel, false) - - dotCmd.Stdout = writer - dotCmd.Stderr = writer - - err = dotCmd.Run() - if err != nil { - return err - } - - params.Log.Infof("Done setting up dotfiles into the devcontainer") - - return nil -} - -func buildDotCmdAgentArguments( - dotfilesRepo, dotfilesScript string, - strictHostKey, debug bool, -) []string { - agentArguments := []string{ - "agent", - "workspace", - "install-dotfiles", - "--repository", - dotfilesRepo, - } - - if strictHostKey { - agentArguments = append(agentArguments, "--strict-host-key-checking") - } - - if debug { - agentArguments = append(agentArguments, "--debug") - } - - if dotfilesScript != "" { - agentArguments = append(agentArguments, "--install-script", dotfilesScript) - } - - return agentArguments -} - -func buildDotCmd( - dotfilesRepo, dotfilesScript string, - envFiles, envKeyValuePairs []string, - client client2.BaseWorkspaceClient, - strictHostKey bool, - logger log.Logger, -) (*exec.Cmd, error) { - sshCmd := []string{ - "ssh", - "--agent-forwarding=true", - "--start-services=true", - } - - envFilesKeyValuePairs, err := collectDotfilesScriptEnvKeyValuePairs(envFiles) - if err != nil { - return nil, err - } - - allEnvKeyValuesPairs := slices.Concat(envFilesKeyValuePairs, envKeyValuePairs) - allEnvKeys := extractKeysFromEnvKeyValuePairs(allEnvKeyValuesPairs) - for _, envKey := range allEnvKeys { - sshCmd = append(sshCmd, "--send-env", envKey) - } - - remoteUser, err := devssh.GetUser( - client.WorkspaceConfig().ID, - client.WorkspaceConfig().SSHConfigPath, - client.WorkspaceConfig().SSHConfigIncludePath, - ) - if err != nil { - remoteUser = "root" - } - - agentArguments := buildDotCmdAgentArguments( - dotfilesRepo, dotfilesScript, - strictHostKey, - logger.GetLevel() == logrus.DebugLevel, - ) - sshCmd = append(sshCmd, - "--user", - remoteUser, - "--context", - client.Context(), - client.Workspace(), - "--log-output=raw", - "--command", - agent.ContainerDevPodHelperLocation+" "+strings.Join(agentArguments, " "), - ) - execPath, err := os.Executable() - if err != nil { - return nil, err - } - - dotCmd := exec.Command( - execPath, - sshCmd..., - ) - - dotCmd.Env = append(dotCmd.Environ(), allEnvKeyValuesPairs...) - return dotCmd, nil -} - -func extractKeysFromEnvKeyValuePairs(envKeyValuePairs []string) []string { - keys := []string{} - for _, env := range envKeyValuePairs { - keyValue := strings.SplitN(env, "=", 2) - if len(keyValue) == 2 { - keys = append(keys, keyValue[0]) - } - } - return keys -} - -func collectDotfilesScriptEnvKeyValuePairs(envFiles []string) ([]string, error) { - keyValues := []string{} - for _, file := range envFiles { - envFromFile, err := config2.ParseKeyValueFile(file) - if err != nil { - return nil, err - } - keyValues = append(keyValues, envFromFile...) - } - return keyValues, nil -} -``` - -- [ ] **Step 4: Add `import "os"` to test file and run tests** - -Run: `cd pkg/dotfiles && go test -v ./...` -Expected: PASS for all helper function tests - -- [ ] **Step 5: Update `cmd/up.go` to use `pkg/dotfiles`** - -Replace the `setupDotfiles` call in `cmd/up.go` `configureWorkspace` method (around line 385): - -```go -// Old: -if err := setupDotfiles( - cmd.DotfilesSource, - cmd.DotfilesScript, - cmd.DotfilesScriptEnvFile, - cmd.DotfilesScriptEnv, - client, - devPodConfig, - log, -); err != nil { - return err -} - -// New: -if err := dotfiles.Setup(dotfiles.SetupParams{ - Source: cmd.DotfilesSource, - Script: cmd.DotfilesScript, - EnvFiles: cmd.DotfilesScriptEnvFile, - EnvKeyValues: cmd.DotfilesScriptEnv, - Client: client, - DevPodConfig: devPodConfig, - Log: log, -}); err != nil { - return err -} -``` - -Remove the following functions from `cmd/up.go`: `setupDotfiles`, `buildDotCmd`, `buildDotCmdAgentArguments`, `extractKeysFromEnvKeyValuePairs`, `collectDotfilesScriptEnvKeyvaluePairs`. - -Add import: `"github.com/skevetter/devpod/pkg/dotfiles"` - -Remove now-unused imports from `cmd/up.go`: `"slices"` (if no other uses remain). - -- [ ] **Step 6: Verify build and existing tests pass** - -Run: `go build ./... && go test ./cmd/... ./pkg/dotfiles/...` -Expected: BUILD OK, PASS - -- [ ] **Step 7: Commit** - -```bash -git add pkg/dotfiles/ cmd/up.go -git commit -m "$(cat <<'EOF' -refactor: extract pkg/dotfiles for dotfiles setup - -Move dotfiles cloning and installation logic from cmd/up.go into a -dedicated pkg/dotfiles package. Add unit tests for pure helper functions. -EOF -)" -``` - ---- - -### Task 2: Extract `pkg/ide/opener/` - -**Files:** -- Create: `pkg/ide/opener/opener.go` -- Create: `pkg/ide/opener/opener_test.go` -- Modify: `cmd/up.go:400-583, 840-1056` (remove moved functions) - -- [ ] **Step 1: Write failing tests** - -Create `pkg/ide/opener/opener_test.go`: - -```go -package opener - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestParseAddressAndPort(t *testing.T) { - tests := []struct { - name string - bindAddr string - defaultPort int - wantAddr string - wantPort int - wantErr bool - }{ - { - name: "empty uses default port", - bindAddr: "", - defaultPort: 8080, - wantAddr: "", // will be the found port as string - wantPort: 0, // will be >= defaultPort - wantErr: false, - }, - { - name: "explicit host:port", - bindAddr: "127.0.0.1:9090", - defaultPort: 8080, - wantAddr: "127.0.0.1:9090", - wantPort: 9090, - wantErr: false, - }, - { - name: "missing port returns error", - bindAddr: "127.0.0.1:", - defaultPort: 8080, - wantErr: true, - }, - { - name: "invalid format returns error", - bindAddr: "not-a-host-port", - defaultPort: 8080, - wantErr: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - addr, port, err := ParseAddressAndPort(tt.bindAddr, tt.defaultPort) - if tt.wantErr { - assert.Error(t, err) - return - } - assert.NoError(t, err) - if tt.bindAddr == "" { - // When empty, a free port is found - assert.Greater(t, port, 0) - } else { - assert.Equal(t, tt.wantAddr, addr) - assert.Equal(t, tt.wantPort, port) - } - }) - } -} -``` - -- [ ] **Step 2: Run tests to verify they fail** - -Run: `cd pkg/ide/opener && go test -v ./...` -Expected: FAIL -- package doesn't exist yet - -- [ ] **Step 3: Create `pkg/ide/opener/opener.go`** - -Move the following from `cmd/up.go`: -- `ideOpener` struct and methods -> restructured as package-level `Open` function -- `openVSCodeFlavor` -> unexported -- `openJetBrains` -> unexported -- `startVSCodeInBrowser` -> unexported -- `startJupyterNotebookInBrowser` -> unexported -- `startRStudioInBrowser` -> unexported -- `startFleet` -> unexported -- `parseAddressAndPort` -> exported as `ParseAddressAndPort` (used by tests, useful utility) - -```go -package opener - -import ( - "bytes" - "context" - "fmt" - "net" - "os" - "os/exec" - "strconv" - "strings" - - "github.com/sirupsen/logrus" - client2 "github.com/skevetter/devpod/pkg/client" - "github.com/skevetter/devpod/pkg/command" - "github.com/skevetter/devpod/pkg/config" - config2 "github.com/skevetter/devpod/pkg/devcontainer/config" - "github.com/skevetter/devpod/pkg/ide/fleet" - "github.com/skevetter/devpod/pkg/ide/jetbrains" - "github.com/skevetter/devpod/pkg/ide/jupyter" - "github.com/skevetter/devpod/pkg/ide/openvscode" - "github.com/skevetter/devpod/pkg/ide/rstudio" - "github.com/skevetter/devpod/pkg/ide/vscode" - "github.com/skevetter/devpod/pkg/ide/zed" - open2 "github.com/skevetter/devpod/pkg/open" - "github.com/skevetter/devpod/pkg/port" - "github.com/skevetter/log" - "github.com/skratchdot/open-golang/open" -) - -// Params holds everything the IDE opener needs from the command layer. -type Params struct { - GPGAgentForwarding bool - SSHAuthSockID string - GitSSHSigningKey string - DevPodConfig *config.Config - Client client2.BaseWorkspaceClient - User string - Result *config2.Result - Log log.Logger -} - -// Open launches the appropriate IDE based on ideName. -func Open( - ctx context.Context, - ideName string, - ideOptions map[string]config.OptionValue, - params Params, -) error { - folder := params.Result.SubstitutionContext.ContainerWorkspaceFolder - workspace := params.Client.Workspace() - user := params.User - - switch ideName { - case string(config.IDEVSCode), string(config.IDEVSCodeInsiders), string(config.IDECursor), - string(config.IDECodium), string(config.IDEPositron), string(config.IDEWindsurf), - string(config.IDEAntigravity), string(config.IDEBob): - return openVSCodeFlavor(ctx, ideName, folder, workspace, ideOptions, params.Log) - - case string(config.IDERustRover), string(config.IDEGoland), string(config.IDEPyCharm), - string(config.IDEPhpStorm), string(config.IDEIntellij), string(config.IDECLion), - string(config.IDERider), string(config.IDERubyMine), string(config.IDEWebStorm), - string(config.IDEDataSpell): - return openJetBrains(ideName, folder, workspace, user, ideOptions, params.Log) - - case string(config.IDEOpenVSCode): - return startVSCodeInBrowser( - params.GPGAgentForwarding, ctx, params.DevPodConfig, params.Client, - folder, user, ideOptions, params.SSHAuthSockID, params.GitSSHSigningKey, params.Log, - ) - - case string(config.IDEFleet): - return startFleet(ctx, params.Client, params.Log) - - case string(config.IDEZed): - return zed.Open(ctx, ideOptions, user, folder, workspace, params.Log) - - case string(config.IDEJupyterNotebook): - return startJupyterNotebookInBrowser( - params.GPGAgentForwarding, ctx, params.DevPodConfig, params.Client, - user, ideOptions, params.SSHAuthSockID, params.GitSSHSigningKey, params.Log, - ) - - case string(config.IDERStudio): - return startRStudioInBrowser( - params.GPGAgentForwarding, ctx, params.DevPodConfig, params.Client, - user, ideOptions, params.SSHAuthSockID, params.GitSSHSigningKey, params.Log, - ) - - default: - return nil - } -} - -// ParseAddressAndPort parses a bind address option into address and port. -// If bindAddressOption is empty, finds an available port starting from defaultPort. -func ParseAddressAndPort(bindAddressOption string, defaultPort int) (string, int, error) { - if bindAddressOption == "" { - portName, err := port.FindAvailablePort(defaultPort) - if err != nil { - return "", 0, err - } - return fmt.Sprintf("%d", portName), portName, nil - } - - address := bindAddressOption - _, portStr, err := net.SplitHostPort(address) - if err != nil { - return "", 0, fmt.Errorf("parse host:port: %w", err) - } - if portStr == "" { - return "", 0, fmt.Errorf("parse ADDRESS: expected host:port, got %s", address) - } - - portName, err := strconv.Atoi(portStr) - if err != nil { - return "", 0, fmt.Errorf("parse host:port: %w", err) - } - - return address, portName, nil -} - -func openVSCodeFlavor( - ctx context.Context, - ideName, folder, workspace string, - ideOptions map[string]config.OptionValue, - logger log.Logger, -) error { - flavorMap := map[string]vscode.Flavor{ - string(config.IDEVSCode): vscode.FlavorStable, - string(config.IDEVSCodeInsiders): vscode.FlavorInsiders, - string(config.IDECursor): vscode.FlavorCursor, - string(config.IDECodium): vscode.FlavorCodium, - string(config.IDEPositron): vscode.FlavorPositron, - string(config.IDEWindsurf): vscode.FlavorWindsurf, - string(config.IDEAntigravity): vscode.FlavorAntigravity, - string(config.IDEBob): vscode.FlavorBob, - } - - params := vscode.OpenParams{ - Workspace: workspace, - Folder: folder, - NewWindow: vscode.Options.GetValue(ideOptions, vscode.OpenNewWindow) == config.BoolTrue, - Flavor: flavorMap[ideName], - Log: logger, - } - - return vscode.Open(ctx, params) -} - -func openJetBrains( - ideName, folder, workspace, user string, - ideOptions map[string]config.OptionValue, - logger log.Logger, -) error { - type jetbrainsFactory func() interface{ OpenGateway(string, string) error } - - jetbrainsMap := map[string]jetbrainsFactory{ - string(config.IDERustRover): func() interface{ OpenGateway(string, string) error } { - return jetbrains.NewRustRoverServer(user, ideOptions, logger) - }, - string(config.IDEGoland): func() interface{ OpenGateway(string, string) error } { - return jetbrains.NewGolandServer(user, ideOptions, logger) - }, - string(config.IDEPyCharm): func() interface{ OpenGateway(string, string) error } { - return jetbrains.NewPyCharmServer(user, ideOptions, logger) - }, - string(config.IDEPhpStorm): func() interface{ OpenGateway(string, string) error } { - return jetbrains.NewPhpStorm(user, ideOptions, logger) - }, - string(config.IDEIntellij): func() interface{ OpenGateway(string, string) error } { - return jetbrains.NewIntellij(user, ideOptions, logger) - }, - string(config.IDECLion): func() interface{ OpenGateway(string, string) error } { - return jetbrains.NewCLionServer(user, ideOptions, logger) - }, - string(config.IDERider): func() interface{ OpenGateway(string, string) error } { - return jetbrains.NewRiderServer(user, ideOptions, logger) - }, - string(config.IDERubyMine): func() interface{ OpenGateway(string, string) error } { - return jetbrains.NewRubyMineServer(user, ideOptions, logger) - }, - string(config.IDEWebStorm): func() interface{ OpenGateway(string, string) error } { - return jetbrains.NewWebStormServer(user, ideOptions, logger) - }, - string(config.IDEDataSpell): func() interface{ OpenGateway(string, string) error } { - return jetbrains.NewDataSpellServer(user, ideOptions, logger) - }, - } - - if factory, ok := jetbrainsMap[ideName]; ok { - return factory().OpenGateway(folder, workspace) - } - return fmt.Errorf("unknown JetBrains IDE: %s", ideName) -} - -// startVSCodeInBrowser, startJupyterNotebookInBrowser, startRStudioInBrowser, startFleet -// are moved here verbatim from cmd/up.go. They call into pkg/tunnel for browser tunnels -// and pkg/gpg for GPG forwarding (after those packages are extracted in Tasks 3-4). -// Until then, they import the functions directly. The full code for these functions -// is identical to cmd/up.go lines 840-993 and 995-1056. -``` - -**Note for implementer:** The `startVSCodeInBrowser`, `startJupyterNotebookInBrowser`, `startRStudioInBrowser`, and `startFleet` functions should be copied verbatim from `cmd/up.go` into `pkg/ide/opener/opener.go`. This temporarily duplicates `performGpgForwarding`, `startBrowserTunnel`, and `createSSHCommand` — they exist in both `cmd/up.go` (still used by other callers) and `pkg/ide/opener/`. Tasks 3-4 resolve this: `performGpgForwarding` moves to `pkg/gpg`, `startBrowserTunnel` and `createSSHCommand` move to `pkg/tunnel`, and `pkg/ide/opener` is updated to import from those packages. The duplication lives for exactly 2 commits. - -- [ ] **Step 4: Run tests to verify they pass** - -Run: `cd pkg/ide/opener && go test -v ./...` -Expected: PASS - -- [ ] **Step 5: Update `cmd/up.go` to use `pkg/ide/opener`** - -Replace the `openIDE` method in `cmd/up.go`: - -```go -// Old: -func (cmd *UpCmd) openIDE( - ctx context.Context, - devPodConfig *config.Config, - client client2.BaseWorkspaceClient, - wctx *workspaceContext, - log log.Logger, -) error { - if !cmd.OpenIDE { - return nil - } - ideConfig := client.WorkspaceConfig().IDE - o := newIDEOpener(cmd, devPodConfig, client, wctx, log) - return o.open(ctx, ideConfig.Name, ideConfig.Options) -} - -// New: -func (cmd *UpCmd) openIDE( - ctx context.Context, - devPodConfig *config.Config, - client client2.BaseWorkspaceClient, - wctx *workspaceContext, - log log.Logger, -) error { - if !cmd.OpenIDE { - return nil - } - ideConfig := client.WorkspaceConfig().IDE - return opener.Open(ctx, ideConfig.Name, ideConfig.Options, opener.Params{ - GPGAgentForwarding: cmd.GPGAgentForwarding, - SSHAuthSockID: cmd.SSHAuthSockID, - GitSSHSigningKey: cmd.GitSSHSigningKey, - DevPodConfig: devPodConfig, - Client: client, - User: wctx.user, - Result: wctx.result, - Log: log, - }) -} -``` - -Remove from `cmd/up.go`: `ideOpener` struct, `newIDEOpener`, `ideOpener.open`, `ideOpener.openVSCodeFlavor`, `ideOpener.openJetBrains`, `startVSCodeInBrowser`, `startJupyterNotebookInBrowser`, `startRStudioInBrowser`, `startFleet`, `parseAddressAndPort`. - -Add import: `"github.com/skevetter/devpod/pkg/ide/opener"` - -Remove now-unused imports: IDE-specific packages (`vscode`, `jetbrains`, `jupyter`, `openvscode`, `rstudio`, `fleet`, `zed`), `"net"`, `open2`, `"github.com/skratchdot/open-golang/open"`. - -- [ ] **Step 6: Verify build and tests pass** - -Run: `go build ./... && go test ./cmd/... ./pkg/ide/opener/...` -Expected: BUILD OK, PASS - -- [ ] **Step 7: Commit** - -```bash -git add pkg/ide/opener/ cmd/up.go -git commit -m "$(cat <<'EOF' -refactor: extract pkg/ide/opener for IDE launch dispatch - -Move IDE opening logic (VSCode, JetBrains, browser-based IDEs, Fleet, Zed) -from cmd/up.go into a dedicated pkg/ide/opener package with unit tests. -EOF -)" -``` - ---- - -### Task 3: Extract `pkg/gpg/` - -**Files:** -- Create: `pkg/gpg/forward.go` -- Create: `pkg/gpg/forward_test.go` -- Modify: `pkg/ide/opener/opener.go` (update imports to use `pkg/gpg`) - -- [ ] **Step 1: Write failing test** - -Create `pkg/gpg/forward_test.go`: - -```go -package gpg - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestBuildForwardCommand(t *testing.T) { - args := buildForwardArgs("/usr/bin/devpod", "root", "test-context", "test-workspace") - assert.Contains(t, args, "ssh") - assert.Contains(t, args, "--gpg-agent-forwarding=true") - assert.Contains(t, args, "--agent-forwarding=true") - assert.Contains(t, args, "--user") - assert.Contains(t, args, "root") - assert.Contains(t, args, "--context") - assert.Contains(t, args, "test-context") - assert.Contains(t, args, "test-workspace") - assert.Contains(t, args, "--command") - assert.Contains(t, args, "sleep infinity") -} -``` - -- [ ] **Step 2: Run test to verify it fails** - -Run: `cd pkg/gpg && go test -v ./...` -Expected: FAIL - -- [ ] **Step 3: Create `pkg/gpg/forward.go`** - -```go -package gpg - -import ( - "os" - "os/exec" - - client2 "github.com/skevetter/devpod/pkg/client" - devssh "github.com/skevetter/devpod/pkg/ssh" - "github.com/skevetter/log" -) - -// ForwardAgent starts a background SSH connection that forwards the local -// GPG agent to the remote workspace. -func ForwardAgent(client client2.BaseWorkspaceClient, logger log.Logger) error { - logger.Debug("gpg forwarding enabled, performing immediately") - - execPath, err := os.Executable() - if err != nil { - return err - } - - remoteUser, err := devssh.GetUser( - client.WorkspaceConfig().ID, - client.WorkspaceConfig().SSHConfigPath, - client.WorkspaceConfig().SSHConfigIncludePath, - ) - if err != nil { - remoteUser = "root" - } - - logger.Info("forwarding gpg-agent") - - args := buildForwardArgs(execPath, remoteUser, client.Context(), client.Workspace()) - - go func() { - err = exec.Command(execPath, args...).Run() - if err != nil { - logger.Error("failure in forwarding gpg-agent") - } - }() - - return nil -} - -func buildForwardArgs(execPath, user, context, workspace string) []string { - return []string{ - "ssh", - "--gpg-agent-forwarding=true", - "--agent-forwarding=true", - "--start-services=true", - "--user", - user, - "--context", - context, - workspace, - "--log-output=raw", - "--command", "sleep infinity", - } -} -``` - -- [ ] **Step 4: Run test to verify it passes** - -Run: `cd pkg/gpg && go test -v ./...` -Expected: PASS - -- [ ] **Step 5: Update `pkg/ide/opener/opener.go` to use `pkg/gpg`** - -Replace inline `performGpgForwarding` calls in the browser IDE functions with `gpg.ForwardAgent`. Add import `"github.com/skevetter/devpod/pkg/gpg"`. Remove `performGpgForwarding` from wherever it currently lives. - -- [ ] **Step 6: Remove `performGpgForwarding` from `cmd/up.go`** - -Delete the function (originally lines 1555-1600). Update imports. - -- [ ] **Step 7: Verify build and tests pass** - -Run: `go build ./... && go test ./pkg/gpg/... ./pkg/ide/opener/... ./cmd/...` -Expected: BUILD OK, PASS - -- [ ] **Step 8: Commit** - -```bash -git add pkg/gpg/ pkg/ide/opener/ cmd/up.go -git commit -m "$(cat <<'EOF' -refactor: extract pkg/gpg for GPG agent forwarding - -Move GPG agent forwarding logic into a dedicated pkg/gpg package. -Update pkg/ide/opener to use it for browser-based IDEs. -EOF -)" -``` - ---- - -### Task 4: Move browser tunnel and backhaul into `pkg/tunnel/` - -**Files:** -- Create: `pkg/tunnel/browser.go` -- Create: `pkg/tunnel/browser_test.go` -- Modify: `pkg/ide/opener/opener.go` (update to call `tunnel.StartBrowserTunnel`) -- Modify: `cmd/up.go` (remove `startBrowserTunnel`, `setupBackhaul`, `createSSHCommand`) - -- [ ] **Step 1: Write failing test** - -Create `pkg/tunnel/browser_test.go`: - -```go -package tunnel - -import ( - "testing" - - "github.com/sirupsen/logrus" - "github.com/stretchr/testify/assert" -) - -func TestCreateSSHCommandArgs(t *testing.T) { - tests := []struct { - name string - context string - workspace string - debug bool - extraArgs []string - wantArgs []string - }{ - { - name: "basic command", - context: "default", - workspace: "my-workspace", - debug: false, - extraArgs: []string{"--stdio"}, - wantArgs: []string{ - "ssh", - "--user=root", - "--agent-forwarding=false", - "--start-services=false", - "--context", - "default", - "my-workspace", - "--stdio", - }, - }, - { - name: "with debug", - context: "default", - workspace: "my-workspace", - debug: true, - extraArgs: nil, - wantArgs: []string{ - "ssh", - "--user=root", - "--agent-forwarding=false", - "--start-services=false", - "--context", - "default", - "my-workspace", - "--debug", - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - args := buildSSHCommandArgs(tt.context, tt.workspace, tt.debug, tt.extraArgs) - assert.Equal(t, tt.wantArgs, args) - }) - } -} -``` - -- [ ] **Step 2: Run test to verify it fails** - -Run: `cd pkg/tunnel && go test -v -run TestCreateSSHCommandArgs` -Expected: FAIL -- `buildSSHCommandArgs` undefined - -- [ ] **Step 3: Create `pkg/tunnel/browser.go`** - -Move from `cmd/up.go`: -- `startBrowserTunnel` -> exported as `StartBrowserTunnel` -- `setupBackhaul` -> exported as `SetupBackhaul` -- `createSSHCommand` -> exported as `CreateSSHCommand` (used by `pkg/ide/opener` for Fleet) -- Extract `buildSSHCommandArgs` as a testable pure function - -```go -package tunnel - -import ( - "context" - "fmt" - "io" - "os" - "os/exec" - - "github.com/sirupsen/logrus" - client2 "github.com/skevetter/devpod/pkg/client" - "github.com/skevetter/devpod/pkg/client/clientimplementation" - "github.com/skevetter/devpod/pkg/config" - devssh "github.com/skevetter/devpod/pkg/ssh" - "github.com/skevetter/log" - "golang.org/x/crypto/ssh" -) - -// BrowserTunnelParams holds parameters for starting a browser-based IDE tunnel. -type BrowserTunnelParams struct { - DevPodConfig *config.Config - Client client2.BaseWorkspaceClient - User string - TargetURL string - ForwardPorts bool - ExtraPorts []string - AuthSockID string - GitSSHSigningKey string - Log log.Logger -} - -// StartBrowserTunnel starts an SSH tunnel for browser-based IDEs. -func StartBrowserTunnel(ctx context.Context, params BrowserTunnelParams) error { - // Setup backhaul if authSockID is set - if params.AuthSockID != "" { - go func() { - if err := SetupBackhaul(params.Client, params.AuthSockID, params.Log); err != nil { - params.Log.Error("Failed to setup backhaul SSH connection: ", err) - } - }() - } - - // handle daemon client directly - daemonClient, ok := params.Client.(client2.DaemonClient) - if ok { - toolClient, _, err := daemonClient.SSHClients(ctx, params.User) - if err != nil { - return err - } - defer func() { _ = toolClient.Close() }() - - err = clientimplementation.StartServicesDaemon( - ctx, - clientimplementation.StartServicesDaemonOptions{ - DevPodConfig: params.DevPodConfig, - Client: daemonClient, - SSHClient: toolClient, - User: params.User, - Log: params.Log, - ForwardPorts: params.ForwardPorts, - ExtraPorts: params.ExtraPorts, - }, - ) - if err != nil { - return err - } - <-ctx.Done() - return nil - } - - err := NewTunnel( - ctx, - func(ctx context.Context, stdin io.Reader, stdout io.Writer) error { - writer := params.Log.Writer(logrus.DebugLevel, false) - defer func() { _ = writer.Close() }() - - cmd, err := CreateSSHCommand(ctx, params.Client, params.Log, []string{ - "--log-output=raw", - fmt.Sprintf("--reuse-ssh-auth-sock=%s", params.AuthSockID), - "--stdio", - }) - if err != nil { - return err - } - cmd.Stdout = stdout - cmd.Stdin = stdin - cmd.Stderr = writer - return cmd.Run() - }, - func(ctx context.Context, containerClient *ssh.Client) error { - streamLogger, ok := params.Log.(*log.StreamLogger) - if ok { - streamLogger.JSON(logrus.InfoLevel, map[string]string{ - "url": params.TargetURL, - "done": "true", - }) - } - - configureDockerCredentials := params.DevPodConfig.ContextOption( - config.ContextOptionSSHInjectDockerCredentials, - ) == config.BoolTrue - configureGitCredentials := params.DevPodConfig.ContextOption( - config.ContextOptionSSHInjectGitCredentials, - ) == config.BoolTrue - configureGitSSHSignatureHelper := params.DevPodConfig.ContextOption( - config.ContextOptionGitSSHSignatureForwarding, - ) == config.BoolTrue - - err := RunServices( - ctx, - RunServicesOptions{ - DevPodConfig: params.DevPodConfig, - ContainerClient: containerClient, - User: params.User, - ForwardPorts: params.ForwardPorts, - ExtraPorts: params.ExtraPorts, - PlatformOptions: nil, - Workspace: params.Client.WorkspaceConfig(), - ConfigureDockerCredentials: configureDockerCredentials, - ConfigureGitCredentials: configureGitCredentials, - ConfigureGitSSHSignatureHelper: configureGitSSHSignatureHelper, - GitSSHSigningKey: params.GitSSHSigningKey, - Log: params.Log, - }, - ) - if err != nil { - return fmt.Errorf("run credentials server in browser tunnel: %w", err) - } - - <-ctx.Done() - return nil - }, - ) - return err -} - -// SetupBackhaul sets up a long-running SSH connection to keep an AUTH_SOCK alive. -func SetupBackhaul(client client2.BaseWorkspaceClient, authSockID string, logger log.Logger) error { - execPath, err := os.Executable() - if err != nil { - return err - } - - remoteUser, err := devssh.GetUser( - client.WorkspaceConfig().ID, - client.WorkspaceConfig().SSHConfigPath, - client.WorkspaceConfig().SSHConfigIncludePath, - ) - if err != nil { - remoteUser = "root" - } - - args := []string{ - "ssh", - "--agent-forwarding=true", - fmt.Sprintf("--reuse-ssh-auth-sock=%s", authSockID), - "--start-services=false", - "--user", - remoteUser, - "--context", - client.Context(), - client.Workspace(), - "--log-output=raw", - "--command", - "while true; do sleep 6000000; done", - } - - if logger.GetLevel() == logrus.DebugLevel { - args = append(args, "--debug") - } - - logger.Info("Setting up backhaul SSH connection") - - writer := logger.Writer(logrus.InfoLevel, false) - dotCmd := exec.Command(execPath, args...) - dotCmd.Stdout = writer - dotCmd.Stderr = writer - - if err := dotCmd.Run(); err != nil { - return err - } - - logger.Infof("Done setting up backhaul") - return nil -} - -// CreateSSHCommand builds an exec.Cmd for SSH into the workspace. -func CreateSSHCommand( - ctx context.Context, - client client2.BaseWorkspaceClient, - logger log.Logger, - extraArgs []string, -) (*exec.Cmd, error) { - execPath, err := os.Executable() - if err != nil { - return nil, err - } - - args := buildSSHCommandArgs( - client.Context(), - client.Workspace(), - logger.GetLevel() == logrus.DebugLevel, - extraArgs, - ) - - return exec.CommandContext(ctx, execPath, args...), nil -} - -func buildSSHCommandArgs(context, workspace string, debug bool, extraArgs []string) []string { - args := []string{ - "ssh", - "--user=root", - "--agent-forwarding=false", - "--start-services=false", - "--context", - context, - workspace, - } - if debug { - args = append(args, "--debug") - } - args = append(args, extraArgs...) - return args -} -``` - -- [ ] **Step 4: Run test to verify it passes** - -Run: `cd pkg/tunnel && go test -v -run TestCreateSSHCommandArgs` -Expected: PASS - -- [ ] **Step 5: Update `pkg/ide/opener/opener.go`** - -Replace inline `startBrowserTunnel` calls with `tunnel.StartBrowserTunnel`. Replace `createSSHCommand` calls (in `startFleet`) with `tunnel.CreateSSHCommand`. Add import `"github.com/skevetter/devpod/pkg/tunnel"`. - -- [ ] **Step 6: Remove from `cmd/up.go`** - -Delete: `startBrowserTunnel`, `setupBackhaul`, `createSSHCommand`. -Remove now-unused imports: `"io"`, `"golang.org/x/crypto/ssh"`. - -- [ ] **Step 7: Verify build and tests pass** - -Run: `go build ./... && go test ./pkg/tunnel/... ./pkg/ide/opener/... ./cmd/...` -Expected: BUILD OK, PASS - -- [ ] **Step 8: Commit** - -```bash -git add pkg/tunnel/browser.go pkg/tunnel/browser_test.go pkg/ide/opener/ cmd/up.go -git commit -m "$(cat <<'EOF' -refactor: move browser tunnel and backhaul into pkg/tunnel - -Move startBrowserTunnel, setupBackhaul, and createSSHCommand from cmd/up.go -into pkg/tunnel where they naturally belong alongside existing tunnel logic. -EOF -)" -``` - ---- - -### Task 5: Move provider update check into `pkg/workspace/` - -**Files:** -- Modify: `pkg/workspace/provider.go` (add CheckProviderUpdate, GetProInstance) -- Create: `pkg/workspace/provider_update_test.go` -- Modify: `cmd/up.go` (remove `checkProviderUpdate`, `getProInstance`) - -- [ ] **Step 1: Write failing tests** - -Create `pkg/workspace/provider_update_test.go`: - -```go -package workspace - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestShouldSkipProviderUpdate(t *testing.T) { - tests := []struct { - name string - currentVersion string - isDevVersion bool - isInternal bool - wantSkip bool - }{ - { - name: "dev version skips", - isDevVersion: true, - wantSkip: true, - }, - { - name: "internal source skips", - isInternal: true, - wantSkip: true, - }, - { - name: "same version skips", - currentVersion: "v0.5.0", - wantSkip: false, // handled by semver comparison, not this function - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := shouldSkipProviderUpdate(tt.isDevVersion, tt.isInternal) - assert.Equal(t, tt.wantSkip, got) - }) - } -} - -func TestGetProInstance_NilWhenNoInstances(t *testing.T) { - // GetProInstance with nil config should return nil gracefully - result := GetProInstance(nil, "some-provider", nil) - assert.Nil(t, result) -} -``` - -- [ ] **Step 2: Run tests to verify they fail** - -Run: `cd pkg/workspace && go test -v -run "TestShouldSkip|TestGetPro"` -Expected: FAIL -- functions not defined - -- [ ] **Step 3: Add functions to `pkg/workspace/provider.go`** - -Append to existing `pkg/workspace/provider.go`: - -```go -// CheckProviderUpdate ensures the provider version matches the pro instance version. -func CheckProviderUpdate( - devPodConfig *config.Config, - proInstance *provider2.ProInstance, - log log.Logger, -) error { - if version.GetVersion() == version.DevVersion { - log.Debugf("skipping provider upgrade check during development") - return nil - } - if proInstance == nil { - log.Debug("no pro instance available, skipping provider upgrade check") - return nil - } - - newVersion, err := platform.GetProInstanceDevPodVersion(proInstance) - if err != nil { - return fmt.Errorf("version for pro instance %s: %w", proInstance.Host, err) - } - - p, err := FindProvider(devPodConfig, proInstance.Provider, log) - if err != nil { - return fmt.Errorf("get provider config for pro provider %s: %w", proInstance.Provider, err) - } - if shouldSkipProviderUpdate( - p.Config.Version == version.DevVersion, - p.Config.Source.Internal, - ) { - return nil - } - - v1, err := semver.Parse(strings.TrimPrefix(newVersion, "v")) - if err != nil { - return fmt.Errorf("parse version %s: %w", newVersion, err) - } - v2, err := semver.Parse(strings.TrimPrefix(p.Config.Version, "v")) - if err != nil { - return fmt.Errorf("parse version %s: %w", p.Config.Version, err) - } - if v1.Compare(v2) == 0 { - return nil - } - log.Infof( - "New provider version available, attempting to update %s from %s to %s", - proInstance.Provider, - p.Config.Version, - newVersion, - ) - - providerSource, err := ResolveProviderSource(devPodConfig, proInstance.Provider, log) - if err != nil { - return fmt.Errorf("resolve provider source %s: %w", proInstance.Provider, err) - } - - splitted := strings.Split(providerSource, "@") - if len(splitted) == 0 { - return fmt.Errorf("no provider source found %s", providerSource) - } - providerSource = splitted[0] + "@" + newVersion - - _, err = UpdateProvider(devPodConfig, proInstance.Provider, providerSource, log) - if err != nil { - return fmt.Errorf("update provider %s: %w", proInstance.Provider, err) - } - - log.WithFields(logrus.Fields{ - "provider": proInstance.Provider, - }).Done("updated provider") - return nil -} - -func shouldSkipProviderUpdate(isDevVersion, isInternal bool) bool { - return isDevVersion || isInternal -} - -// GetProInstance finds the pro instance associated with a provider. -func GetProInstance( - devPodConfig *config.Config, - providerName string, - log log.Logger, -) *provider2.ProInstance { - if devPodConfig == nil { - return nil - } - - proInstances, err := ListProInstances(devPodConfig, log) - if err != nil { - return nil - } - if len(proInstances) == 0 { - return nil - } - - proInstance, ok := FindProviderProInstance(proInstances, providerName) - if !ok { - return nil - } - - return proInstance -} -``` - -Add necessary imports to `pkg/workspace/provider.go`: `"github.com/blang/semver/v4"`, `"github.com/sirupsen/logrus"`, `"github.com/skevetter/devpod/pkg/platform"`, `"github.com/skevetter/devpod/pkg/version"`. - -- [ ] **Step 4: Run tests to verify they pass** - -Run: `cd pkg/workspace && go test -v -run "TestShouldSkip|TestGetPro"` -Expected: PASS - -- [ ] **Step 5: Update `cmd/up.go`** - -In `prepareClient` method, replace: - -```go -// Old: -proInstance := getProInstance(devPodConfig, client.Provider(), logger) -err = checkProviderUpdate(devPodConfig, proInstance, logger) - -// New: -proInstance := workspace2.GetProInstance(devPodConfig, client.Provider(), logger) -err = workspace2.CheckProviderUpdate(devPodConfig, proInstance, logger) -``` - -Remove from `cmd/up.go`: `checkProviderUpdate`, `getProInstance` functions. -Remove now-unused imports: `"github.com/blang/semver/v4"`, `"github.com/skevetter/devpod/pkg/platform"`, `"github.com/skevetter/devpod/pkg/version"` (if no other uses). - -- [ ] **Step 6: Verify build and tests pass** - -Run: `go build ./... && go test ./pkg/workspace/... ./cmd/...` -Expected: BUILD OK, PASS - -- [ ] **Step 7: Commit** - -```bash -git add pkg/workspace/provider.go pkg/workspace/provider_update_test.go cmd/up.go -git commit -m "$(cat <<'EOF' -refactor: move provider update check into pkg/workspace - -Move checkProviderUpdate and getProInstance from cmd/up.go into -pkg/workspace where they naturally belong alongside FindProvider -and UpdateProvider. -EOF -)" -``` - ---- - -### Task 6: Final cleanup of `cmd/up.go` - -**Files:** -- Modify: `cmd/up.go` - -- [ ] **Step 1: Remove all dead imports** - -Run: `go build ./cmd/...` - -Fix any unused import errors. The following imports should now be removable from `cmd/up.go` (verify each): -- `"bytes"` (was used by `startFleet`) -- `"io"` (was used by `startBrowserTunnel`) -- `"net"` (was used by `parseAddressAndPort`) -- `"slices"` (was used by dotfiles) -- `"github.com/blang/semver/v4"` (was used by `checkProviderUpdate`) -- Various IDE sub-packages (`fleet`, `jetbrains`, `jupyter`, `openvscode`, `rstudio`, `vscode`, `zed`) -- `"github.com/skevetter/devpod/pkg/command"` (was used by `startFleet`) -- `open2 "github.com/skevetter/devpod/pkg/open"` (was used by browser IDEs) -- `"github.com/skevetter/devpod/pkg/platform"` (was used by `checkProviderUpdate`) -- `"github.com/skevetter/devpod/pkg/port"` (was used by `parseAddressAndPort`) -- `"github.com/skevetter/devpod/pkg/version"` (was used by `checkProviderUpdate`) -- `devssh "github.com/skevetter/devpod/pkg/ssh"` (was used by `createSSHCommand` and `setupDotfiles`) -- `"github.com/skratchdot/open-golang/open"` (was used by `startFleet`) -- `"golang.org/x/crypto/ssh"` (was used by `startBrowserTunnel`) - -- [ ] **Step 2: Verify the file is ~350-400 lines** - -Run: `wc -l cmd/up.go` -Expected: approximately 350-450 lines - -- [ ] **Step 3: Verify full build and all tests pass** - -Run: `go build ./... && go test ./...` -Expected: BUILD OK, all PASS - -- [ ] **Step 4: Commit** - -```bash -git add cmd/up.go -git commit -m "$(cat <<'EOF' -refactor: clean up cmd/up.go after extractions - -Remove dead imports and unused code after extracting dotfiles, IDE opener, -GPG forwarding, browser tunnel, and provider update logic into pkg/. -EOF -)" -``` - ---- - -## Verification Checklist - -After all 6 commits: - -- [ ] `go build ./...` passes -- [ ] `go test ./...` passes -- [ ] `go vet ./...` passes -- [ ] `cmd/up.go` is under 450 lines -- [ ] No circular imports (cmd/ -> pkg/, pkg/ -> pkg/, never pkg/ -> cmd/) -- [ ] All new packages have test files diff --git a/docs/superpowers/specs/2026-04-11-cmd-up-refactor-design.md b/docs/superpowers/specs/2026-04-11-cmd-up-refactor-design.md deleted file mode 100644 index d664c3f32..000000000 --- a/docs/superpowers/specs/2026-04-11-cmd-up-refactor-design.md +++ /dev/null @@ -1,182 +0,0 @@ -# Design: Extract cmd/up.go into pkg/ packages - -**Date:** 2026-04-11 -**Status:** Approved -**Approach:** Single PR, one commit per extracted package (Approach C) - -## Problem - -`cmd/up.go` is 1839 lines with 6 distinct responsibility areas packed into one file: -command definition, client preparation, devPodUp orchestration, IDE opening, -SSH/tunnel/credentials setup, and dotfiles/utilities. This makes it hard to -navigate, test, and modify. - -## Goal - -Break `cmd/up.go` into focused `pkg/` packages, each with unit tests. The file -should shrink to ~350-400 lines of command-level orchestration. All behavior must -be preserved. - -## Package Extractions - -### 1. `pkg/dotfiles/` (new) - -**Source lines:** 1384-1553 -**Functions moved:** `setupDotfiles`, `buildDotCmd`, `buildDotCmdAgentArguments`, -`extractKeysFromEnvKeyValuePairs`, `collectDotfilesScriptEnvKeyvaluePairs` - -**Exported API:** - -```go -package dotfiles - -type SetupParams struct { - Source string - Script string - EnvFiles []string - EnvKeyValues []string - Client client.BaseWorkspaceClient - DevPodConfig *config.Config - Log log.Logger -} - -func Setup(params SetupParams) error -``` - -**Tests:** Table-driven tests for `extractKeysFromEnvKeyValuePairs`, -`collectDotfilesScriptEnvKeyvaluePairs`, and `buildDotCmdAgentArguments` -(pure functions). Integration-style test for `Setup` with a mock client. - -### 2. `pkg/ide/opener/` (new) - -**Source lines:** 400-583, 840-1056 -**Functions moved:** `ideOpener` struct and methods, `openVSCodeFlavor`, -`openJetBrains`, `startVSCodeInBrowser`, `startJupyterNotebookInBrowser`, -`startRStudioInBrowser`, `startFleet`, `parseAddressAndPort` - -**Exported API:** - -```go -package opener - -type Params struct { - GPGAgentForwarding bool - SSHAuthSockID string - GitSSHSigningKey string - DevPodConfig *config.Config - Client client.BaseWorkspaceClient - User string - Workdir string - Result *config2.Result - Log log.Logger -} - -func Open(ctx context.Context, ideName string, ideOptions map[string]config.OptionValue, params Params) error -``` - -**Tests:** `parseAddressAndPort` table-driven tests. IDE dispatch routing test -(verify correct branch taken per IDE name). - -### 3. `pkg/gpg/` (new) - -**Source lines:** 1555-1600 -**Functions moved:** `performGpgForwarding` - -**Exported API:** - -```go -package gpg - -func ForwardAgent(client client.BaseWorkspaceClient, log log.Logger) error -``` - -**Tests:** Verify command construction arguments. - -### 4. Extend `pkg/tunnel/` (browser tunnel + backhaul) - -**Source lines:** 1089-1263 -**Functions moved:** `startBrowserTunnel`, `setupBackhaul`, `createSSHCommand` -(internal helper) - -**New exports:** - -```go -// In pkg/tunnel/ - -type BrowserTunnelParams struct { - DevPodConfig *config.Config - Client client.BaseWorkspaceClient - User string - TargetURL string - ForwardPorts bool - ExtraPorts []string - AuthSockID string - GitSSHSigningKey string - Log log.Logger -} - -func StartBrowserTunnel(ctx context.Context, params BrowserTunnelParams) error - -func SetupBackhaul(client client.BaseWorkspaceClient, authSockID string, log log.Logger) error -``` - -**Tests:** Unit test for SSH command argument construction. - -### 5. Move provider update check to `pkg/workspace/` - -**Source lines:** 1602-1693 -**Functions moved:** `checkProviderUpdate`, `getProInstance` - -**New exports:** - -```go -// In pkg/workspace/ - -func CheckProviderUpdate(devPodConfig *config.Config, proInstance *provider2.ProInstance, log log.Logger) error - -func GetProInstance(devPodConfig *config.Config, providerName string, log log.Logger) *provider2.ProInstance -``` - -**Tests:** Table-driven test for version comparison logic (dev version skip, -same version skip, upgrade path). - -## What stays in cmd/up.go (~350-400 lines) - -- `UpCmd` struct, `NewUpCmd`, flag registration methods (~180 lines) -- `execute`, `validate`, `prepareClient` (command orchestration) -- `Run`, `prepareWorkspace`, `executeDevPodUp`, `configureWorkspace`, `openIDE` - (thin orchestration calling into pkg/) -- `devPodUp`, `devPodUpMachine`, `devPodUpProxy`, `devPodUpDaemon` (client - dispatch, tightly coupled to cobra context) -- `WithSignals`, `validatePodmanFlags`, `isValidMapping` (command utilities) -- `mergeDevPodUpOptions`, `mergeEnvFromFiles` (option merging) - -## Commit Sequence - -1. `refactor: extract pkg/dotfiles for dotfiles setup` -2. `refactor: extract pkg/ide/opener for IDE launch dispatch` -3. `refactor: extract pkg/gpg for GPG agent forwarding` -4. `refactor: move browser tunnel and backhaul into pkg/tunnel` -5. `refactor: move provider update check into pkg/workspace` -6. `refactor: clean up cmd/up.go after extractions` - -Each commit moves a responsibility area out, adds tests, and updates -`cmd/up.go` to call the new package. No intermediate broken state. - -## Cross-cutting Dependencies - -`createSSHCommand` is used by `startBrowserTunnel`, `setupBackhaul`, and -`startFleet` (IDE opener). It moves to `pkg/tunnel/` as an internal helper, -and `pkg/ide/opener/` imports `pkg/tunnel/` to reuse it (or it gets promoted -to an exported function in `pkg/tunnel/`). Direction is still pkg/ -> pkg/, -no circular risk. - -## Risks and Mitigations - -- **Circular imports:** The extracted packages depend on `pkg/client`, - `pkg/config`, `pkg/ssh` etc. but not on `cmd/`. Direction is always - cmd/ -> pkg/, never reverse. No circular risk. -- **Behavior regression:** Pure refactor with no logic changes. Existing - integration tests plus new unit tests provide coverage. -- **Large PR:** Mitigated by clean per-commit structure. Each commit is - independently reviewable. diff --git a/pkg/tunnel/browser.go b/pkg/tunnel/browser.go index a5c8221ad..1d55bc0af 100644 --- a/pkg/tunnel/browser.go +++ b/pkg/tunnel/browser.go @@ -136,7 +136,7 @@ func SetupBackhaul(client client2.BaseWorkspaceClient, authSockID string, logger } //nolint:gosec // execPath is the current binary, arguments are controlled - dotCmd := exec.Command( + backhaulCmd := exec.Command( execPath, "ssh", "--agent-forwarding=true", @@ -153,17 +153,17 @@ func SetupBackhaul(client client2.BaseWorkspaceClient, authSockID string, logger ) if logger.GetLevel() == logrus.DebugLevel { - dotCmd.Args = append(dotCmd.Args, "--debug") + backhaulCmd.Args = append(backhaulCmd.Args, "--debug") } logger.Info("Setting up backhaul SSH connection") writer := logger.Writer(logrus.InfoLevel, false) - dotCmd.Stdout = writer - dotCmd.Stderr = writer + backhaulCmd.Stdout = writer + backhaulCmd.Stderr = writer - err = dotCmd.Run() + err = backhaulCmd.Run() if err != nil { return err } diff --git a/pkg/tunnel/browser_test.go b/pkg/tunnel/browser_test.go index 6ead5c258..dd626187a 100644 --- a/pkg/tunnel/browser_test.go +++ b/pkg/tunnel/browser_test.go @@ -6,6 +6,13 @@ import ( "github.com/stretchr/testify/assert" ) +func baseSSHArgs(ctx, ws string) []string { + return []string{ + "ssh", "--user=root", "--agent-forwarding=false", + "--start-services=false", "--context", ctx, ws, + } +} + func TestBuildSSHCommandArgs(t *testing.T) { tests := []struct { name string @@ -16,51 +23,23 @@ func TestBuildSSHCommandArgs(t *testing.T) { expected []string }{ { - name: "basic", - context: "default", - workspace: "my-workspace", - expected: []string{ - "ssh", - "--user=root", - "--agent-forwarding=false", - "--start-services=false", - "--context", - "default", - "my-workspace", - }, + name: "basic", context: "default", workspace: "my-workspace", + expected: baseSSHArgs("default", "my-workspace"), }, { - name: "with debug", - context: "default", - workspace: "my-workspace", - debug: true, - expected: []string{ - "ssh", - "--user=root", - "--agent-forwarding=false", - "--start-services=false", - "--context", - "default", - "my-workspace", - "--debug", - }, + name: "with debug", context: "default", workspace: "my-workspace", + debug: true, + expected: append(baseSSHArgs("default", "my-workspace"), "--debug"), }, { - name: "with extra args", - context: "prod", - workspace: "ws", + name: "with extra args", context: "prod", workspace: "ws", extraArgs: []string{"--stdio", "--log-output=raw"}, - expected: []string{ - "ssh", - "--user=root", - "--agent-forwarding=false", - "--start-services=false", - "--context", - "prod", - "ws", - "--stdio", - "--log-output=raw", - }, + expected: append(baseSSHArgs("prod", "ws"), "--stdio", "--log-output=raw"), + }, + { + name: "with debug and extra args", context: "default", workspace: "my-workspace", + debug: true, extraArgs: []string{"--stdio"}, + expected: append(baseSSHArgs("default", "my-workspace"), "--debug", "--stdio"), }, } diff --git a/pkg/workspace/provider_update_test.go b/pkg/workspace/provider_update_test.go index ddcd414a5..4f1ad78c4 100644 --- a/pkg/workspace/provider_update_test.go +++ b/pkg/workspace/provider_update_test.go @@ -47,11 +47,29 @@ func TestShouldSkipProviderUpdate(t *testing.T) { } } -func TestGetProInstance_EmptyProviderName(t *testing.T) { - // GetProInstance returns nil when provider name doesn't match any pro instance. - // We can't easily test with a real config without disk I/O, but we can verify - // that shouldSkipProviderUpdate is the composable unit for logic testing. - // This is a placeholder for integration-level testing. - result := shouldSkipProviderUpdate(false, false) - assert.False(t, result, "regular provider should not be skipped") +func TestProviderVersionNeedsUpdate(t *testing.T) { + tests := []struct { + name, newVer, curVer string + expected, expectErr bool + }{ + {"same version", "v0.5.0", "v0.5.0", false, false}, + {"newer version", "v0.6.0", "v0.5.0", true, false}, + {"older version (downgrade)", "v0.4.0", "v0.5.0", true, false}, + {"mixed v prefix", "v0.6.0", "0.5.0", true, false}, + {"patch difference", "v1.2.4", "v1.2.3", true, false}, + {"invalid new version", "not-a-version", "v0.5.0", false, true}, + {"invalid current version", "v0.5.0", "not-a-version", false, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := providerVersionNeedsUpdate(tt.newVer, tt.curVer) + if tt.expectErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.Equal(t, tt.expected, result) + } + }) + } } From c1290ebb4e33a28db7520883f28c047dbe0697ae Mon Sep 17 00:00:00 2001 From: Samuel K Date: Sun, 12 Apr 2026 00:36:58 -0500 Subject: [PATCH 8/8] fix: address GPG forwarding nitpicks from review - Fix error variable shadowing in ForwardAgent goroutine - Include actual error in gpg-agent forwarding failure log - Use assert.Equal for strict argument ordering in GPG test --- pkg/gpg/forward.go | 5 ++--- pkg/gpg/forward_test.go | 24 +++++++++++++----------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/pkg/gpg/forward.go b/pkg/gpg/forward.go index d919f8397..cbbe1bb98 100644 --- a/pkg/gpg/forward.go +++ b/pkg/gpg/forward.go @@ -33,9 +33,8 @@ func ForwardAgent(client client2.BaseWorkspaceClient, logger log.Logger) error { go func() { //nolint:gosec // execPath comes from os.Executable() - err = exec.Command(execPath, args...).Run() - if err != nil { - logger.Error("failure in forwarding gpg-agent") + if runErr := exec.Command(execPath, args...).Run(); runErr != nil { + logger.Errorf("failure in forwarding gpg-agent: %v", runErr) } }() diff --git a/pkg/gpg/forward_test.go b/pkg/gpg/forward_test.go index e6559e4a9..d8a599dec 100644 --- a/pkg/gpg/forward_test.go +++ b/pkg/gpg/forward_test.go @@ -7,15 +7,17 @@ import ( ) func TestBuildForwardArgs(t *testing.T) { - args := buildForwardArgs("root", "test-context", "test-workspace") - assert.Contains(t, args, "ssh") - assert.Contains(t, args, "--gpg-agent-forwarding=true") - assert.Contains(t, args, "--agent-forwarding=true") - assert.Contains(t, args, "--user") - assert.Contains(t, args, "root") - assert.Contains(t, args, "--context") - assert.Contains(t, args, "test-context") - assert.Contains(t, args, "test-workspace") - assert.Contains(t, args, "--command") - assert.Contains(t, args, "sleep infinity") + got := buildForwardArgs("root", "test-context", "test-workspace") + expected := []string{ + "ssh", + "--gpg-agent-forwarding=true", + "--agent-forwarding=true", + "--start-services=true", + "--user", "root", + "--context", "test-context", + "test-workspace", + "--log-output=raw", + "--command", "sleep infinity", + } + assert.Equal(t, expected, got) }