fix(delivery): cleanup build files for mounted volume

pkg/agent/delivery/local_docker_test.go

@@ -16,6 +16,12 @@ import (
 
 const testArch = "amd64"
 
+const (
+	testSeedWorkspaceID = "ws1"
+	testSeedVolumeName  = "ws1-workspace"
+	testSeedSourceDir   = "/local/src"
+)
+
 func TestLocalDockerDelivery_Phase(t *testing.T) {
 	d := &LocalDockerDelivery{}
 	assert.Equal(t, PhasePreStart, d.Phase())
@@ -415,3 +421,56 @@ func TestLocalDockerDelivery_Cleanup_RemovesManagedVolumes(t *testing.T) {
 	assert.Contains(t, removed, "devsy-agent-ws1")
 	assert.Contains(t, removed, "ws1-workspace")
 }
+
+func TestLocalDockerDelivery_SeedExcludesBuildInternal(t *testing.T) {
+	tmpDir := t.TempDir()
+	logPath := filepath.Join(tmpDir, "run.log")
+
+	scriptPath := filepath.Join(tmpDir, "fake-docker.sh")
+	script := "#!/bin/sh\n" +
+		"if [ \"$1\" = \"volume\" ] && [ \"$2\" = \"inspect\" ]; then exit 1; fi\n" +
+		"if [ \"$1\" = \"volume\" ] && [ \"$2\" = \"create\" ]; then exit 0; fi\n" +
+		"if [ \"$1\" = \"run\" ]; then echo \"$@\" >> \"" + logPath + "\"; exit 0; fi\n" +
+		"exit 0\n"
+	require.NoError(t, os.WriteFile(scriptPath, []byte(script), 0o600))
+	// #nosec G302 -- test script must be executable
+	require.NoError(t, os.Chmod(scriptPath, 0o755))
+
+	d := &LocalDockerDelivery{DockerCommand: scriptPath}
+	err := d.SeedWorkspaceVolume(context.Background(), WorkspaceSeedOptions{
+		WorkspaceID: testSeedWorkspaceID,
+		VolumeName:  testSeedVolumeName,
+		SourceDir:   testSeedSourceDir,
+	})
+	require.NoError(t, err)
+
+	logged, err := os.ReadFile(logPath) //nolint:gosec // test reads a temp file we control
+	require.NoError(t, err)
+	assert.Contains(t, string(logged), "--exclude")
+	assert.Contains(t, string(logged), ".devsy-internal")
+}
+
+func TestLocalDockerDelivery_Seed_CopyAndCleanupFailureJoined(t *testing.T) {
+	tmpDir := t.TempDir()
+
+	scriptPath := filepath.Join(tmpDir, "fake-docker.sh")
+	script := "#!/bin/sh\n" +
+		"if [ \"$1\" = \"volume\" ] && [ \"$2\" = \"inspect\" ]; then exit 1; fi\n" +
+		"if [ \"$1\" = \"volume\" ] && [ \"$2\" = \"create\" ]; then exit 0; fi\n" +
+		"if [ \"$1\" = \"volume\" ] && [ \"$2\" = \"rm\" ]; then echo 'rm boom' 1>&2; exit 1; fi\n" +
+		"if [ \"$1\" = \"run\" ]; then echo 'copy boom' 1>&2; exit 1; fi\n" +
+		"exit 0\n"
+	require.NoError(t, os.WriteFile(scriptPath, []byte(script), 0o600))
+	// #nosec G302 -- test script must be executable
+	require.NoError(t, os.Chmod(scriptPath, 0o755))
+
+	d := &LocalDockerDelivery{DockerCommand: scriptPath}
+	err := d.SeedWorkspaceVolume(context.Background(), WorkspaceSeedOptions{
+		WorkspaceID: testSeedWorkspaceID,
+		VolumeName:  testSeedVolumeName,
+		SourceDir:   testSeedSourceDir,
+	})
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "seed workspace volume")
+	assert.Contains(t, err.Error(), "remove partial volume")
+}

pkg/agent/delivery/workspace_seed.go

@@ -2,10 +2,12 @@ package delivery
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"strings"
 
 	pkgconfig "github.com/devsy-org/devsy/pkg/config"
+	"github.com/devsy-org/devsy/pkg/devcontainer/config"
 	"github.com/devsy-org/devsy/pkg/log"
 )
 
@@ -52,18 +54,21 @@ func (d *LocalDockerDelivery) SeedWorkspaceVolume(
 	}
 
 	labels := pkgconfig.DockerVolumeLabels(opts.WorkspaceID, pkgconfig.VolumeRoleWorkspace)
+	labels[pkgconfig.DockerSeededLabel] = pkgconfig.LabelValueTrue
 	if err := d.createVolume(ctx, opts.VolumeName, labels); err != nil {
 		return fmt.Errorf("create workspace volume: %w", err)
 	}
 
 	if err := d.copyDirIntoVolume(ctx, opts.SourceDir, opts.VolumeName); err != nil {
-		// Leave the volume in place but unseeded so a retry re-copies.
+		if rmErr := d.removeVolume(ctx, opts.VolumeName); rmErr != nil {
+			return errors.Join(
+				fmt.Errorf("seed workspace volume: %w", err),
+				fmt.Errorf("remove partial volume %s: %w", opts.VolumeName, rmErr),
+			)
+		}
 		return fmt.Errorf("seed workspace volume: %w", err)
 	}
 
-	if err := d.markVolumeSeeded(ctx, opts.VolumeName); err != nil {
-		log.Debugf("failed to mark volume %s seeded: %v", opts.VolumeName, err)
-	}
 	log.Infof("seeded workspace volume %s from %s", opts.VolumeName, opts.SourceDir)
 	return nil
 }
@@ -107,10 +112,6 @@ func (d *LocalDockerDelivery) prepareSeedTarget(
 	return true, nil
 }
 
-// volumeSeedState reports whether the volume is devsy-managed (from its label)
-// and whether it has already been seeded (from a sentinel file written inside
-// the volume after a successful copy). Docker cannot add labels to an existing
-// volume, so seeded state is tracked with the sentinel rather than a label.
 func (d *LocalDockerDelivery) volumeSeedState(
 	ctx context.Context,
 	name string,
@@ -121,68 +122,42 @@ func (d *LocalDockerDelivery) volumeSeedState(
 
 	out, err := d.cmd(ctx,
 		"volume", "inspect",
-		"--format", "{{index .Labels \""+pkgconfig.DockerManagedLabel+"\"}}",
+		"--format", "{{index .Labels \""+pkgconfig.DockerManagedLabel+"\"}},"+
+			"{{index .Labels \""+pkgconfig.DockerSeededLabel+"\"}}",
 		name,
 	).CombinedOutput()
 	if err != nil {
 		return false, false, fmt.Errorf("inspect volume %s: %s: %w", name, string(out), err)
 	}
-	managed = strings.TrimSpace(string(out)) == "true"
-
-	return managed, d.volumeSeeded(ctx, name), nil
-}
-
-// volumeSeeded reports whether the seeded sentinel file exists in the volume.
-func (d *LocalDockerDelivery) volumeSeeded(ctx context.Context, name string) bool {
-	args := []string{
-		cmdRun, flagRM,
-		"-v", name + ":/target:ro",
-		d.helperImageName(),
-		"sh", "-c", "[ -e /target/" + seededSentinel + " ]",
-	}
-	return d.cmd(ctx, args...).Run() == nil
+	managedStr, seededStr, _ := strings.Cut(strings.TrimSpace(string(out)), ",")
+	return managedStr == pkgconfig.LabelValueTrue, seededStr == pkgconfig.LabelValueTrue, nil
 }
 
 func (d *LocalDockerDelivery) volumeExists(ctx context.Context, name string) bool {
 	err := d.cmd(ctx, "volume", "inspect", name).Run()
 	return err == nil
 }
 
-// copyDirIntoVolume copies the contents of sourceDir into the volume root using
-// a throwaway helper container. The source is bind-mounted read-only.
 func (d *LocalDockerDelivery) copyDirIntoVolume(
 	ctx context.Context,
 	sourceDir, volumeName string,
 ) error {
-	args := []string{
-		cmdRun, flagRM,
-		"-v", sourceDir + ":/source:ro",
-		"-v", volumeName + ":/target",
-		d.helperImageName(),
-		"sh", "-c", "cp -a /source/. /target/",
-	}
-	out, err := d.cmd(ctx, args...).CombinedOutput()
-	if err != nil {
-		return fmt.Errorf("%s: %w", string(out), err)
+	var script strings.Builder
+	script.WriteString("cd /source && tar -c")
+	for _, artifact := range config.BuildArtifactExcludes() {
+		script.WriteString(" --exclude='" + artifact + "'")
 	}
-	return nil
-}
-
-// markVolumeSeeded records that the volume has been populated by writing a
-// sentinel file inside it. Docker cannot add labels to an existing volume, so
-// the sentinel is the source of truth for seeded state.
-func (d *LocalDockerDelivery) markVolumeSeeded(ctx context.Context, volumeName string) error {
+	script.WriteString(" . | tar -x -C /target")
 	args := []string{
 		cmdRun, flagRM,
+		"-v", sourceDir + ":/source:ro",
 		"-v", volumeName + ":/target",
 		d.helperImageName(),
-		"sh", "-c", "touch /target/" + seededSentinel,
+		"sh", "-c", script.String(),
 	}
 	out, err := d.cmd(ctx, args...).CombinedOutput()
 	if err != nil {
 		return fmt.Errorf("%s: %w", string(out), err)
 	}
 	return nil
 }
-
-const seededSentinel = ".devsy-seeded"

pkg/agent/tunnelserver/tunnelserver.go

@@ -453,6 +453,8 @@ func (t *tunnelServer) StreamWorkspace(
 		}
 	}
 
+	excludes = append(excludes, config.BuildArtifactExcludes()...)
+
 	buf := bufio.NewWriterSize(NewStreamWriter(stream), 10*1024)
 	err = extract.WriteTarExclude(buf, t.workspace.Source.LocalFolder, false, excludes)
 	if err != nil {
@@ -497,6 +499,8 @@ func (t *tunnelServer) StreamMount(
 		}
 	}
 
+	excludes = append(excludes, config.BuildArtifactExcludes()...)
+
 	buf := bufio.NewWriterSize(NewStreamWriter(stream), 10*1024)
 	err := extract.WriteTarExclude(buf, mount.Source, false, excludes)
 	if err != nil {

pkg/devcontainer/build.go

@@ -672,8 +672,7 @@ func getContainerContextAndDockerfile(
 }
 
 func cleanupBuildInformation(c *config.DevContainerConfig) {
-	contextPath := config.GetContextPath(c)
-	_ = os.RemoveAll(filepath.Join(contextPath, config.DevsyContextFeatureFolder))
+	config.RemoveBuildArtifacts(config.GetContextPath(c))
 }
 
 func featureSecretOpts(options provider.BuildOptions) *feature.SecretOptions {

pkg/devcontainer/config/artifacts.go

@@ -0,0 +1,25 @@
+package config
+
+import (
+	"os"
+	"path/filepath"
+
+	"github.com/devsy-org/devsy/pkg/log"
+)
+
+var buildArtifactNames = []string{
+	DevsyContextFeatureFolder,
+}
+
+func BuildArtifactExcludes() []string {
+	return append([]string(nil), buildArtifactNames...)
+}
+
+func RemoveBuildArtifacts(contextPath string) {
+	for _, name := range buildArtifactNames {
+		path := filepath.Join(contextPath, name)
+		if err := os.RemoveAll(path); err != nil {
+			log.Debugf("failed to remove build artifact %s: %v", path, err)
+		}
+	}
+}

pkg/devcontainer/config/artifacts_test.go

@@ -0,0 +1,53 @@
+package config
+
+import (
+	"os"
+	"path/filepath"
+	"slices"
+	"testing"
+)
+
+func TestBuildArtifactExcludesIncludesFeatureFolder(t *testing.T) {
+	got := BuildArtifactExcludes()
+	if !slices.Contains(got, DevsyContextFeatureFolder) {
+		t.Errorf("expected %q in excludes, got %v", DevsyContextFeatureFolder, got)
+	}
+}
+
+func TestBuildArtifactExcludesReturnsCopy(t *testing.T) {
+	got := BuildArtifactExcludes()
+	if len(got) == 0 {
+		t.Fatal("expected at least one artifact")
+	}
+	got[0] = "mutated"
+	if BuildArtifactExcludes()[0] == "mutated" {
+		t.Error("BuildArtifactExcludes must return a defensive copy")
+	}
+}
+
+func TestRemoveBuildArtifacts(t *testing.T) {
+	contextPath := t.TempDir()
+	artifactDir := filepath.Join(contextPath, DevsyContextFeatureFolder)
+	// #nosec G301 -- test fixture
+	if err := os.MkdirAll(filepath.Join(artifactDir, "0"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	keep := filepath.Join(contextPath, "keep.txt")
+	// #nosec G306 -- test fixture
+	if err := os.WriteFile(keep, []byte("x"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	RemoveBuildArtifacts(contextPath)
+
+	if _, err := os.Stat(artifactDir); !os.IsNotExist(err) {
+		t.Errorf("expected artifact dir removed, stat err = %v", err)
+	}
+	if _, err := os.Stat(keep); err != nil {
+		t.Errorf("workspace file should survive, got %v", err)
+	}
+}
+
+func TestRemoveBuildArtifactsAbsentIsNoError(t *testing.T) {
+	RemoveBuildArtifacts(t.TempDir())
+}

pkg/devcontainer/config/prebuild.go

@@ -42,7 +42,9 @@ func CalculatePrebuildHash(params PrebuildHashParams) (string, error) {
 		log.Debugf("failed to read .dockerignore: %v", err)
 		return "", fmt.Errorf("failed to read dockerignore: %w", err)
 	}
-	excludes = append(excludes, DevsyContextFeatureFolder+"/")
+	for _, artifact := range BuildArtifactExcludes() {
+		excludes = append(excludes, artifact+"/")
+	}
 
 	var includes []string
 	if params.BuildInfo != nil && params.BuildInfo.Dockerfile != nil {

pkg/devcontainer/prebuild.go

@@ -3,8 +3,6 @@ package devcontainer
 import (
 	"context"
 	"fmt"
-	"os"
-	"path/filepath"
 	"strings"
 
 	"github.com/devsy-org/devsy/pkg/devcontainer/build"
@@ -27,11 +25,7 @@ func (r *runner) Build(ctx context.Context, options provider.BuildOptions) (stri
 
 	prebuildRepo := getPrebuildRepository(substitutedConfig)
 
-	// remove build information
-	defer func() {
-		contextPath := config.GetContextPath(substitutedConfig.Config)
-		_ = os.RemoveAll(filepath.Join(contextPath, config.DevsyContextFeatureFolder))
-	}()
+	defer config.RemoveBuildArtifacts(config.GetContextPath(substitutedConfig.Config))
 
 	// check if we need to build container
 	buildInfo, err := r.build(ctx, substitutedConfig, substitutionContext, options)