fix(review round 2): daemon symlink tars, orphan-safe removes, unique seed names, git ctx, IsErrNotFound, collision-free slugs, branch validation; web first-run WS, draft reset, IME guard, churn fix, test swap repair; 106 daemon + 190 web tests green

daemon/docker.go

@@ -7,6 +7,7 @@ import (
 	"bytes"
 	"context"
 	"crypto/rand"
+	"crypto/sha256"
 	"encoding/hex"
 	"encoding/json"
 	"errors"
@@ -22,6 +23,7 @@ import (
 	"sync"
 	"time"
 
+	cerrdefs "github.com/containerd/errdefs"
 	"github.com/docker/docker/api/types/build"
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/filters"
@@ -69,10 +71,20 @@ var errNoContainer = errors.New("no container for session")
 
 var repoPathRe = regexp.MustCompile(`^[A-Za-z0-9_.-]+(/[A-Za-z0-9_.-]+)+$`)
 
+// branchRe guards spawn branch names against git option injection.
+var branchRe = regexp.MustCompile(`^[A-Za-z0-9._/-]+$`)
+
 // validRepoPath accepts "group/project" style paths (at least two segments).
 func validRepoPath(repo string) bool { return repoPathRe.MatchString(repo) }
 
-func repoSlug(repo string) string { return strings.ReplaceAll(repo, "/", "--") }
+// repoSlug maps a repo path to a collision-free filesystem/volume name:
+// the `/`→`--` form alone is ambiguous ("a/b--c" vs "a/b/c"), so the
+// first 6 hex chars of the sha256 of the full path disambiguate. Changing
+// this changes existing volume names (one-time fresh clone per repo).
+func repoSlug(repo string) string {
+	sum := sha256.Sum256([]byte(repo))
+	return strings.ReplaceAll(repo, "/", "--") + "-" + hex.EncodeToString(sum[:3])
+}
 
 // newUUID returns a random RFC 4122 v4 UUID string.
 func newUUID() string {
@@ -214,11 +226,11 @@ func (s *Spawner) deleteJob(sessionID string) {
 
 // Start launches the async spawn pipeline and returns the new sessionId.
 func (s *Spawner) Start(ctx context.Context, repo, branch string) (SpawnResult, error) {
-	if _, err := s.imageExists(ctx); err != nil {
+	exists, err := s.imageExists(ctx)
+	if err != nil {
 		return SpawnResult{}, fmt.Errorf("docker unavailable: %w", err)
 	}
-	exists, err := s.imageExists(ctx)
-	if err == nil && !exists {
+	if !exists {
 		if _, statErr := os.Stat(s.dockerfile); statErr != nil {
 			return SpawnResult{}, fmt.Errorf(
 				"image %s not found and no worker Dockerfile at %s (set %s or run `make worker-image`)",
@@ -258,7 +270,7 @@ func (s *Spawner) runJob(repo, branch, sessionID string) {
 	lock.Lock()
 	defer lock.Unlock()
 
-	if err := s.cloneOrUpdate(repo, branch, slug); err != nil {
+	if err := s.cloneOrUpdate(s.ctx, repo, branch, slug); err != nil {
 		s.setJob(sessionID, repo, stateError, "", err.Error())
 		return
 	}
@@ -283,7 +295,7 @@ func (s *Spawner) runJob(repo, branch, sessionID string) {
 // cloneOrUpdate clones the repo into reposDir/<slug> or fast-forwards it.
 // Credentials never appear in the clone URL (which git persists into
 // .git/config); auth is passed per-invocation via http.extraHeader.
-func (s *Spawner) cloneOrUpdate(repo, branch, slug string) error {
+func (s *Spawner) cloneOrUpdate(ctx context.Context, repo, branch, slug string) error {
 	dir := filepath.Join(s.reposDir, slug)
 	cloneURL, err := s.cloneURL(repo)
 	if err != nil {
@@ -291,15 +303,15 @@ func (s *Spawner) cloneOrUpdate(repo, branch, slug string) error {
 	}
 	if st, err := os.Stat(filepath.Join(dir, ".git")); err == nil && st.IsDir() {
 		auth := s.gitAuthArgs()
-		if branch != "" && gitBranch(dir) != branch {
-			if err := gitRun(dir, append(append([]string{}, auth...), "fetch", "origin", branch)...); err != nil {
+		if branch != "" && gitBranch(ctx, dir) != branch {
+			if err := gitRun(ctx, dir, append(append([]string{}, auth...), "fetch", "origin", branch)...); err != nil {
 				return fmt.Errorf("git fetch %s %s: %w", repo, branch, err)
 			}
-			if err := gitRun(dir, "checkout", branch); err != nil {
+			if err := gitRun(ctx, dir, "checkout", branch); err != nil {
 				return fmt.Errorf("git checkout %s: %w", repo, err)
 			}
 		}
-		if err := gitRun(dir, append(append([]string{}, auth...), "pull", "--ff-only")...); err != nil {
+		if err := gitRun(ctx, dir, append(append([]string{}, auth...), "pull", "--ff-only")...); err != nil {
 			return fmt.Errorf("git pull %s: %w", repo, err)
 		}
 		return nil
@@ -312,7 +324,7 @@ func (s *Spawner) cloneOrUpdate(repo, branch, slug string) error {
 		args = append(args, "--branch", branch)
 	}
 	args = append(args, "--", cloneURL, dir)
-	if err := gitRun("", args...); err != nil {
+	if err := gitRun(ctx, "", args...); err != nil {
 		return fmt.Errorf("git clone %s: %w", repo, err)
 	}
 	return nil
@@ -338,16 +350,16 @@ func (s *Spawner) gitAuthArgs() []string {
 }
 
 // gitBranch returns the checked-out branch of an existing clone, "" on failure.
-func gitBranch(dir string) string {
-	out, err := exec.Command("git", "-C", dir, "rev-parse", "--abbrev-ref", "HEAD").Output()
+func gitBranch(ctx context.Context, dir string) string {
+	out, err := exec.CommandContext(ctx, "git", "-C", dir, "rev-parse", "--abbrev-ref", "HEAD").Output()
 	if err != nil {
 		return ""
 	}
 	return strings.TrimSpace(string(out))
 }
 
-func gitRun(dir string, args ...string) error {
-	cmd := exec.Command("git", args...)
+func gitRun(ctx context.Context, dir string, args ...string) error {
+	cmd := exec.CommandContext(ctx, "git", args...)
 	if dir != "" {
 		cmd.Dir = dir
 	}
@@ -445,7 +457,7 @@ func (s *Spawner) createAndStart(ctx context.Context, repo, slug, sessionID stri
 		return "", err
 	}
 	if fresh {
-		if err := s.seedVolume(ctx, slug, repoVolume); err != nil {
+		if err := s.seedVolume(ctx, slug, repoVolume, sessionID); err != nil {
 			// drop the half-seeded volume so the next spawn retries fresh
 			// instead of silently booting into an empty workspace.
 			if rmErr := s.cli.VolumeRemove(context.Background(), repoVolume, true); rmErr != nil {
@@ -497,9 +509,14 @@ func (s *Spawner) createAndStart(ctx context.Context, repo, slug, sessionID stri
 }
 
 // ensureRepoVolume creates the per-repo volume; reports whether it is fresh.
+// Only a definitive "no such volume" (docker errdefs NotFound) counts as
+// fresh: a transient inspect error must never re-seed over an existing
+// volume, so it propagates instead.
 func (s *Spawner) ensureRepoVolume(ctx context.Context, name string) (bool, error) {
 	if _, err := s.cli.VolumeInspect(ctx, name); err == nil {
 		return false, nil
+	} else if !cerrdefs.IsNotFound(err) {
+		return false, fmt.Errorf("volume %s: %w", name, err)
 	}
 	if _, err := s.cli.VolumeCreate(ctx, volume.CreateOptions{Name: name}); err != nil {
 		return false, fmt.Errorf("volume %s: %w", name, err)
@@ -511,7 +528,7 @@ func (s *Spawner) ensureRepoVolume(ctx context.Context, name string) (bool, erro
 // streaming a tar of the clone into a paused container via the docker API
 // (CopyToContainer). No bind mounts: bind sources are HOST paths, but the
 // clone lives inside the daemon container's filesystem.
-func (s *Spawner) seedVolume(ctx context.Context, slug, repoVolume string) error {
+func (s *Spawner) seedVolume(ctx context.Context, slug, repoVolume, sessionID string) error {
 	repoDir := filepath.Join(s.reposDir, slug)
 	cfg := &container.Config{
 		Image: imageRefWorker,
@@ -521,7 +538,14 @@ func (s *Spawner) seedVolume(ctx context.Context, slug, repoVolume string) error
 		Binds: []string{repoVolume + ":" + workspaceMount},
 		Init:  &[]bool{true}[0],
 	}
-	created, err := s.cli.ContainerCreate(ctx, cfg, hostCfg, nil, nil, "lvmh-seed-"+slug)
+	// Unique name per session: a fixed name would conflict forever after a
+	// crash between create and the deferred remove.
+	unq := strings.ReplaceAll(sessionID, "-", "")
+	if len(unq) > 8 {
+		unq = unq[:8]
+	}
+	name := "lvmh-seed-" + slug + "-" + unq
+	created, err := s.cli.ContainerCreate(ctx, cfg, hostCfg, nil, nil, name)
 	if err != nil {
 		return err
 	}
@@ -554,7 +578,9 @@ func (s *Spawner) removeContainer(ctx context.Context, id string) {
 }
 
 // RemoveSession stops and removes the container spawned for a session.
-func (s *Spawner) RemoveSession(ctx context.Context, sessionID string) error {
+// Stop and remove run on background contexts on purpose: a caller hanging up
+// mid-request must not leave an orphaned container behind a deleted DB row.
+func (s *Spawner) RemoveSession(_ context.Context, sessionID string) error {
 	row, ok, err := s.store.GetContainer(sessionID)
 	if err != nil {
 		return err
@@ -568,7 +594,7 @@ func (s *Spawner) RemoveSession(ctx context.Context, sessionID string) error {
 		return fmt.Errorf("docker stop: %w", err)
 	}
 	cancel()
-	s.removeContainer(ctx, row.ContainerID)
+	s.removeContainer(context.Background(), row.ContainerID)
 	if err := s.store.DeleteContainer(sessionID); err != nil {
 		return err
 	}
@@ -577,7 +603,9 @@ func (s *Spawner) RemoveSession(ctx context.Context, sessionID string) error {
 	return nil
 }
 
-// tarDir writes a tar archive of dir (excluding .git-heavy junk) into w.
+// tarDir writes a tar archive of dir (nothing excluded, .git included) into w.
+// Symlinks are archived as symlinks with their target in Linkname so docker's
+// untar can recreate them; only regular files carry content.
 func tarDir(w io.Writer, dir string) error {
 	tw := tar.NewWriter(w)
 	defer tw.Close()
@@ -593,7 +621,14 @@ func tarDir(w io.Writer, dir string) error {
 			return nil
 		}
 		name := filepath.ToSlash(rel)
-		hdr, err := tar.FileInfoHeader(info, "")
+		link := ""
+		if info.Mode()&os.ModeSymlink != 0 {
+			link, err = os.Readlink(path)
+			if err != nil {
+				return err
+			}
+		}
+		hdr, err := tar.FileInfoHeader(info, link)
 		if err != nil {
 			return err
 		}