fix(devbox): reconcile runx bin dir in place to avoid concurrent races

internal/devbox/devbox.go

@@ -1197,13 +1197,10 @@ func (d *Devbox) Lockfile() *lock.File {
 
 func (d *Devbox) RunXPaths(ctx context.Context) (string, error) {
 	runxBinPath := filepath.Join(d.projectDir, ".devbox", "virtenv", "runx", "bin")
-	if err := os.RemoveAll(runxBinPath); err != nil {
-		return "", err
-	}
-	if err := os.MkdirAll(runxBinPath, 0o755); err != nil {
-		return "", err
-	}
 
+	// Map of symlink name -> target for every runx binary. If two packages
+	// provide a binary with the same name, the first one wins.
+	links := map[string]string{}
 	for _, pkg := range d.InstallablePackages() {
 		if !pkg.IsRunX() {
 			continue
@@ -1223,17 +1220,63 @@ func (d *Devbox) RunXPaths(ctx context.Context) (string, error) {
 				return "", err
 			}
 			for _, file := range files {
-				src := filepath.Join(path, file.Name())
-				dst := filepath.Join(runxBinPath, file.Name())
-				if err := os.Symlink(src, dst); err != nil && !errors.Is(err, os.ErrExist) {
-					return "", err
+				if _, ok := links[file.Name()]; !ok {
+					links[file.Name()] = filepath.Join(path, file.Name())
 				}
 			}
 		}
 	}
+	if err := syncSymlinks(runxBinPath, links); err != nil {
+		return "", err
+	}
 	return runxBinPath, nil
 }
 
+// syncSymlinks makes dir contain exactly the symlinks in links (name ->
+// target). It reconciles the directory in place instead of recreating it so
+// that concurrent devbox processes (and running shells that have dir in their
+// PATH) never observe it missing or half-populated.
+func syncSymlinks(dir string, links map[string]string) error {
+	if err := os.MkdirAll(dir, 0o755); err != nil {
+		return err
+	}
+
+	entries, err := os.ReadDir(dir)
+	if err != nil {
+		return err
+	}
+	for _, entry := range entries {
+		name := entry.Name()
+		want, ok := links[name]
+		if ok && isSymlinkTo(filepath.Join(dir, name), want) {
+			continue
+		}
+		err := os.RemoveAll(filepath.Join(dir, name))
+		if err != nil && !errors.Is(err, os.ErrNotExist) {
+			return err
+		}
+	}
+
+	for name, target := range links {
+		dst := filepath.Join(dir, name)
+		err := os.Symlink(target, dst)
+		if err == nil {
+			continue
+		}
+		// Another process may have created the link first.
+		if errors.Is(err, os.ErrExist) && isSymlinkTo(dst, target) {
+			continue
+		}
+		return err
+	}
+	return nil
+}
+
+func isSymlinkTo(path, target string) bool {
+	got, err := os.Readlink(path)
+	return err == nil && got == target
+}
+
 func validateEnvironment(environment string) (string, error) {
 	if environment == "" {
 		return "dev", nil

internal/devbox/devbox_test.go

@@ -132,3 +132,58 @@ func devboxForTesting(t *testing.T) *Devbox {
 
 	return d
 }
+
+func TestSyncSymlinks(t *testing.T) {
+	dir := filepath.Join(t.TempDir(), "runx", "bin")
+	require.NoError(t, os.MkdirAll(dir, 0o755))
+
+	targets := t.TempDir()
+	target := func(name string) string { return filepath.Join(targets, name) }
+
+	// Pre-existing state: one correct link, one stale link pointing at the
+	// wrong target, one link that's no longer wanted, and a regular file.
+	require.NoError(t, os.Symlink(target("correct"), filepath.Join(dir, "correct")))
+	require.NoError(t, os.Symlink(target("old"), filepath.Join(dir, "stale")))
+	require.NoError(t, os.Symlink(target("removed"), filepath.Join(dir, "removed")))
+	require.NoError(t, os.WriteFile(filepath.Join(dir, "file"), nil, 0o644))
+
+	links := map[string]string{
+		"correct": target("correct"),
+		"stale":   target("stale"),
+		"new":     target("new"),
+		"file":    target("file"),
+	}
+	require.NoError(t, syncSymlinks(dir, links))
+
+	entries, err := os.ReadDir(dir)
+	require.NoError(t, err)
+	got := map[string]string{}
+	for _, e := range entries {
+		dst, err := os.Readlink(filepath.Join(dir, e.Name()))
+		require.NoError(t, err, "%s should be a symlink", e.Name())
+		got[e.Name()] = dst
+	}
+	assert.Equal(t, links, got)
+}
+
+func TestSyncSymlinksConcurrent(t *testing.T) {
+	dir := filepath.Join(t.TempDir(), "runx", "bin")
+	targets := t.TempDir()
+	links := map[string]string{}
+	for i := range 50 {
+		name := fmt.Sprintf("bin%d", i)
+		links[name] = filepath.Join(targets, name)
+	}
+
+	errs := make(chan error, 8)
+	for range cap(errs) {
+		go func() { errs <- syncSymlinks(dir, links) }()
+	}
+	for range cap(errs) {
+		assert.NoError(t, <-errs)
+	}
+
+	entries, err := os.ReadDir(dir)
+	require.NoError(t, err)
+	assert.Len(t, entries, len(links))
+}