Merge pull request #2301 from dgageot/fix-sandbox

Fix `docker run --sandbox`

Author: dgageot

cmd/root/run.go

@@ -144,9 +144,8 @@ func (f *runExecFlags) runRunCommand(cmd *cobra.Command, args []string) (command
 		}()
 	}
 
-	// If --sandbox is set, delegate everything to docker sandbox.
 	if f.sandbox {
-		return runInSandbox(cmd, &f.runConfig, f.sandboxTemplate)
+		return runInSandbox(ctx, cmd, args, &f.runConfig, f.sandboxTemplate)
 	}
 
 	out := cli.NewPrinter(cmd.OutOrStdout())

cmd/root/sandbox.go

@@ -2,14 +2,17 @@ package root
 
 import (
 	"cmp"
+	"context"
 	"errors"
 	"fmt"
 	"log/slog"
 	"os"
 	"os/exec"
+	"path/filepath"
 
 	"github.com/docker/cli/cli"
 	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
 
 	"github.com/docker/docker-agent/pkg/config"
 	"github.com/docker/docker-agent/pkg/environment"
@@ -20,30 +23,35 @@ import (
 // runInSandbox delegates the current command to a Docker sandbox.
 // It ensures a sandbox exists (creating or recreating as needed), then
 // executes docker agent inside it via `docker sandbox exec`.
-func runInSandbox(cmd *cobra.Command, runConfig *config.RuntimeConfig, template string) error {
+func runInSandbox(ctx context.Context, cmd *cobra.Command, args []string, runConfig *config.RuntimeConfig, template string) error {
 	if environment.InSandbox() {
 		return fmt.Errorf("already running inside a Docker sandbox (VM %s)", os.Getenv("SANDBOX_VM_ID"))
 	}
 
-	ctx := cmd.Context()
 	if err := sandbox.CheckAvailable(ctx); err != nil {
 		return err
 	}
 
-	dockerAgentArgs := sandbox.BuildCagentArgs(os.Args)
-	agentRef := sandbox.AgentRefFromArgs(dockerAgentArgs)
-	configDir := paths.GetConfigDir()
+	var agentRef string
+	if len(args) > 0 {
+		agentRef = args[0]
+	}
 
-	// Always forward config directory paths so the sandbox-side
-	// docker agent resolves it to the same host directories
-	// (which is mounted read-write by ensureSandbox).
-	dockerAgentArgs = sandbox.AppendFlagIfMissing(dockerAgentArgs, "--config-dir", configDir)
+	configDir := paths.GetConfigDir()
+	dockerAgentArgs := dockerAgentArgs(cmd, args, configDir)
+	dockerAgentArgs = append(dockerAgentArgs, args...)
+	dockerAgentArgs = append(dockerAgentArgs, "--config-dir", configDir)
 
 	stopTokenWriter := sandbox.StartTokenWriterIfNeeded(ctx, configDir, runConfig.ModelsGateway)
 	defer stopTokenWriter()
 
-	// Ensure a sandbox with the right workspace mounts exists.
-	wd := cmp.Or(runConfig.WorkingDir, ".")
+	// Resolve wd to an absolute path so that it matches the absolute
+	// workspace paths returned by `docker sandbox ls --json`.
+	wd, err := filepath.Abs(cmp.Or(runConfig.WorkingDir, "."))
+	if err != nil {
+		return fmt.Errorf("resolving workspace path: %w", err)
+	}
+
 	name, err := sandbox.Ensure(ctx, wd, sandbox.ExtraWorkspace(wd, agentRef), template, configDir)
 	if err != nil {
 		return err
@@ -60,7 +68,7 @@ func runInSandbox(cmd *cobra.Command, runConfig *config.RuntimeConfig, template
 		envFlags = append(envFlags, "-e", envModelsGateway+"="+gateway)
 	}
 
-	dockerCmd := sandbox.BuildExecCmd(ctx, name, dockerAgentArgs, envFlags, envVars)
+	dockerCmd := sandbox.BuildExecCmd(ctx, name, wd, dockerAgentArgs, envFlags, envVars)
 	slog.Debug("Executing in sandbox", "name", name, "args", dockerCmd.Args)
 
 	if err := dockerCmd.Run(); err != nil {
@@ -72,3 +80,31 @@ func runInSandbox(cmd *cobra.Command, runConfig *config.RuntimeConfig, template
 
 	return nil
 }
+
+func dockerAgentArgs(cmd *cobra.Command, args []string, configDir string) []string {
+	var dockerAgentArgs []string
+	hasYolo := false
+	cmd.Flags().Visit(func(f *pflag.Flag) {
+		if f.Name == "sandbox" || f.Name == "config-dir" {
+			return
+		}
+
+		if f.Name == "yolo" {
+			hasYolo = true
+		}
+
+		if f.Value.Type() == "bool" {
+			dockerAgentArgs = append(dockerAgentArgs, "--"+f.Name)
+		} else {
+			dockerAgentArgs = append(dockerAgentArgs, "--"+f.Name, f.Value.String())
+		}
+	})
+	if !hasYolo {
+		dockerAgentArgs = append(dockerAgentArgs, "--yolo")
+	}
+
+	dockerAgentArgs = append(dockerAgentArgs, args...)
+	dockerAgentArgs = append(dockerAgentArgs, "--config-dir", configDir)
+
+	return dockerAgentArgs
+}

go.mod

@@ -193,7 +193,7 @@ require (
 	github.com/sergi/go-diff v1.4.0 // indirect
 	github.com/sirupsen/logrus v1.9.4 // indirect
 	github.com/skeema/knownhosts v1.3.1 // indirect
-	github.com/spf13/pflag v1.0.10 // indirect
+	github.com/spf13/pflag v1.0.10
 	github.com/stretchr/objx v0.5.2 // indirect
 	github.com/tidwall/gjson v1.18.0 // indirect
 	github.com/tidwall/match v1.1.1 // indirect

pkg/sandbox/args.go

@@ -1,119 +1,13 @@
 package sandbox
 
 import (
-	"log/slog"
 	"os"
 	"path/filepath"
 	"strings"
 
-	"github.com/docker/cli/cli-plugins/plugin"
-
 	"github.com/docker/docker-agent/pkg/userconfig"
 )
 
-// Flags stripped when forwarding args to the sandbox.
-var (
-	// stripFlags are standalone boolean flags that don't take a value.
-	stripFlags = map[string]bool{
-		"--sandbox": true,
-		"--debug":   true,
-		"-d":        true,
-	}
-	// stripFlagsWithValue are flags followed by a separate value argument.
-	// They are also recognised in --flag=value form.
-	stripFlagsWithValue = map[string]bool{
-		"--log-file":       true,
-		"--template":       true,
-		"--models-gateway": true,
-	}
-)
-
-// BuildCagentArgs takes os.Args and returns the arguments to pass after
-// "--" to the sandbox. It strips the binary name, "--sandbox", and the first
-// occurrence of the "run" subcommand. If the agent reference is a user-defined
-// alias, it is resolved to its path so the sandbox (which lacks the host's
-// user config) receives a concrete reference.
-// It also injects --yolo since the sandbox provides isolation.
-// Host-only flags --debug/-d, --log-file, --template, and --models-gateway
-// are stripped because they reference host paths/logging, sandbox creation
-// options, or settings forwarded via env var that don't apply inside the
-// sandbox.
-//
-//	["cagent", "run", "./agent.yaml", "--sandbox", "--debug"] → ["./agent.yaml", "--yolo"]
-//	["cagent", "--debug", "run", "--sandbox", "myalias"]      → ["/path/to/agent.yaml", "--yolo"]
-func BuildCagentArgs(argv []string) []string {
-	out := make([]string, 0, len(argv))
-	runStripped := false
-	agentStripped := false
-	agentResolved := false
-	hasYolo := false
-	skipNext := false
-	for _, a := range argv[1:] { // skip binary name
-		if skipNext {
-			skipNext = false
-			continue
-		}
-		if stripFlags[a] {
-			continue
-		}
-		if stripFlagsWithValue[a] {
-			skipNext = true
-			continue
-		}
-		if isEqualsFormOf(a, stripFlagsWithValue) {
-			continue
-		}
-		if a == "--yolo" {
-			hasYolo = true
-		}
-		if !runStripped && a == "run" {
-			runStripped = true
-			continue
-		}
-		if !plugin.RunningStandalone() && !agentStripped && a == "agent" {
-			agentStripped = true
-			continue
-		}
-		// The first positional arg after "run" is the agent reference.
-		// Resolve it if it's a user-defined alias.
-		if runStripped && !agentResolved && !strings.HasPrefix(a, "-") {
-			agentResolved = true
-			if resolved := ResolveAlias(a); resolved != "" {
-				slog.Debug("Resolved alias for sandbox", "alias", a, "path", resolved)
-				a = resolved
-			}
-		}
-		out = append(out, a)
-	}
-	// The sandbox provides isolation, so auto-approve all tool calls.
-	if !hasYolo {
-		out = append(out, "--yolo")
-	}
-	return out
-}
-
-// isEqualsFormOf reports whether arg matches "--flag=..." for any flag in the set.
-func isEqualsFormOf(arg string, flags map[string]bool) bool {
-	for f := range flags {
-		if strings.HasPrefix(arg, f+"=") {
-			return true
-		}
-	}
-	return false
-}
-
-// AgentRefFromArgs returns the first positional (non-flag) argument from the
-// docker-agent arg list, which is the agent file reference. Returns "" if there are
-// no positional arguments.
-func AgentRefFromArgs(args []string) string {
-	for _, a := range args {
-		if !strings.HasPrefix(a, "-") {
-			return a
-		}
-	}
-	return ""
-}
-
 // ResolveAlias returns the alias path if name is a user-defined alias,
 // or an empty string otherwise.
 func ResolveAlias(name string) string {
@@ -167,14 +61,3 @@ func looksLikeLocalFile(path string) bool {
 	info, err := os.Stat(path)
 	return err == nil && !info.IsDir()
 }
-
-// AppendFlagIfMissing appends "--flag value" to args unless args already
-// contain the flag (in either "--flag value" or "--flag=value" form).
-func AppendFlagIfMissing(args []string, flag, value string) []string {
-	for _, a := range args {
-		if a == flag || strings.HasPrefix(a, flag+"=") {
-			return args
-		}
-	}
-	return append(args, flag, value)
-}

pkg/sandbox/sandbox.go

@@ -106,7 +106,7 @@ func Ensure(ctx context.Context, wd, extra, template, configDir string) (string,
 		createArgs = append(createArgs, "-t", template)
 	}
 	createArgs = append(createArgs, "cagent", wd)
-	if extra != "" {
+	if extra != "" && extra != wd {
 		createArgs = append(createArgs, extra+":ro")
 	}
 	// Mount config directory read-only so the sandbox can
@@ -133,8 +133,8 @@ func Ensure(ctx context.Context, wd, extra, template, configDir string) (string,
 }
 
 // BuildExecCmd assembles the `docker sandbox exec` command.
-func BuildExecCmd(ctx context.Context, name string, cagentArgs, envFlags, envVars []string) *exec.Cmd {
-	execArgs := []string{"sandbox", "exec", "-it"}
+func BuildExecCmd(ctx context.Context, name, wd string, cagentArgs, envFlags, envVars []string) *exec.Cmd {
+	execArgs := []string{"sandbox", "exec", "-it", "-w", wd}
 	execArgs = append(execArgs, envFlags...)
 	execArgs = append(execArgs, name, "cagent", "run")
 	execArgs = append(execArgs, cagentArgs...)