From 5265a49f69086ab918e17de31ca363c8b2eb7236 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Thu, 9 Jul 2026 16:29:02 -0500 Subject: [PATCH 01/11] fix(flatpak): run CLI on host so podman/docker resolve Inside the Flatpak sandbox the container runtime (docker/podman), git and ssh are not on $PATH and the host filesystem is only partially visible, so workspace creation failed with "podman: executable file not found in $PATH" (#634). Re-execute the CLI on the host via `flatpak-spawn --host` at command entry, mirroring pkg/agent.rerunAsRoot. The `internal` subtree is excluded because it owns the daemon whose Unix socket lives in the per-sandbox /tmp and must stay co-located with the desktop's daemon client. The re-exec is a no-op outside a sandbox, so the CLI is unchanged on macOS, Windows and native Linux. Also refactor Execute() into a thin os.Exit(run()) wrapper so deferred telemetry flushing runs on every exit path, and switch the exit-code error handling to errors.AsType so a wrapped ExitError still propagates its status. Fixes #634 --- cmd/root.go | 97 +++++++++++++++++++++-------------- cmd/root_test.go | 36 +++++++++++++ desktop/flatpak/devsy-wrapper | 3 +- pkg/flatpak/flatpak.go | 63 +++++++++++++++++++++++ pkg/flatpak/flatpak_test.go | 81 +++++++++++++++++++++++++++++ 5 files changed, 239 insertions(+), 41 deletions(-) create mode 100644 cmd/root_test.go create mode 100644 pkg/flatpak/flatpak.go create mode 100644 pkg/flatpak/flatpak_test.go diff --git a/cmd/root.go b/cmd/root.go index 2b4c2ee05..30cebcef9 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -24,6 +24,7 @@ import ( "github.com/devsy-org/devsy/pkg/config" cliErrors "github.com/devsy-org/devsy/pkg/errors" "github.com/devsy-org/devsy/pkg/exitcode" + "github.com/devsy-org/devsy/pkg/flatpak" "github.com/devsy-org/devsy/pkg/log" "github.com/devsy-org/devsy/pkg/telemetry" "github.com/devsy-org/devsy/pkg/version" @@ -48,6 +49,8 @@ const ( // feature is not ready for general use; set DEVSY_PRO_ENABLED=true to // expose it (e.g. for internal testing). envProEnabled = "DEVSY_PRO_ENABLED" + + internalCommand = "internal" ) func proEnabled() bool { @@ -61,23 +64,30 @@ func isMachineLogFormat(format string) bool { return format == logOutputJSON || format == logOutputLogfmt } -// Execute adds all child commands to the root command and sets flags appropriately. -// This is called by main.main(). It only needs to happen once to the rootCmd. func Execute() { - rootCmd, globalFlags := BuildRoot() + os.Exit(run()) +} - // Bootstrap pre-Execute so subcommands that override PersistentPreRunE - // without chaining (e.g. pro, agent) still get telemetry. +// run builds and executes the root command, returning the process exit code. +func run() int { + rootCmd, globalFlags := BuildRoot() target := rootCmd if found, _, findErr := rootCmd.Find(os.Args[1:]); findErr == nil && found != nil { target = found } collector := telemetry.BootstrapCLI(target) rootCmd.SetContext(telemetry.WithCollector(gocontext.Background(), collector)) + defer func() { collector.Flush() }() + if topLevelCommand(target) != internalCommand { + if shouldExit, err := flatpak.ReexecOnHost(); err != nil { + collector.RecordCLI(err) + return exitCodeForError(err, globalFlags) + } else if shouldExit { + return 0 + } + } err := rootCmd.Execute() - - // Re-apply opt-out post-Execute for the same PreRunE-bypass case. if devsyConfig, cfgErr := config.LoadConfig( globalFlags.Context, globalFlags.Provider, @@ -86,45 +96,54 @@ func Execute() { } collector.RecordCLI(err) - collector.Flush() if err != nil { - //nolint:all - if sshExitErr, ok := err.(*ssh.ExitError); ok { - log.Errorf("SSH command failed with exit code %d", sshExitErr.ExitStatus()) - os.Exit(sshExitErr.ExitStatus()) - } + return exitCodeForError(err, globalFlags) + } + return 0 +} - //nolint:all - if execExitErr, ok := err.(*exec.ExitError); ok { - log.Errorf("Command failed with exit code %d", execExitErr.ExitCode()) - os.Exit(execExitErr.ExitCode()) +func topLevelCommand(cmd *cobra.Command) string { + if cmd == nil { + return "" + } + for cmd.HasParent() { + if !cmd.Parent().HasParent() { + return cmd.Name() } + cmd = cmd.Parent() + } + return "" +} - cliErr := cliErrors.Classify(err, cliErrors.ClassifyContext{}) - // Always emit the error through zap so the configured log encoder - // (json/logfmt/text) governs the wire format. JSONError preserves - // the full err.Error() chain in the top-level "msg" field and ships - // the structured CLIError under "cliError" for the desktop IPC. - log.JSONError(cliErr) - // In human-friendly text mode, follow up with hint/doc affordances - // that don't fit cleanly into the zap line. These extras are - // suppressed in machine-readable modes so log streams stay parseable. - if !isMachineLogFormat(globalFlags.LogOutput) { - if cliErr.Hint != "" { - fmt.Fprintf(os.Stderr, "Hint: %s\n", cliErr.Hint) - } - if cliErr.DocURL != "" { - fmt.Fprintf(os.Stderr, "See: %s\n", cliErr.DocURL) - } +// exitCodeForError emits err through the configured log encoder and returns the +// process exit code that reflects the failure. +func exitCodeForError(err error, globalFlags *flags.GlobalFlags) int { + if err == nil { + return 0 + } + if sshExitErr, ok := errors.AsType[*ssh.ExitError](err); ok { + log.Errorf("SSH command failed with exit code %d", sshExitErr.ExitStatus()) + return sshExitErr.ExitStatus() + } + if execExitErr, ok := errors.AsType[*exec.ExitError](err); ok { + log.Errorf("Command failed with exit code %d", execExitErr.ExitCode()) + return execExitErr.ExitCode() + } + + cliErr := cliErrors.Classify(err, cliErrors.ClassifyContext{}) + log.JSONError(cliErr) + if !isMachineLogFormat(globalFlags.LogOutput) { + if cliErr.Hint != "" { + fmt.Fprintf(os.Stderr, "Hint: %s\n", cliErr.Hint) } - // Signal workspace-not-found via a distinct exit code so parent - // processes (e.g. SetupBackhaul) can detect the registration race - // without parsing stderr. - if errors.Is(err, workspace.ErrWorkspaceNotFound) { - os.Exit(exitcode.WorkspaceNotFound) + if cliErr.DocURL != "" { + fmt.Fprintf(os.Stderr, "See: %s\n", cliErr.DocURL) } - os.Exit(1) } + if errors.Is(err, workspace.ErrWorkspaceNotFound) { + return exitcode.WorkspaceNotFound + } + return 1 } // BuildRoot constructs the root command and returns it alongside the parsed diff --git a/cmd/root_test.go b/cmd/root_test.go new file mode 100644 index 000000000..1568c02f2 --- /dev/null +++ b/cmd/root_test.go @@ -0,0 +1,36 @@ +package cmd + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestTopLevelCommand verifies the classifier that gates the Flatpak host +// re-exec: the daemon lives under `internal`, which must stay in-sandbox, while +// user-facing commands like `up`/`ssh` must be routed to the host. +func TestTopLevelCommand(t *testing.T) { + rootCmd, _ := BuildRoot() + + cases := []struct { + name string + args []string + want string + }{ + {"daemon stays in sandbox", []string{internalCommand, "daemon-local"}, internalCommand}, + {"nested internal command", []string{internalCommand, "ssh-server"}, internalCommand}, + {"workspace up routes to host", []string{"workspace", "up", "."}, "workspace"}, + {"workspace ssh routes to host", []string{"workspace", "ssh", "my-ws"}, "workspace"}, + {"provider list routes to host", []string{"provider", "list"}, "provider"}, + {"bare root", []string{}, ""}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + found, _, err := rootCmd.Find(tc.args) + require.NoError(t, err) + assert.Equal(t, tc.want, topLevelCommand(found)) + }) + } +} diff --git a/desktop/flatpak/devsy-wrapper b/desktop/flatpak/devsy-wrapper index e21e66cc3..3c65fada5 100755 --- a/desktop/flatpak/devsy-wrapper +++ b/desktop/flatpak/devsy-wrapper @@ -5,8 +5,7 @@ export ELECTRON_IS_DEV=0 # Determine if running inside Flatpak if [ -f "/.flatpak-info" ]; then - # Sync the devsy binary to a host-accessible location - DEVSY_HOME="${HOME}/.local/share/devsy" + DEVSY_HOME="${XDG_DATA_HOME:-${HOME}/.local/share}/devsy" mkdir -p "${DEVSY_HOME}" if [ -f /app/bin/devsy ]; then diff --git a/pkg/flatpak/flatpak.go b/pkg/flatpak/flatpak.go new file mode 100644 index 000000000..4d3490811 --- /dev/null +++ b/pkg/flatpak/flatpak.go @@ -0,0 +1,63 @@ +// Package flatpak handles running the CLI outside the Flatpak sandbox. +package flatpak + +import ( + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + + "github.com/devsy-org/devsy/pkg/log" +) + +var flatpakInfoPath = "/.flatpak-info" + +const flatpakSpawn = "flatpak-spawn" + +func InSandbox() bool { + _, err := os.Stat(flatpakInfoPath) + return err == nil +} + +func HostBinaryPath() string { + dataHome := os.Getenv("XDG_DATA_HOME") + if dataHome == "" { + dataHome = filepath.Join(os.Getenv("HOME"), ".local", "share") + } + return filepath.Join(dataHome, "devsy", "devsy") +} + +// ReexecOnHost re-executes the current process on the host via `flatpak-spawn --host`. +func ReexecOnHost() (bool, error) { + if !InSandbox() { + return false, nil + } + + hostBinary := HostBinaryPath() + if _, err := os.Stat(hostBinary); err != nil { + return false, fmt.Errorf( + "host devsy binary not found at %s; the Flatpak launcher should sync it on startup: %w", + hostBinary, err, + ) + } + + spawnArgs := append([]string{"--host", hostBinary}, os.Args[1:]...) + log.Debugf("re-executing on host via flatpak-spawn: args=%v", os.Args[1:]) + + //nolint:gosec + cmd := exec.Command(flatpakSpawn, spawnArgs...) + cmd.Stdin = os.Stdin + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + cmd.Env = os.Environ() + + if err := cmd.Run(); err != nil { + if exitErr, ok := errors.AsType[*exec.ExitError](err); ok { + return true, exitErr + } + return false, err + } + + return true, nil +} diff --git a/pkg/flatpak/flatpak_test.go b/pkg/flatpak/flatpak_test.go new file mode 100644 index 000000000..648d9858b --- /dev/null +++ b/pkg/flatpak/flatpak_test.go @@ -0,0 +1,81 @@ +package flatpak + +import ( + "os" + "path/filepath" + "testing" +) + +func TestInSandbox(t *testing.T) { + t.Run("false when marker absent", func(t *testing.T) { + withFlatpakInfoPath(t, filepath.Join(t.TempDir(), "does-not-exist")) + if InSandbox() { + t.Fatal("expected InSandbox to be false when marker file is absent") + } + }) + + t.Run("true when marker present", func(t *testing.T) { + marker := filepath.Join(t.TempDir(), ".flatpak-info") + if err := os.WriteFile(marker, []byte("[Application]\n"), 0o600); err != nil { + t.Fatalf("write marker: %v", err) + } + withFlatpakInfoPath(t, marker) + if !InSandbox() { + t.Fatal("expected InSandbox to be true when marker file exists") + } + }) +} + +func TestHostBinaryPath(t *testing.T) { + t.Run("honors XDG_DATA_HOME", func(t *testing.T) { + t.Setenv("XDG_DATA_HOME", "/custom/data") + if got, want := HostBinaryPath(), "/custom/data/devsy/devsy"; got != want { + t.Fatalf("HostBinaryPath() = %q, want %q", got, want) + } + }) + + t.Run("falls back to HOME/.local/share", func(t *testing.T) { + t.Setenv("XDG_DATA_HOME", "") + t.Setenv("HOME", "/home/tester") + if got, want := HostBinaryPath(), "/home/tester/.local/share/devsy/devsy"; got != want { + t.Fatalf("HostBinaryPath() = %q, want %q", got, want) + } + }) +} + +func TestReexecOnHost_NoopOutsideSandbox(t *testing.T) { + withFlatpakInfoPath(t, filepath.Join(t.TempDir(), "does-not-exist")) + + shouldExit, err := ReexecOnHost() + if err != nil { + t.Fatalf("ReexecOnHost() error = %v, want nil", err) + } + if shouldExit { + t.Fatal("ReexecOnHost() shouldExit = true outside sandbox, want false") + } +} + +func TestReexecOnHost_MissingHostBinary(t *testing.T) { + dir := t.TempDir() + marker := filepath.Join(dir, ".flatpak-info") + if err := os.WriteFile(marker, []byte("[Application]\n"), 0o600); err != nil { + t.Fatalf("write marker: %v", err) + } + withFlatpakInfoPath(t, marker) + t.Setenv("XDG_DATA_HOME", dir) + + shouldExit, err := ReexecOnHost() + if err == nil { + t.Fatal("ReexecOnHost() error = nil, want missing-binary error") + } + if shouldExit { + t.Fatal("ReexecOnHost() shouldExit = true on pre-check failure, want false") + } +} + +func withFlatpakInfoPath(t *testing.T, path string) { + t.Helper() + orig := flatpakInfoPath + flatpakInfoPath = path + t.Cleanup(func() { flatpakInfoPath = orig }) +} From 81cee240641ca319c88ab8f317cbaa144248309e Mon Sep 17 00:00:00 2001 From: Samuel K Date: Thu, 9 Jul 2026 16:36:15 -0500 Subject: [PATCH 02/11] test(flatpak): reuse command-name constants to satisfy goconst Replace repeated "workspace"/"provider"/"list" string literals in the cmd tests with shared constants, and drop the redundant doc comment on TestTopLevelCommand. --- cmd/env_flags_test.go | 8 +++++--- cmd/root_test.go | 9 +++------ 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/cmd/env_flags_test.go b/cmd/env_flags_test.go index 9f048d901..57e18dea4 100644 --- a/cmd/env_flags_test.go +++ b/cmd/env_flags_test.go @@ -13,6 +13,8 @@ const ( envDevsyHost = "DEVSY_HOST" envDevsyProject = "DEVSY_PROJECT" cmdWorkspace = "workspace" + cmdProvider = "provider" + cmdList = "list" ) // TestOptInEnvFlags_AppliesEnvValueToFlag verifies that for each opt-in flag, @@ -29,7 +31,7 @@ func TestOptInEnvFlags_AppliesEnvValueToFlag(t *testing.T) { }{ {flagName: "home", envName: "DEVSY_HOME", want: "/tmp/h", persistent: true}, {flagName: "context", envName: "DEVSY_CONTEXT", want: "ctx-a", persistent: true}, - {flagName: "provider", envName: "DEVSY_PROVIDER", want: "docker", persistent: true}, + {flagName: cmdProvider, envName: "DEVSY_PROVIDER", want: "docker", persistent: true}, {flagName: "debug", envName: "DEVSY_DEBUG", want: "true", persistent: true}, { cmdPath: "pro workspace list", @@ -110,7 +112,7 @@ func TestOptInEnvFlags_CLIOverridesEnv(t *testing.T) { seen, _ = c.Flags().GetString("host") return nil } - rootCmd.SetArgs([]string{"pro", cmdWorkspace, "list", "--host", "cli-value"}) + rootCmd.SetArgs([]string{"pro", cmdWorkspace, cmdList, "--host", "cli-value"}) require.NoError(t, rootCmd.Execute()) assert.Equal(t, "cli-value", seen, "CLI flag must override env value") } @@ -128,7 +130,7 @@ func TestOptInEnvFlags_EnvSatisfiesRequired(t *testing.T) { seen, _ = c.Flags().GetString("host") return nil } - rootCmd.SetArgs([]string{"pro", cmdWorkspace, "list"}) + rootCmd.SetArgs([]string{"pro", cmdWorkspace, cmdList}) require.NoError(t, rootCmd.Execute(), "required --host must be satisfied by env") assert.Equal(t, "from-env.example.com", seen) } diff --git a/cmd/root_test.go b/cmd/root_test.go index 1568c02f2..ddd12a53b 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -7,9 +7,6 @@ import ( "github.com/stretchr/testify/require" ) -// TestTopLevelCommand verifies the classifier that gates the Flatpak host -// re-exec: the daemon lives under `internal`, which must stay in-sandbox, while -// user-facing commands like `up`/`ssh` must be routed to the host. func TestTopLevelCommand(t *testing.T) { rootCmd, _ := BuildRoot() @@ -20,9 +17,9 @@ func TestTopLevelCommand(t *testing.T) { }{ {"daemon stays in sandbox", []string{internalCommand, "daemon-local"}, internalCommand}, {"nested internal command", []string{internalCommand, "ssh-server"}, internalCommand}, - {"workspace up routes to host", []string{"workspace", "up", "."}, "workspace"}, - {"workspace ssh routes to host", []string{"workspace", "ssh", "my-ws"}, "workspace"}, - {"provider list routes to host", []string{"provider", "list"}, "provider"}, + {"workspace up routes to host", []string{cmdWorkspace, "up", "."}, cmdWorkspace}, + {"workspace ssh routes to host", []string{cmdWorkspace, "ssh", "my-ws"}, cmdWorkspace}, + {"provider list routes to host", []string{cmdProvider, cmdList}, cmdProvider}, {"bare root", []string{}, ""}, } From 267b2760caa4c4ba242a9fb020bc7c83e7a3a049 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Fri, 10 Jul 2026 11:23:48 -0500 Subject: [PATCH 03/11] fix(cli): surface errors for unknown commands and bad flags Malformed invocations (unknown command, bad flag) exited 1 with no output: the failure happens before PersistentPreRunE, so the logger was never initialized (a no-op logger swallowed the error) and cobra's own error printing was disabled by SilenceErrors/SilenceUsage. Initialize the logger up front in run(), and silence cobra only in machine mode (--log-output json/logfmt, used by the desktop IPC). In interactive text mode cobra now prints its native "Error: ... / Did you mean / Run 'devsy --help' for usage." while machine mode still emits the structured cliError. exitCodeForError mirrors the split to avoid double-printing. --- cmd/flag_aliases_test.go | 1 - cmd/root.go | 135 +++++++++++++++++++++++++++++++++------ cmd/root_test.go | 29 +++++++++ 3 files changed, 144 insertions(+), 21 deletions(-) diff --git a/cmd/flag_aliases_test.go b/cmd/flag_aliases_test.go index 0b9108c57..5e1a8d919 100644 --- a/cmd/flag_aliases_test.go +++ b/cmd/flag_aliases_test.go @@ -13,7 +13,6 @@ import ( const ( flagConfig = "--config" flagDevcontainerPath = "devcontainer-path" - flagLogFormat = "--log-format" flagOverrideConfig = "--override-config" flagExtraDevContainerPath = "extra-devcontainer-path" flagDotfilesRepository = "--dotfiles-repository" diff --git a/cmd/root.go b/cmd/root.go index 30cebcef9..fb20845f0 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "os/exec" + "strings" "github.com/devsy-org/devsy/cmd/completion" cliconfig "github.com/devsy-org/devsy/cmd/config" @@ -38,6 +39,12 @@ import ( const ( logOutputJSON = "json" logOutputLogfmt = "logfmt" + logOutputText = "text" + + // flagLogOutput and flagLogFormat are the persistent flags (and alias) that + // select the log encoder; flagLogFormat is a hidden alias for flagLogOutput. + flagLogOutput = "--log-output" + flagLogFormat = "--log-format" groupCore = "core" groupConfig = "config" @@ -64,6 +71,26 @@ func isMachineLogFormat(format string) bool { return format == logOutputJSON || format == logOutputLogfmt } +// logOutputFromArgs extracts the --log-output / --log-format value from raw args +// before cobra parses them. This is needed to decide, up front, whether cobra +// should print its own human-readable error/usage text: an unknown command or +// flag error is raised before the persistent flags are bound, so the parsed +// GlobalFlags value is not yet available at that point. Returns the default +// "text" when the flag is absent. +func logOutputFromArgs(args []string) string { + for i, arg := range args { + for _, name := range []string{flagLogOutput, flagLogFormat} { + if val, ok := strings.CutPrefix(arg, name+"="); ok { + return val + } + if arg == name && i+1 < len(args) { + return args[i+1] + } + } + } + return logOutputText +} + func Execute() { os.Exit(run()) } @@ -78,10 +105,38 @@ func run() int { collector := telemetry.BootstrapCLI(target) rootCmd.SetContext(telemetry.WithCollector(gocontext.Background(), collector)) defer func() { collector.Flush() }() + + // Errors from an unknown command or a bad flag surface before cobra binds + // the persistent flags, so globalFlags.LogOutput is not yet populated on + // those paths. Recover the intended format from the raw args so both the + // early logger and the error renderer agree on the output mode. + logOutput := logOutputFromArgs(os.Args[1:]) + machineMode := isMachineLogFormat(logOutput) + + // In interactive text mode let cobra print its own error+usage text for + // malformed invocations (unknown command, bad flag) — the idiomatic CLI + // behavior, including "Did you mean" suggestions. In machine mode keep cobra + // silent so only the structured cliError reaches the stream. exitCodeForError + // mirrors this split to avoid double-printing. + rootCmd.SilenceErrors = machineMode + rootCmd.SilenceUsage = machineMode + + // Initialize the logger up front so failures that occur before (or instead + // of) PersistentPreRunE — an unknown command, a flag parse error — still + // reach the user instead of being swallowed by the default no-op logger. + // PersistentPreRunE re-initializes with the fully parsed flags for the + // normal path; Init is idempotent. + log.Init(log.Config{ + Verbosity: globalFlags.Verbosity, + Quiet: globalFlags.Quiet, + Debug: globalFlags.Debug, + Format: logOutput, + }) + if topLevelCommand(target) != internalCommand { if shouldExit, err := flatpak.ReexecOnHost(); err != nil { collector.RecordCLI(err) - return exitCodeForError(err, globalFlags) + return exitCodeForError(err, logOutput) } else if shouldExit { return 0 } @@ -97,7 +152,7 @@ func run() int { collector.RecordCLI(err) if err != nil { - return exitCodeForError(err, globalFlags) + return exitCodeForError(err, logOutput) } return 0 } @@ -115,35 +170,67 @@ func topLevelCommand(cmd *cobra.Command) string { return "" } -// exitCodeForError emits err through the configured log encoder and returns the -// process exit code that reflects the failure. -func exitCodeForError(err error, globalFlags *flags.GlobalFlags) int { +// exitCodeForError renders err and returns the process exit code that reflects +// the failure. +// +// In machine mode the structured cliError is emitted through the log encoder +// for the desktop IPC to parse. In interactive text mode cobra has already +// printed the "Error: " line (SilenceErrors is off), so we only add the +// human-facing hint/doc affordances cobra does not know about, avoiding a +// duplicate error line. +func exitCodeForError(err error, logOutput string) int { if err == nil { return 0 } + + machineMode := isMachineLogFormat(logOutput) + + // A remote SSH command or a spawned subprocess carries its own exit status; + // surface it verbatim. + if code, ok := passthroughExitCode(err, machineMode); ok { + return code + } + + renderCLIError(err, machineMode) + if errors.Is(err, workspace.ErrWorkspaceNotFound) { + return exitcode.WorkspaceNotFound + } + return 1 +} + +// passthroughExitCode returns the exit status of a subprocess or SSH command +// error verbatim. The second result is false when err is neither. +func passthroughExitCode(err error, machineMode bool) (int, bool) { if sshExitErr, ok := errors.AsType[*ssh.ExitError](err); ok { - log.Errorf("SSH command failed with exit code %d", sshExitErr.ExitStatus()) - return sshExitErr.ExitStatus() + if machineMode { + log.Errorf("SSH command failed with exit code %d", sshExitErr.ExitStatus()) + } + return sshExitErr.ExitStatus(), true } if execExitErr, ok := errors.AsType[*exec.ExitError](err); ok { - log.Errorf("Command failed with exit code %d", execExitErr.ExitCode()) - return execExitErr.ExitCode() + if machineMode { + log.Errorf("Command failed with exit code %d", execExitErr.ExitCode()) + } + return execExitErr.ExitCode(), true } + return 0, false +} +// renderCLIError emits err for the user. Machine mode ships the structured +// cliError through the log encoder for the desktop IPC; text mode adds only the +// hint/doc affordances, since cobra already printed the "Error: " line. +func renderCLIError(err error, machineMode bool) { cliErr := cliErrors.Classify(err, cliErrors.ClassifyContext{}) - log.JSONError(cliErr) - if !isMachineLogFormat(globalFlags.LogOutput) { - if cliErr.Hint != "" { - fmt.Fprintf(os.Stderr, "Hint: %s\n", cliErr.Hint) - } - if cliErr.DocURL != "" { - fmt.Fprintf(os.Stderr, "See: %s\n", cliErr.DocURL) - } + if machineMode { + log.JSONError(cliErr) + return } - if errors.Is(err, workspace.ErrWorkspaceNotFound) { - return exitcode.WorkspaceNotFound + if cliErr.Hint != "" { + fmt.Fprintf(os.Stderr, "Hint: %s\n", cliErr.Hint) + } + if cliErr.DocURL != "" { + fmt.Fprintf(os.Stderr, "See: %s\n", cliErr.DocURL) } - return 1 } // BuildRoot constructs the root command and returns it alongside the parsed @@ -163,6 +250,14 @@ func BuildRoot() (*cobra.Command, *flags.GlobalFlags) { _ = completion.RegisterFlagCompletionFuns(rootCmd, globalFlags) rootCmd.PersistentPreRunE = func(cobraCmd *cobra.Command, _ []string) error { + // Reaching PersistentPreRunE means args and flags parsed cleanly, so the + // invocation is valid: any error from here on is a runtime failure, not a + // usage mistake. Silence cobra's usage dump so runtime errors render as a + // clean message (via exitCodeForError) rather than a full help screen. + // Usage/parse errors occur before this hook and still get cobra's native + // error+usage output in text mode. + cobraCmd.SilenceUsage = true + log.Init(log.Config{ Verbosity: globalFlags.Verbosity, Quiet: globalFlags.Quiet, diff --git a/cmd/root_test.go b/cmd/root_test.go index ddd12a53b..ad3cb3153 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -31,3 +31,32 @@ func TestTopLevelCommand(t *testing.T) { }) } } + +func TestLogOutputFromArgs(t *testing.T) { + cases := []struct { + name string + args []string + want string + }{ + {"absent defaults to text", []string{cmdProvider, cmdList}, "text"}, + {"space-separated log-output", []string{"up", "--log-output", "json"}, "json"}, + {"equals-form log-output", []string{"up", "--log-output=json"}, "json"}, + {"log-format alias", []string{"up", "--log-format", "logfmt"}, "logfmt"}, + {"equals-form log-format alias", []string{"--log-format=json", "up"}, "json"}, + {"flag before unknown command", []string{"--log-output", "json", "bogus"}, "json"}, + {"trailing flag with no value", []string{"up", "--log-output"}, "text"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, logOutputFromArgs(tc.args)) + }) + } +} + +func TestIsMachineLogFormat(t *testing.T) { + assert.True(t, isMachineLogFormat(logOutputJSON)) + assert.True(t, isMachineLogFormat(logOutputLogfmt)) + assert.False(t, isMachineLogFormat("text")) + assert.False(t, isMachineLogFormat("")) +} From f53b797f0ba96165c6a78130ef9981213b32fb7d Mon Sep 17 00:00:00 2001 From: Samuel K Date: Fri, 10 Jul 2026 11:29:55 -0500 Subject: [PATCH 04/11] fix(ci): build a static CLI so agent injection works in musl containers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The desktop CI built the CLI with a plain `go build`, which defaults to CGO enabled on the runner and produces a glibc-dynamically-linked binary. Because that same binary is injected into workspace containers as the agent, it failed to exec in musl-based images (e.g. alpine) — the dynamic loader /lib64/ld-linux-x86-64.so.2 is absent — surfacing as "inject agent: EOF" (container exits 127) during workspace up. Match the goreleaser release build: CGO_ENABLED=0 with the netgo/osusergo tags, yielding a fully static binary that runs regardless of the container's libc. --- .github/workflows/desktop-ci.yml | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/.github/workflows/desktop-ci.yml b/.github/workflows/desktop-ci.yml index bea4914d0..b7e9c630e 100644 --- a/.github/workflows/desktop-ci.yml +++ b/.github/workflows/desktop-ci.yml @@ -74,27 +74,36 @@ jobs: cache: npm cache-dependency-path: desktop/package-lock.json + # CGO_ENABLED=0 + netgo/osusergo produce a static binary, matching the + # goreleaser release build. This is required because the CLI is also + # injected as the workspace agent into containers: a glibc-dynamic build + # cannot exec in musl-based images (e.g. alpine), which surfaces as + # "inject agent: EOF" during workspace up. - name: Build CLI (unix) if: runner.os != 'Windows' + env: + CGO_ENABLED: "0" run: | if [ "${{ matrix.go-arch }}" = "universal" ]; then # macOS universal binary: build both architectures and lipo them - GOOS=darwin GOARCH=amd64 go build -o devsy-amd64 . - GOOS=darwin GOARCH=arm64 go build -o devsy-arm64 . + GOOS=darwin GOARCH=amd64 go build -tags netgo,osusergo -o devsy-amd64 . + GOOS=darwin GOARCH=arm64 go build -tags netgo,osusergo -o devsy-arm64 . lipo -create -output desktop/resources/bin/devsy devsy-amd64 devsy-arm64 rm devsy-amd64 devsy-arm64 else - GOOS=${{ matrix.go-os }} GOARCH=${{ matrix.go-arch }} go build -o desktop/resources/bin/devsy . + GOOS=${{ matrix.go-os }} GOARCH=${{ matrix.go-arch }} go build -tags netgo,osusergo -o desktop/resources/bin/devsy . fi chmod +x desktop/resources/bin/devsy - name: Build CLI (windows) if: runner.os == 'Windows' shell: pwsh + env: + CGO_ENABLED: "0" run: | $env:GOOS = "windows" $env:GOARCH = "amd64" - go build -o desktop/resources/bin/devsy.exe . + go build -tags netgo,osusergo -o desktop/resources/bin/devsy.exe . - name: Configure Windows build tools if: runner.os == 'Windows' From feceea6dd064eafb7128dfa067b67922a314fc33 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Fri, 10 Jul 2026 11:37:31 -0500 Subject: [PATCH 05/11] docs: trim verbose comments per style guidelines --- .github/workflows/desktop-ci.yml | 7 ++--- cmd/root.go | 49 ++++++-------------------------- 2 files changed, 11 insertions(+), 45 deletions(-) diff --git a/.github/workflows/desktop-ci.yml b/.github/workflows/desktop-ci.yml index b7e9c630e..892541d34 100644 --- a/.github/workflows/desktop-ci.yml +++ b/.github/workflows/desktop-ci.yml @@ -74,11 +74,8 @@ jobs: cache: npm cache-dependency-path: desktop/package-lock.json - # CGO_ENABLED=0 + netgo/osusergo produce a static binary, matching the - # goreleaser release build. This is required because the CLI is also - # injected as the workspace agent into containers: a glibc-dynamic build - # cannot exec in musl-based images (e.g. alpine), which surfaces as - # "inject agent: EOF" during workspace up. + # Static build (matches goreleaser): the CLI is injected as the workspace + # agent and must exec in musl-based containers (e.g. alpine). - name: Build CLI (unix) if: runner.os != 'Windows' env: diff --git a/cmd/root.go b/cmd/root.go index fb20845f0..8634ed336 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -41,8 +41,6 @@ const ( logOutputLogfmt = "logfmt" logOutputText = "text" - // flagLogOutput and flagLogFormat are the persistent flags (and alias) that - // select the log encoder; flagLogFormat is a hidden alias for flagLogOutput. flagLogOutput = "--log-output" flagLogFormat = "--log-format" @@ -72,11 +70,7 @@ func isMachineLogFormat(format string) bool { } // logOutputFromArgs extracts the --log-output / --log-format value from raw args -// before cobra parses them. This is needed to decide, up front, whether cobra -// should print its own human-readable error/usage text: an unknown command or -// flag error is raised before the persistent flags are bound, so the parsed -// GlobalFlags value is not yet available at that point. Returns the default -// "text" when the flag is absent. +// before cobra binds the persistent flags. Returns "text" when absent. func logOutputFromArgs(args []string) string { for i, arg := range args { for _, name := range []string{flagLogOutput, flagLogFormat} { @@ -106,26 +100,15 @@ func run() int { rootCmd.SetContext(telemetry.WithCollector(gocontext.Background(), collector)) defer func() { collector.Flush() }() - // Errors from an unknown command or a bad flag surface before cobra binds - // the persistent flags, so globalFlags.LogOutput is not yet populated on - // those paths. Recover the intended format from the raw args so both the - // early logger and the error renderer agree on the output mode. + // Cobra prints its own error/usage in text mode; machine mode stays silent + // so only the structured cliError reaches the stream. logOutput := logOutputFromArgs(os.Args[1:]) machineMode := isMachineLogFormat(logOutput) - - // In interactive text mode let cobra print its own error+usage text for - // malformed invocations (unknown command, bad flag) — the idiomatic CLI - // behavior, including "Did you mean" suggestions. In machine mode keep cobra - // silent so only the structured cliError reaches the stream. exitCodeForError - // mirrors this split to avoid double-printing. rootCmd.SilenceErrors = machineMode rootCmd.SilenceUsage = machineMode - // Initialize the logger up front so failures that occur before (or instead - // of) PersistentPreRunE — an unknown command, a flag parse error — still - // reach the user instead of being swallowed by the default no-op logger. - // PersistentPreRunE re-initializes with the fully parsed flags for the - // normal path; Init is idempotent. + // Initialize logging before Execute so errors on paths that skip + // PersistentPreRunE (unknown command, flag parse error) still surface. log.Init(log.Config{ Verbosity: globalFlags.Verbosity, Quiet: globalFlags.Quiet, @@ -172,21 +155,12 @@ func topLevelCommand(cmd *cobra.Command) string { // exitCodeForError renders err and returns the process exit code that reflects // the failure. -// -// In machine mode the structured cliError is emitted through the log encoder -// for the desktop IPC to parse. In interactive text mode cobra has already -// printed the "Error: " line (SilenceErrors is off), so we only add the -// human-facing hint/doc affordances cobra does not know about, avoiding a -// duplicate error line. func exitCodeForError(err error, logOutput string) int { if err == nil { return 0 } machineMode := isMachineLogFormat(logOutput) - - // A remote SSH command or a spawned subprocess carries its own exit status; - // surface it verbatim. if code, ok := passthroughExitCode(err, machineMode); ok { return code } @@ -216,9 +190,8 @@ func passthroughExitCode(err error, machineMode bool) (int, bool) { return 0, false } -// renderCLIError emits err for the user. Machine mode ships the structured -// cliError through the log encoder for the desktop IPC; text mode adds only the -// hint/doc affordances, since cobra already printed the "Error: " line. +// renderCLIError emits the structured cliError in machine mode; in text mode +// cobra already printed the error line, so only hint/doc affordances are added. func renderCLIError(err error, machineMode bool) { cliErr := cliErrors.Classify(err, cliErrors.ClassifyContext{}) if machineMode { @@ -250,12 +223,8 @@ func BuildRoot() (*cobra.Command, *flags.GlobalFlags) { _ = completion.RegisterFlagCompletionFuns(rootCmd, globalFlags) rootCmd.PersistentPreRunE = func(cobraCmd *cobra.Command, _ []string) error { - // Reaching PersistentPreRunE means args and flags parsed cleanly, so the - // invocation is valid: any error from here on is a runtime failure, not a - // usage mistake. Silence cobra's usage dump so runtime errors render as a - // clean message (via exitCodeForError) rather than a full help screen. - // Usage/parse errors occur before this hook and still get cobra's native - // error+usage output in text mode. + // Args parsed cleanly by now, so any later error is a runtime failure: + // suppress cobra's usage dump. Parse errors occur before this hook. cobraCmd.SilenceUsage = true log.Init(log.Config{ From a58a6b844a4455de705d6d43d115fea93535ea0d Mon Sep 17 00:00:00 2001 From: Samuel K Date: Fri, 10 Jul 2026 11:57:23 -0500 Subject: [PATCH 06/11] test(cli): use flag/format constants to satisfy goconst --- cmd/root_test.go | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/cmd/root_test.go b/cmd/root_test.go index ad3cb3153..b226fede4 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -38,13 +38,25 @@ func TestLogOutputFromArgs(t *testing.T) { args []string want string }{ - {"absent defaults to text", []string{cmdProvider, cmdList}, "text"}, - {"space-separated log-output", []string{"up", "--log-output", "json"}, "json"}, - {"equals-form log-output", []string{"up", "--log-output=json"}, "json"}, - {"log-format alias", []string{"up", "--log-format", "logfmt"}, "logfmt"}, - {"equals-form log-format alias", []string{"--log-format=json", "up"}, "json"}, - {"flag before unknown command", []string{"--log-output", "json", "bogus"}, "json"}, - {"trailing flag with no value", []string{"up", "--log-output"}, "text"}, + {"absent defaults to text", []string{cmdProvider, cmdList}, logOutputText}, + {"space-separated log-output", []string{"up", flagLogOutput, logOutputJSON}, logOutputJSON}, + { + "equals-form log-output", + []string{"up", flagLogOutput + "=" + logOutputJSON}, + logOutputJSON, + }, + {"log-format alias", []string{"up", flagLogFormat, logOutputLogfmt}, logOutputLogfmt}, + { + "equals-form log-format alias", + []string{flagLogFormat + "=" + logOutputJSON, "up"}, + logOutputJSON, + }, + { + "flag before unknown command", + []string{flagLogOutput, logOutputJSON, "bogus"}, + logOutputJSON, + }, + {"trailing flag with no value", []string{"up", flagLogOutput}, logOutputText}, } for _, tc := range cases { @@ -57,6 +69,6 @@ func TestLogOutputFromArgs(t *testing.T) { func TestIsMachineLogFormat(t *testing.T) { assert.True(t, isMachineLogFormat(logOutputJSON)) assert.True(t, isMachineLogFormat(logOutputLogfmt)) - assert.False(t, isMachineLogFormat("text")) + assert.False(t, isMachineLogFormat(logOutputText)) assert.False(t, isMachineLogFormat("")) } From 9c4201b21db861d8699cfa18174d63f65dd1592e Mon Sep 17 00:00:00 2001 From: Samuel K Date: Fri, 10 Jul 2026 12:38:51 -0500 Subject: [PATCH 07/11] fix(exec): resolve docker path from provider options, not just OS env ResolveDockerCommand expanded agent.docker.path (typically the template "${DOCKER_PATH}") with os.ExpandEnv, but DOCKER_PATH is a provider option rather than an OS environment variable. It therefore expanded to empty and fell back to "docker", so `workspace exec` failed with `docker: executable file not found` on podman-only hosts even though the provider was configured with DOCKER_PATH=podman. Expand against the workspace's resolved provider options first, falling back to the OS environment. --- pkg/workspace/exec.go | 20 +++++++++++++++++++- pkg/workspace/exec_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/pkg/workspace/exec.go b/pkg/workspace/exec.go index 198beefcd..3fba2d887 100644 --- a/pkg/workspace/exec.go +++ b/pkg/workspace/exec.go @@ -55,7 +55,14 @@ func ResolveDockerCommand( } if providerConfig.Agent.Docker.Path != "" { - if expanded := os.ExpandEnv(providerConfig.Agent.Docker.Path); expanded != "" { + // agent.docker.path is typically a provider-option template such as + // "${DOCKER_PATH}". Expand it against the workspace's resolved provider + // options (falling back to the OS environment) so the configured value + // is honored — the options are not present in the process environment. + if expanded := expandWithOptions( + providerConfig.Agent.Docker.Path, + workspace.Provider.Options, + ); expanded != "" { return expanded } } @@ -63,6 +70,17 @@ func ResolveDockerCommand( return DefaultDockerCommand } +// expandWithOptions expands ${VAR} references in s using the given resolved +// provider options first, then the OS environment. +func expandWithOptions(s string, options map[string]config.OptionValue) string { + return os.Expand(s, func(key string) string { + if opt, ok := options[key]; ok && opt.Value != "" { + return opt.Value + } + return os.Getenv(key) + }) +} + func LoadExecResult( workspaceConfig *provider2.Workspace, containerDetails *devcconfig.ContainerDetails, diff --git a/pkg/workspace/exec_test.go b/pkg/workspace/exec_test.go index 68b2d4138..28472b7ec 100644 --- a/pkg/workspace/exec_test.go +++ b/pkg/workspace/exec_test.go @@ -6,9 +6,37 @@ import ( "io" "testing" + "github.com/devsy-org/devsy/pkg/config" devcconfig "github.com/devsy-org/devsy/pkg/devcontainer/config" ) +func TestExpandWithOptions(t *testing.T) { + t.Setenv("DEVSY_TEST_OSVAR", "from-os") + opts := map[string]config.OptionValue{ + "DOCKER_PATH": {Value: "podman"}, + "EMPTY": {Value: ""}, + } + + cases := []struct { + name string + in string + want string + }{ + {"resolves from options", "${DOCKER_PATH}", "podman"}, + {"literal passthrough", "docker", "docker"}, + {"empty option falls back to os env", "${EMPTY}", ""}, + {"unknown key falls back to os env", "${DEVSY_TEST_OSVAR}", "from-os"}, + {"missing everywhere is empty", "${DEVSY_TEST_ABSENT}", ""}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := expandWithOptions(tc.in, opts); got != tc.want { + t.Fatalf("expandWithOptions(%q) = %q, want %q", tc.in, got, tc.want) + } + }) + } +} + func TestExecOneShotOptions_ResolveTimeout_Clamp(t *testing.T) { opts := ExecOneShotOptions{ TimeoutSeconds: 10000, From 20ff97e0219dd573a4df6d576f9faf3d044ef782 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Fri, 10 Jul 2026 13:16:25 -0500 Subject: [PATCH 08/11] fix(cli): keep cobra silent for the internal agent subtree The conditional cobra silencing regressed the agent protocol: `internal agent ...` commands are invoked without --log-output=json (they force JSON internally) and drive a binary/JSON protocol on stdout. With silencing keyed only on --log-output, a parse/usage error dumped cobra's human-readable Usage block into that stream, breaking the e2e log-stream contract (up-handle-errors). Treat the internal subtree as always machine-facing: silence cobra regardless of --log-output. Extracted configureOutput to keep run() within complexity limits, and added a regression test for the contract. --- cmd/root.go | 43 +++++++++++++++++++++++++++---------------- cmd/root_test.go | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 16 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index 8634ed336..9d266d835 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -100,23 +100,10 @@ func run() int { rootCmd.SetContext(telemetry.WithCollector(gocontext.Background(), collector)) defer func() { collector.Flush() }() - // Cobra prints its own error/usage in text mode; machine mode stays silent - // so only the structured cliError reaches the stream. - logOutput := logOutputFromArgs(os.Args[1:]) - machineMode := isMachineLogFormat(logOutput) - rootCmd.SilenceErrors = machineMode - rootCmd.SilenceUsage = machineMode - - // Initialize logging before Execute so errors on paths that skip - // PersistentPreRunE (unknown command, flag parse error) still surface. - log.Init(log.Config{ - Verbosity: globalFlags.Verbosity, - Quiet: globalFlags.Quiet, - Debug: globalFlags.Debug, - Format: logOutput, - }) + isInternal := topLevelCommand(target) == internalCommand + logOutput := configureOutput(rootCmd, globalFlags, isInternal) - if topLevelCommand(target) != internalCommand { + if !isInternal { if shouldExit, err := flatpak.ReexecOnHost(); err != nil { collector.RecordCLI(err) return exitCodeForError(err, logOutput) @@ -140,6 +127,30 @@ func run() int { return 0 } +// configureOutput sets cobra's error/usage silencing and initializes logging, +// returning the resolved log output format. Cobra prints its own error/usage in +// text mode; machine mode (structured --log-output, or the internal subtree +// which drives the agent protocol on stdout) stays silent. Logging is set up +// before Execute so errors on paths that skip PersistentPreRunE still surface. +func configureOutput( + rootCmd *cobra.Command, + globalFlags *flags.GlobalFlags, + isInternal bool, +) string { + logOutput := logOutputFromArgs(os.Args[1:]) + machineMode := isMachineLogFormat(logOutput) || isInternal + rootCmd.SilenceErrors = machineMode + rootCmd.SilenceUsage = machineMode + + log.Init(log.Config{ + Verbosity: globalFlags.Verbosity, + Quiet: globalFlags.Quiet, + Debug: globalFlags.Debug, + Format: logOutput, + }) + return logOutput +} + func topLevelCommand(cmd *cobra.Command) string { if cmd == nil { return "" diff --git a/cmd/root_test.go b/cmd/root_test.go index b226fede4..aae4efa29 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -1,6 +1,7 @@ package cmd import ( + "os" "testing" "github.com/stretchr/testify/assert" @@ -72,3 +73,40 @@ func TestIsMachineLogFormat(t *testing.T) { assert.False(t, isMachineLogFormat(logOutputText)) assert.False(t, isMachineLogFormat("")) } + +// TestConfigureOutput_SilencesCobra locks in the output contract: cobra prints +// its own error/usage only in interactive text mode. Machine formats and the +// internal subtree (which drives the agent protocol on stdout) must stay +// silent so cobra's usage text never corrupts the stream. +func TestConfigureOutput_SilencesCobra(t *testing.T) { + cases := []struct { + name string + args []string + isInternal bool + wantSilent bool + }{ + {"text interactive", []string{"up"}, false, false}, + {"json machine", []string{"up", flagLogOutput, logOutputJSON}, false, true}, + {"logfmt machine", []string{"up", flagLogOutput, logOutputLogfmt}, false, true}, + {"internal text stays silent", []string{internalCommand}, true, true}, + { + "internal json stays silent", + []string{internalCommand, flagLogOutput, logOutputJSON}, + true, + true, + }, + } + + origArgs := os.Args + t.Cleanup(func() { os.Args = origArgs }) + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + rootCmd, globalFlags := BuildRoot() + os.Args = append([]string{"devsy"}, tc.args...) + configureOutput(rootCmd, globalFlags, tc.isInternal) + assert.Equal(t, tc.wantSilent, rootCmd.SilenceErrors) + assert.Equal(t, tc.wantSilent, rootCmd.SilenceUsage) + }) + } +} From 951f8ac10756a31c486ee88598ec2335bc98c29d Mon Sep 17 00:00:00 2001 From: Samuel K Date: Fri, 10 Jul 2026 13:57:59 -0500 Subject: [PATCH 09/11] feat(podman): warn when rootless podman lacks systemd linger MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rootless Podman has no persistent daemon, so its containers live under the user's systemd session and are reaped when that session ends (SSH disconnect, or the desktop closing the spawning process). A workspace that came up successfully then stops on its own — unlike Docker, whose system daemon keeps containers alive independently of login sessions. Detect this configuration (rootless podman + linger disabled) during container creation and surface a HostWarning telling the user to run `loginctl enable-linger`. The warning flows through the result envelope, so both the CLI and the desktop app can display it. Detection never blocks workspace creation and is skipped for Docker/nerdctl. --- pkg/devcontainer/single.go | 19 ++++++++++ pkg/docker/linger.go | 71 ++++++++++++++++++++++++++++++++++++++ pkg/docker/linger_test.go | 28 +++++++++++++++ 3 files changed, 118 insertions(+) create mode 100644 pkg/docker/linger.go create mode 100644 pkg/docker/linger_test.go diff --git a/pkg/devcontainer/single.go b/pkg/devcontainer/single.go index dd920de6f..708e4aac6 100644 --- a/pkg/devcontainer/single.go +++ b/pkg/devcontainer/single.go @@ -294,6 +294,10 @@ func (r *runner) resolveNewContainer( return nil, err } + if w := r.lingerWarning(ctx); w != "" { + hostWarnings = append(hostWarnings, w) + } + return &resolvedContainer{ details: containerDetails, mergedConfig: mergedConfig, @@ -301,6 +305,21 @@ func (r *runner) resolveNewContainer( }, nil } +// lingerWarning surfaces a warning when a newly created container runs under +// rootless Podman without systemd linger, where it would be reaped on session +// end. Returns "" for other drivers/runtimes or when the state cannot be read. +func (r *runner) lingerWarning(ctx context.Context) string { + dockerDriver, ok := r.driver.(driver.DockerDriver) + if !ok { + return "" + } + helper, err := dockerDriver.DockerHelper() + if err != nil { + return "" + } + return helper.LingerWarning(ctx) +} + // buildNewContainerConfig builds the image (deleting the existing container // first when recreating) and produces the merged devcontainer config from the // build's image metadata. diff --git a/pkg/docker/linger.go b/pkg/docker/linger.go new file mode 100644 index 000000000..37c377f0f --- /dev/null +++ b/pkg/docker/linger.go @@ -0,0 +1,71 @@ +package docker + +import ( + "context" + "os" + "os/user" + "strings" + "time" +) + +// LingerWarning returns a non-empty warning when the container runtime is +// rootless Podman and systemd linger is disabled for the current user. In that +// configuration systemd reaps the user's containers when the login session +// ends (e.g. SSH disconnect, or the desktop app closing the spawning process), +// so a workspace that started successfully stops on its own — unlike Docker, +// whose system daemon keeps containers alive independently of user sessions. +// +// It returns "" when the situation does not apply or cannot be determined; +// detection never blocks workspace creation. +func (r *DockerHelper) LingerWarning(ctx context.Context) string { + if !r.IsPodman() || !r.isRootless(ctx) { + return "" + } + if lingerEnabled() { + return "" + } + return "rootless Podman without systemd linger: the workspace container will " + + "stop when your login session ends. Run `loginctl enable-linger` to keep " + + "it running after logout (Docker does not require this)." +} + +// isRootless reports whether Podman is running rootless. A detection failure is +// treated as rootless, since that is the case the warning targets and a false +// positive only adds a hint. +func (r *DockerHelper) isRootless(ctx context.Context) bool { + ctx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + + out, err := r.buildCmd(ctx, "info", "--format", "{{.Host.Security.Rootless}}").Output() + if err != nil { + return true + } + return strings.TrimSpace(string(out)) != "false" +} + +// lingerDir is the directory where systemd maintains a marker file per user +// with linger enabled. Declared as a var so tests can override it. +var lingerDir = "/var/lib/systemd/linger" + +// lingerEnabled reports whether systemd linger is enabled for the current user. +// It checks the marker file systemd maintains, avoiding a dependency on +// loginctl being on PATH. A read failure is treated as "enabled" so an +// undetectable state does not produce a spurious warning. +func lingerEnabled() bool { + u, err := user.Current() + if err != nil || u.Username == "" { + return true + } + return userHasLinger(u.Username) +} + +func userHasLinger(username string) bool { + _, err := os.Stat(lingerDir + "/" + username) + if err == nil { + return true + } + if os.IsNotExist(err) { + return false + } + return true +} diff --git a/pkg/docker/linger_test.go b/pkg/docker/linger_test.go new file mode 100644 index 000000000..710a88c97 --- /dev/null +++ b/pkg/docker/linger_test.go @@ -0,0 +1,28 @@ +package docker + +import ( + "os" + "path/filepath" + "testing" +) + +func TestUserHasLinger(t *testing.T) { + dir := t.TempDir() + orig := lingerDir + lingerDir = dir + t.Cleanup(func() { lingerDir = orig }) + + if userHasLinger("alice") { + t.Fatal("expected no linger when marker file is absent") + } + + if err := os.WriteFile(filepath.Join(dir, "alice"), nil, 0o600); err != nil { + t.Fatalf("write marker: %v", err) + } + if !userHasLinger("alice") { + t.Fatal("expected linger when marker file exists") + } + if userHasLinger("bob") { + t.Fatal("expected no linger for a different user") + } +} From acdda973ff27290a73208dfab8104b1035ad9fdd Mon Sep 17 00:00:00 2001 From: Samuel K Date: Fri, 10 Jul 2026 14:06:34 -0500 Subject: [PATCH 10/11] feat(cli): detect machine consumers by provenance, not just log format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Output bifurcation previously keyed only on --log-output, so the desktop app had to pass it on every call and any invocation that forgot leaked human-formatted text into a parsed stream. Consolidate the decision into isMachineConsumer, which treats output as machine-consumed when: the command is in the internal subtree (agent protocol), DEVSY_UI marks a desktop-spawned process (provenance, already set by the desktop for every CLI call), --log-output requests a structured format, or — as a fallback — stderr is not a terminal (piped), matching conventional CLI behavior. A human at a terminal still gets cobra's usage/error text. exitCodeForError and configureOutput now share this single decision so they cannot drift. --- cmd/root.go | 59 +++++++++++++++++++++++++++++++++------------ cmd/root_test.go | 63 ++++++++++++++++++++++++++++++++++++------------ 2 files changed, 91 insertions(+), 31 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index 9d266d835..0834b9862 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -33,6 +33,7 @@ import ( "github.com/go-logr/logr" "github.com/spf13/cobra" "golang.org/x/crypto/ssh" + "golang.org/x/term" "k8s.io/klog/v2" ) @@ -69,8 +70,9 @@ func isMachineLogFormat(format string) bool { return format == logOutputJSON || format == logOutputLogfmt } -// logOutputFromArgs extracts the --log-output / --log-format value from raw args -// before cobra binds the persistent flags. Returns "text" when absent. +// logOutputFromArgs extracts the --log-output / --log-format value from raw +// args before cobra binds the persistent flags. Returns "" when absent so +// callers can distinguish an unset flag from an explicit "text". func logOutputFromArgs(args []string) string { for i, arg := range args { for _, name := range []string{flagLogOutput, flagLogFormat} { @@ -82,7 +84,29 @@ func logOutputFromArgs(args []string) string { } } } - return logOutputText + return "" +} + +// isMachineConsumer reports whether output is being consumed by a machine +// rather than read by a human at a terminal. Cobra's usage/error text and other +// human affordances are suppressed for machine consumers so they cannot corrupt +// a parsed stream. Signals, in order: +// - the internal subtree drives the agent protocol on stdout; +// - DEVSY_UI marks a process spawned by the desktop app (provenance); +// - an explicit structured --log-output (json/logfmt); +// - as a fallback, output redirected off a terminal (piped) with no explicit +// --log-output, matching conventional CLI behavior. +func isMachineConsumer(logOutput string, isInternal bool) bool { + switch { + case isInternal: + return true + case os.Getenv(config.EnvUI) == config.BoolTrue: + return true + case logOutput != "": + return isMachineLogFormat(logOutput) + default: + return !term.IsTerminal(int(os.Stderr.Fd())) //nolint:gosec // fd fits in int + } } func Execute() { @@ -101,12 +125,12 @@ func run() int { defer func() { collector.Flush() }() isInternal := topLevelCommand(target) == internalCommand - logOutput := configureOutput(rootCmd, globalFlags, isInternal) + machineMode := configureOutput(rootCmd, globalFlags, isInternal) if !isInternal { if shouldExit, err := flatpak.ReexecOnHost(); err != nil { collector.RecordCLI(err) - return exitCodeForError(err, logOutput) + return exitCodeForError(err, machineMode) } else if shouldExit { return 0 } @@ -122,33 +146,37 @@ func run() int { collector.RecordCLI(err) if err != nil { - return exitCodeForError(err, logOutput) + return exitCodeForError(err, machineMode) } return 0 } // configureOutput sets cobra's error/usage silencing and initializes logging, -// returning the resolved log output format. Cobra prints its own error/usage in -// text mode; machine mode (structured --log-output, or the internal subtree -// which drives the agent protocol on stdout) stays silent. Logging is set up -// before Execute so errors on paths that skip PersistentPreRunE still surface. +// returning whether output is machine-consumed. Cobra prints its own +// error/usage only for an interactive human; machine consumers stay silent so +// their stream is not corrupted. Logging is set up before Execute so errors on +// paths that skip PersistentPreRunE still surface. func configureOutput( rootCmd *cobra.Command, globalFlags *flags.GlobalFlags, isInternal bool, -) string { +) bool { logOutput := logOutputFromArgs(os.Args[1:]) - machineMode := isMachineLogFormat(logOutput) || isInternal + machineMode := isMachineConsumer(logOutput, isInternal) rootCmd.SilenceErrors = machineMode rootCmd.SilenceUsage = machineMode + format := logOutput + if format == "" { + format = logOutputText + } log.Init(log.Config{ Verbosity: globalFlags.Verbosity, Quiet: globalFlags.Quiet, Debug: globalFlags.Debug, - Format: logOutput, + Format: format, }) - return logOutput + return machineMode } func topLevelCommand(cmd *cobra.Command) string { @@ -166,12 +194,11 @@ func topLevelCommand(cmd *cobra.Command) string { // exitCodeForError renders err and returns the process exit code that reflects // the failure. -func exitCodeForError(err error, logOutput string) int { +func exitCodeForError(err error, machineMode bool) int { if err == nil { return 0 } - machineMode := isMachineLogFormat(logOutput) if code, ok := passthroughExitCode(err, machineMode); ok { return code } diff --git a/cmd/root_test.go b/cmd/root_test.go index aae4efa29..6ac53fbf8 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -4,6 +4,7 @@ import ( "os" "testing" + "github.com/devsy-org/devsy/pkg/config" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -39,7 +40,7 @@ func TestLogOutputFromArgs(t *testing.T) { args []string want string }{ - {"absent defaults to text", []string{cmdProvider, cmdList}, logOutputText}, + {"absent returns empty", []string{cmdProvider, cmdList}, ""}, {"space-separated log-output", []string{"up", flagLogOutput, logOutputJSON}, logOutputJSON}, { "equals-form log-output", @@ -57,7 +58,7 @@ func TestLogOutputFromArgs(t *testing.T) { []string{flagLogOutput, logOutputJSON, "bogus"}, logOutputJSON, }, - {"trailing flag with no value", []string{"up", flagLogOutput}, logOutputText}, + {"trailing flag with no value returns empty", []string{"up", flagLogOutput}, ""}, } for _, tc := range cases { @@ -74,10 +75,46 @@ func TestIsMachineLogFormat(t *testing.T) { assert.False(t, isMachineLogFormat("")) } -// TestConfigureOutput_SilencesCobra locks in the output contract: cobra prints -// its own error/usage only in interactive text mode. Machine formats and the -// internal subtree (which drives the agent protocol on stdout) must stay -// silent so cobra's usage text never corrupts the stream. +// TestIsMachineConsumer locks in the output contract: only an interactive human +// (no machine signal, attached to a terminal) reads human-formatted output. +// The internal subtree (agent protocol), the desktop app (DEVSY_UI), and an +// explicit structured --log-output are all machine consumers. +func TestIsMachineConsumer(t *testing.T) { + cases := []struct { + name string + logOutput string + isInternal bool + devsyUI string + want bool + }{ + {"internal subtree", "", true, "", true}, + {"desktop provenance", "", false, config.BoolTrue, true}, + {"explicit json", logOutputJSON, false, "", true}, + {"explicit logfmt", logOutputLogfmt, false, "", true}, + {"explicit text is human", logOutputText, false, "", false}, + {"internal wins over text", logOutputText, true, "", true}, + {"desktop wins over text", logOutputText, false, config.BoolTrue, true}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Setenv(config.EnvUI, tc.devsyUI) + assert.Equal(t, tc.want, isMachineConsumer(tc.logOutput, tc.isInternal)) + }) + } +} + +// TestIsMachineConsumer_TTYFallback verifies that with no explicit signal the +// decision falls back to whether stderr is a terminal. Tests do not run on a +// TTY, so a bare invocation is treated as machine (piped) output. +func TestIsMachineConsumer_TTYFallback(t *testing.T) { + t.Setenv(config.EnvUI, "") + assert.True(t, isMachineConsumer("", false), + "non-terminal stderr with no explicit format should be machine mode") +} + +// TestConfigureOutput_SilencesCobra locks in that configureOutput mirrors the +// machine/human decision into cobra's error/usage silencing. func TestConfigureOutput_SilencesCobra(t *testing.T) { cases := []struct { name string @@ -85,16 +122,10 @@ func TestConfigureOutput_SilencesCobra(t *testing.T) { isInternal bool wantSilent bool }{ - {"text interactive", []string{"up"}, false, false}, {"json machine", []string{"up", flagLogOutput, logOutputJSON}, false, true}, {"logfmt machine", []string{"up", flagLogOutput, logOutputLogfmt}, false, true}, - {"internal text stays silent", []string{internalCommand}, true, true}, - { - "internal json stays silent", - []string{internalCommand, flagLogOutput, logOutputJSON}, - true, - true, - }, + {"explicit text is human", []string{"up", flagLogOutput, logOutputText}, false, false}, + {"internal stays silent", []string{internalCommand}, true, true}, } origArgs := os.Args @@ -102,9 +133,11 @@ func TestConfigureOutput_SilencesCobra(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { + t.Setenv(config.EnvUI, "") rootCmd, globalFlags := BuildRoot() os.Args = append([]string{"devsy"}, tc.args...) - configureOutput(rootCmd, globalFlags, tc.isInternal) + machineMode := configureOutput(rootCmd, globalFlags, tc.isInternal) + assert.Equal(t, tc.wantSilent, machineMode) assert.Equal(t, tc.wantSilent, rootCmd.SilenceErrors) assert.Equal(t, tc.wantSilent, rootCmd.SilenceUsage) }) From da0af5a2030e5689b273ef62fb1430b8c5adf9fc Mon Sep 17 00:00:00 2001 From: Samuel K Date: Fri, 10 Jul 2026 14:20:15 -0500 Subject: [PATCH 11/11] docs: strip added code comments --- .github/workflows/desktop-ci.yml | 2 -- cmd/root.go | 26 -------------------------- cmd/root_test.go | 9 --------- pkg/devcontainer/single.go | 3 --- pkg/docker/linger.go | 18 ------------------ pkg/flatpak/flatpak.go | 2 -- pkg/workspace/exec.go | 6 ------ 7 files changed, 66 deletions(-) diff --git a/.github/workflows/desktop-ci.yml b/.github/workflows/desktop-ci.yml index 892541d34..f3a8ac5bc 100644 --- a/.github/workflows/desktop-ci.yml +++ b/.github/workflows/desktop-ci.yml @@ -74,8 +74,6 @@ jobs: cache: npm cache-dependency-path: desktop/package-lock.json - # Static build (matches goreleaser): the CLI is injected as the workspace - # agent and must exec in musl-based containers (e.g. alpine). - name: Build CLI (unix) if: runner.os != 'Windows' env: diff --git a/cmd/root.go b/cmd/root.go index 0834b9862..a017ace95 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -70,9 +70,6 @@ func isMachineLogFormat(format string) bool { return format == logOutputJSON || format == logOutputLogfmt } -// logOutputFromArgs extracts the --log-output / --log-format value from raw -// args before cobra binds the persistent flags. Returns "" when absent so -// callers can distinguish an unset flag from an explicit "text". func logOutputFromArgs(args []string) string { for i, arg := range args { for _, name := range []string{flagLogOutput, flagLogFormat} { @@ -87,15 +84,6 @@ func logOutputFromArgs(args []string) string { return "" } -// isMachineConsumer reports whether output is being consumed by a machine -// rather than read by a human at a terminal. Cobra's usage/error text and other -// human affordances are suppressed for machine consumers so they cannot corrupt -// a parsed stream. Signals, in order: -// - the internal subtree drives the agent protocol on stdout; -// - DEVSY_UI marks a process spawned by the desktop app (provenance); -// - an explicit structured --log-output (json/logfmt); -// - as a fallback, output redirected off a terminal (piped) with no explicit -// --log-output, matching conventional CLI behavior. func isMachineConsumer(logOutput string, isInternal bool) bool { switch { case isInternal: @@ -113,7 +101,6 @@ func Execute() { os.Exit(run()) } -// run builds and executes the root command, returning the process exit code. func run() int { rootCmd, globalFlags := BuildRoot() target := rootCmd @@ -151,11 +138,6 @@ func run() int { return 0 } -// configureOutput sets cobra's error/usage silencing and initializes logging, -// returning whether output is machine-consumed. Cobra prints its own -// error/usage only for an interactive human; machine consumers stay silent so -// their stream is not corrupted. Logging is set up before Execute so errors on -// paths that skip PersistentPreRunE still surface. func configureOutput( rootCmd *cobra.Command, globalFlags *flags.GlobalFlags, @@ -192,8 +174,6 @@ func topLevelCommand(cmd *cobra.Command) string { return "" } -// exitCodeForError renders err and returns the process exit code that reflects -// the failure. func exitCodeForError(err error, machineMode bool) int { if err == nil { return 0 @@ -210,8 +190,6 @@ func exitCodeForError(err error, machineMode bool) int { return 1 } -// passthroughExitCode returns the exit status of a subprocess or SSH command -// error verbatim. The second result is false when err is neither. func passthroughExitCode(err error, machineMode bool) (int, bool) { if sshExitErr, ok := errors.AsType[*ssh.ExitError](err); ok { if machineMode { @@ -228,8 +206,6 @@ func passthroughExitCode(err error, machineMode bool) (int, bool) { return 0, false } -// renderCLIError emits the structured cliError in machine mode; in text mode -// cobra already printed the error line, so only hint/doc affordances are added. func renderCLIError(err error, machineMode bool) { cliErr := cliErrors.Classify(err, cliErrors.ClassifyContext{}) if machineMode { @@ -261,8 +237,6 @@ func BuildRoot() (*cobra.Command, *flags.GlobalFlags) { _ = completion.RegisterFlagCompletionFuns(rootCmd, globalFlags) rootCmd.PersistentPreRunE = func(cobraCmd *cobra.Command, _ []string) error { - // Args parsed cleanly by now, so any later error is a runtime failure: - // suppress cobra's usage dump. Parse errors occur before this hook. cobraCmd.SilenceUsage = true log.Init(log.Config{ diff --git a/cmd/root_test.go b/cmd/root_test.go index 6ac53fbf8..1b46fd2c9 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -75,10 +75,6 @@ func TestIsMachineLogFormat(t *testing.T) { assert.False(t, isMachineLogFormat("")) } -// TestIsMachineConsumer locks in the output contract: only an interactive human -// (no machine signal, attached to a terminal) reads human-formatted output. -// The internal subtree (agent protocol), the desktop app (DEVSY_UI), and an -// explicit structured --log-output are all machine consumers. func TestIsMachineConsumer(t *testing.T) { cases := []struct { name string @@ -104,17 +100,12 @@ func TestIsMachineConsumer(t *testing.T) { } } -// TestIsMachineConsumer_TTYFallback verifies that with no explicit signal the -// decision falls back to whether stderr is a terminal. Tests do not run on a -// TTY, so a bare invocation is treated as machine (piped) output. func TestIsMachineConsumer_TTYFallback(t *testing.T) { t.Setenv(config.EnvUI, "") assert.True(t, isMachineConsumer("", false), "non-terminal stderr with no explicit format should be machine mode") } -// TestConfigureOutput_SilencesCobra locks in that configureOutput mirrors the -// machine/human decision into cobra's error/usage silencing. func TestConfigureOutput_SilencesCobra(t *testing.T) { cases := []struct { name string diff --git a/pkg/devcontainer/single.go b/pkg/devcontainer/single.go index 708e4aac6..9b1fdced7 100644 --- a/pkg/devcontainer/single.go +++ b/pkg/devcontainer/single.go @@ -305,9 +305,6 @@ func (r *runner) resolveNewContainer( }, nil } -// lingerWarning surfaces a warning when a newly created container runs under -// rootless Podman without systemd linger, where it would be reaped on session -// end. Returns "" for other drivers/runtimes or when the state cannot be read. func (r *runner) lingerWarning(ctx context.Context) string { dockerDriver, ok := r.driver.(driver.DockerDriver) if !ok { diff --git a/pkg/docker/linger.go b/pkg/docker/linger.go index 37c377f0f..7f75394e8 100644 --- a/pkg/docker/linger.go +++ b/pkg/docker/linger.go @@ -8,15 +8,6 @@ import ( "time" ) -// LingerWarning returns a non-empty warning when the container runtime is -// rootless Podman and systemd linger is disabled for the current user. In that -// configuration systemd reaps the user's containers when the login session -// ends (e.g. SSH disconnect, or the desktop app closing the spawning process), -// so a workspace that started successfully stops on its own — unlike Docker, -// whose system daemon keeps containers alive independently of user sessions. -// -// It returns "" when the situation does not apply or cannot be determined; -// detection never blocks workspace creation. func (r *DockerHelper) LingerWarning(ctx context.Context) string { if !r.IsPodman() || !r.isRootless(ctx) { return "" @@ -29,9 +20,6 @@ func (r *DockerHelper) LingerWarning(ctx context.Context) string { "it running after logout (Docker does not require this)." } -// isRootless reports whether Podman is running rootless. A detection failure is -// treated as rootless, since that is the case the warning targets and a false -// positive only adds a hint. func (r *DockerHelper) isRootless(ctx context.Context) bool { ctx, cancel := context.WithTimeout(ctx, 5*time.Second) defer cancel() @@ -43,14 +31,8 @@ func (r *DockerHelper) isRootless(ctx context.Context) bool { return strings.TrimSpace(string(out)) != "false" } -// lingerDir is the directory where systemd maintains a marker file per user -// with linger enabled. Declared as a var so tests can override it. var lingerDir = "/var/lib/systemd/linger" -// lingerEnabled reports whether systemd linger is enabled for the current user. -// It checks the marker file systemd maintains, avoiding a dependency on -// loginctl being on PATH. A read failure is treated as "enabled" so an -// undetectable state does not produce a spurious warning. func lingerEnabled() bool { u, err := user.Current() if err != nil || u.Username == "" { diff --git a/pkg/flatpak/flatpak.go b/pkg/flatpak/flatpak.go index 4d3490811..e2a08267d 100644 --- a/pkg/flatpak/flatpak.go +++ b/pkg/flatpak/flatpak.go @@ -1,4 +1,3 @@ -// Package flatpak handles running the CLI outside the Flatpak sandbox. package flatpak import ( @@ -28,7 +27,6 @@ func HostBinaryPath() string { return filepath.Join(dataHome, "devsy", "devsy") } -// ReexecOnHost re-executes the current process on the host via `flatpak-spawn --host`. func ReexecOnHost() (bool, error) { if !InSandbox() { return false, nil diff --git a/pkg/workspace/exec.go b/pkg/workspace/exec.go index 3fba2d887..60632e8e0 100644 --- a/pkg/workspace/exec.go +++ b/pkg/workspace/exec.go @@ -55,10 +55,6 @@ func ResolveDockerCommand( } if providerConfig.Agent.Docker.Path != "" { - // agent.docker.path is typically a provider-option template such as - // "${DOCKER_PATH}". Expand it against the workspace's resolved provider - // options (falling back to the OS environment) so the configured value - // is honored — the options are not present in the process environment. if expanded := expandWithOptions( providerConfig.Agent.Docker.Path, workspace.Provider.Options, @@ -70,8 +66,6 @@ func ResolveDockerCommand( return DefaultDockerCommand } -// expandWithOptions expands ${VAR} references in s using the given resolved -// provider options first, then the OS environment. func expandWithOptions(s string, options map[string]config.OptionValue) string { return os.Expand(s, func(key string) string { if opt, ok := options[key]; ok && opt.Value != "" {