fix(delivery): clean up agent delivery volumes on workspace deletion

pkg/agent/delivery/factory.go

@@ -16,6 +16,7 @@ type FactoryOptions struct {
 	WorkspaceID     string
 	DockerCommand   string
 	DockerEnv       []string
+	HelperImage     string
 	IsRemoteDocker  bool
 	ContainerID     string
 	ExecFunc        inject.ExecFunc
@@ -46,6 +47,7 @@ func NewAgentDelivery(opts FactoryOptions) AgentDelivery {
 			return &LocalDockerDelivery{
 				DockerCommand: opts.DockerCommand,
 				Environment:   opts.DockerEnv,
+				HelperImage:   opts.HelperImage,
 			}
 		}
 		log.Debugf("using remote docker delivery for non-local docker daemon")

pkg/agent/delivery/factory_test.go

@@ -17,7 +17,7 @@ func TestNewAgentDelivery_LocalDocker(t *testing.T) {
 				Driver: provider.DockerDriver,
 			},
 		},
-		DockerCommand: "docker",
+		DockerCommand: defaultDockerCmd,
 	}
 
 	d := NewAgentDelivery(opts)

pkg/agent/delivery/local_docker.go

@@ -1,10 +1,14 @@
 package delivery
 
 import (
+	"bytes"
 	"context"
 	"fmt"
+	"io"
 	"os"
 	"os/exec"
+	"path/filepath"
+	"strings"
 
 	"github.com/devsy-org/devsy/pkg/agent"
 	"github.com/devsy-org/devsy/pkg/devcontainer/config"
@@ -14,15 +18,16 @@ import (
 var _ AgentDelivery = (*LocalDockerDelivery)(nil)
 
 const (
-	defaultDockerCmd = "docker"
-	volumePrefix     = "devsy-agent-"
-	volumeMountPath  = "/opt/devsy"
-	helperImage      = "busybox:latest"
+	defaultDockerCmd   = "docker"
+	volumePrefix       = "devsy-agent-"
+	volumeMountPath    = "/opt/devsy"
+	defaultHelperImage = "busybox:latest"
 )
 
 type LocalDockerDelivery struct {
 	DockerCommand string
 	Environment   []string
+	HelperImage   string
 }
 
 func (d *LocalDockerDelivery) Phase() DeliveryPhase {
@@ -77,6 +82,13 @@ func (d *LocalDockerDelivery) createVolume(ctx context.Context, name string) err
 	return nil
 }
 
+func (d *LocalDockerDelivery) helperImageName() string {
+	if d.HelperImage != "" {
+		return d.HelperImage
+	}
+	return defaultHelperImage
+}
+
 func (d *LocalDockerDelivery) populateVolume(
 	ctx context.Context,
 	volumeName string,
@@ -89,6 +101,25 @@ func (d *LocalDockerDelivery) populateVolume(
 	}
 	defer func() { _ = binary.Close() }()
 
+	data, err := io.ReadAll(binary)
+	if err != nil {
+		return fmt.Errorf("read binary: %w", err)
+	}
+
+	err = d.populateVolumeWithHelper(ctx, volumeName, bytes.NewReader(data))
+	if err == nil {
+		return nil
+	}
+	log.Debugf("helper container populate failed, trying direct copy: %v", err)
+
+	return d.populateVolumeDirectCopy(ctx, volumeName, data)
+}
+
+func (d *LocalDockerDelivery) populateVolumeWithHelper(
+	ctx context.Context,
+	volumeName string,
+	binary io.Reader,
+) error {
 	containerName := "devsy-agent-init-" + volumeName
 	script := fmt.Sprintf(
 		"cat > %s/%s && chmod 755 %s/%s",
@@ -99,7 +130,7 @@ func (d *LocalDockerDelivery) populateVolume(
 		"--name", containerName,
 		"-v", volumeName + ":" + volumeMountPath,
 		"-i",
-		helperImage,
+		d.helperImageName(),
 		"sh", "-c", script,
 	}
 
@@ -113,6 +144,44 @@ func (d *LocalDockerDelivery) populateVolume(
 	return nil
 }
 
+func (d *LocalDockerDelivery) populateVolumeDirectCopy(
+	ctx context.Context,
+	volumeName string,
+	data []byte,
+) error {
+	mountpoint, err := d.volumeMountpoint(ctx, volumeName)
+	if err != nil {
+		return fmt.Errorf("inspect volume mountpoint: %w", err)
+	}
+
+	destPath := filepath.Join(mountpoint, binaryName())
+
+	if err := os.WriteFile(destPath, data, 0o600); err != nil {
+		return fmt.Errorf("write binary to volume: %w", err)
+	}
+	// #nosec G302 -- agent binary must be executable
+	if err := os.Chmod(destPath, 0o755); err != nil {
+		return fmt.Errorf("chmod binary: %w", err)
+	}
+
+	return nil
+}
+
+func (d *LocalDockerDelivery) volumeMountpoint(
+	ctx context.Context,
+	volumeName string,
+) (string, error) {
+	out, err := d.cmd(
+		ctx, "volume", "inspect",
+		"--format", "{{.Mountpoint}}",
+		volumeName,
+	).CombinedOutput()
+	if err != nil {
+		return "", fmt.Errorf("%s: %w", string(out), err)
+	}
+	return strings.TrimSpace(string(out)), nil
+}
+
 func (d *LocalDockerDelivery) removeVolume(ctx context.Context, workspaceID string) error {
 	volumeName := volumePrefix + workspaceID
 	out, err := d.cmd(ctx, "volume", "rm", "-f", volumeName).CombinedOutput()

pkg/agent/delivery/local_docker_test.go

@@ -1,9 +1,14 @@
 package delivery
 
 import (
+	"bytes"
 	"context"
+	"io"
+	"os"
+	"path/filepath"
 	"testing"
 
+	"github.com/devsy-org/devsy/pkg/provider"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -37,3 +42,85 @@ func TestBinaryName(t *testing.T) {
 	name := binaryName()
 	assert.Equal(t, "devsy", name)
 }
+
+func TestLocalDockerDelivery_HelperImageName_Default(t *testing.T) {
+	d := &LocalDockerDelivery{}
+	assert.Equal(t, "busybox:latest", d.helperImageName())
+}
+
+func TestLocalDockerDelivery_HelperImageName_Configured(t *testing.T) {
+	d := &LocalDockerDelivery{HelperImage: "registry.internal/tools/busybox:1.36"}
+	assert.Equal(t, "registry.internal/tools/busybox:1.36", d.helperImageName())
+}
+
+func TestNewAgentDelivery_LocalDocker_ThreadsHelperImage(t *testing.T) {
+	opts := FactoryOptions{
+		WorkspaceConfig: &provider.AgentWorkspaceInfo{
+			Agent: provider.ProviderAgentConfig{
+				Driver: provider.DockerDriver,
+			},
+		},
+		DockerCommand: defaultDockerCmd,
+		HelperImage:   "my-registry/busybox:1.35",
+	}
+
+	d := NewAgentDelivery(opts)
+	local, ok := d.(*LocalDockerDelivery)
+	require.True(t, ok)
+	assert.Equal(t, "my-registry/busybox:1.35", local.HelperImage)
+}
+
+func TestNewAgentDelivery_LocalDocker_EmptyHelperImage(t *testing.T) {
+	opts := FactoryOptions{
+		WorkspaceConfig: &provider.AgentWorkspaceInfo{
+			Agent: provider.ProviderAgentConfig{
+				Driver: provider.DockerDriver,
+			},
+		},
+		DockerCommand: defaultDockerCmd,
+	}
+
+	d := NewAgentDelivery(opts)
+	local, ok := d.(*LocalDockerDelivery)
+	require.True(t, ok)
+	assert.Empty(t, local.HelperImage)
+	assert.Equal(t, "busybox:latest", local.helperImageName())
+}
+
+func TestPopulateVolume_FallbackToDirectCopy(t *testing.T) {
+	tmpDir := t.TempDir()
+	mountDir := filepath.Join(tmpDir, "mount")
+	require.NoError(t, os.MkdirAll(mountDir, 0o750))
+
+	scriptPath := filepath.Join(tmpDir, "fake-docker.sh")
+	script := "#!/bin/sh\n" +
+		"case \"$1\" in\n" +
+		"  run) echo \"image not found\" >&2; exit 1 ;;\n" +
+		"  volume) echo \"" + mountDir + "\" ;;\n" +
+		"  *) exit 1 ;;\n" +
+		"esac\n"
+	require.NoError(t, os.WriteFile(scriptPath, []byte(script), 0o600))
+	// #nosec G302 -- test script must be executable
+	require.NoError(t, os.Chmod(scriptPath, 0o755))
+
+	binaryContent := []byte("fake-agent-binary-content")
+	binarySource := func(_ context.Context, _ string) (io.ReadCloser, error) {
+		return io.NopCloser(bytes.NewReader(binaryContent)), nil
+	}
+
+	d := &LocalDockerDelivery{
+		DockerCommand: scriptPath,
+	}
+
+	err := d.populateVolume(context.Background(), "test-vol", binarySource, "amd64")
+	require.NoError(t, err)
+
+	destPath := filepath.Join(mountDir, binaryName())
+	data, err := os.ReadFile(destPath) //nolint:gosec // test reads from a temp directory we control
+	require.NoError(t, err)
+	assert.Equal(t, binaryContent, data)
+
+	info, err := os.Stat(destPath)
+	require.NoError(t, err)
+	assert.Equal(t, os.FileMode(0o755), info.Mode().Perm())
+}

pkg/devcontainer/delete.go

@@ -13,7 +13,9 @@ func (r *runner) Delete(ctx context.Context, options DeleteOptions) error {
 	containerDetails, err := r.Driver.FindDevContainer(ctx, r.ID)
 	if err != nil {
 		return fmt.Errorf("find dev container: %w", err)
-	} else if containerDetails == nil {
+	}
+	defer r.cleanupDeliveryVolume(ctx)
+	if containerDetails == nil {
 		return nil
 	}
 
@@ -40,6 +42,13 @@ func (r *runner) Delete(ctx context.Context, options DeleteOptions) error {
 	return nil
 }
 
+func (r *runner) cleanupDeliveryVolume(ctx context.Context) {
+	strategy := r.newAgentDelivery()
+	if err := strategy.Cleanup(ctx, r.ID); err != nil {
+		log.Debugf("best-effort agent delivery volume cleanup: %v", err)
+	}
+}
+
 func (r *runner) Stop(ctx context.Context) error {
 	containerDetails, err := r.Driver.FindDevContainer(ctx, r.ID)
 	if err != nil {