fix(workspace): replace a dangling symlink before recreating it

internal/module/workspace.go

@@ -2,7 +2,10 @@ package module
 
 import (
 	"context"
+	"errors"
 	"fmt"
+	"io/fs"
+	"os"
 	"path/filepath"
 	"strings"
 
@@ -13,8 +16,10 @@ import (
 // WorkspaceModule manages workspace symlink federation and shell config.
 //
 // Symlink safety rules:
-//   - Only create symlinks when the link path does NOT exist at all.
-//   - Never delete, overwrite, or modify existing symlinks or directories.
+//   - Create a symlink only when the link path does not exist at all, or holds a
+//     dangling symlink (its target is gone) that is removed first.
+//   - Never delete, overwrite, or modify real files, directories, or symlinks
+//     whose target still exists.
 //   - Broken symlinks are repaired only if an explicit target is configured.
 //
 // Dual-workspace architecture:
@@ -159,6 +164,9 @@ func (m *WorkspaceModule) Apply(ctx context.Context, rc *RunContext) (*ApplyResu
 			if !m.targetReachable(rc, symlink) {
 				return nil, fmt.Errorf("symlink target does not exist: %s", cfg.Symlink)
 			}
+			if err := m.removeDanglingSymlink(rc, workspacePath); err != nil {
+				return nil, fmt.Errorf("replacing dangling workspace symlink: %w", err)
+			}
 			if err := rc.Runner.Symlink(symlink, workspacePath); err != nil {
 				return nil, fmt.Errorf("symlinking workspace: %w", err)
 			}
@@ -172,6 +180,9 @@ func (m *WorkspaceModule) Apply(ctx context.Context, rc *RunContext) (*ApplyResu
 			if !m.targetReachable(rc, gdrive) {
 				return nil, fmt.Errorf("gdrive target does not exist: %s", cfg.Gdrive)
 			}
+			if err := m.removeDanglingSymlink(rc, gdriveSymlink); err != nil {
+				return nil, fmt.Errorf("replacing dangling gdrive symlink: %w", err)
+			}
 			if err := rc.Runner.Symlink(gdrive, gdriveSymlink); err != nil {
 				return nil, fmt.Errorf("symlinking gdrive: %w", err)
 			}
@@ -186,6 +197,9 @@ func (m *WorkspaceModule) Apply(ctx context.Context, rc *RunContext) (*ApplyResu
 			gdriveWork := filepath.Join(gdriveSymlink, "work")
 			gdriveXref := filepath.Join(workDir, ".gdrive")
 			if m.targetReachable(rc, gdriveWork) && !m.pathUsable(rc, gdriveXref) {
+				if err := m.removeDanglingSymlink(rc, gdriveXref); err != nil {
+					return nil, fmt.Errorf("replacing dangling work/.gdrive symlink: %w", err)
+				}
 				if err := rc.Runner.Symlink(gdriveWork, gdriveXref); err != nil {
 					return nil, fmt.Errorf("symlinking work/.gdrive: %w", err)
 				}
@@ -201,6 +215,9 @@ func (m *WorkspaceModule) Apply(ctx context.Context, rc *RunContext) (*ApplyResu
 					linkPath := filepath.Join(inboxDir, sub)
 					gdriveTarget := filepath.Join(gdriveSymlink, "work", "inbox", sub)
 					if m.targetReachable(rc, gdriveTarget) && !m.pathUsable(rc, linkPath) {
+						if err := m.removeDanglingSymlink(rc, linkPath); err != nil {
+							return nil, fmt.Errorf("replacing dangling inbox/%s symlink: %w", sub, err)
+						}
 						if err := rc.Runner.Symlink(gdriveTarget, linkPath); err != nil {
 							return nil, fmt.Errorf("symlinking inbox/%s: %w", sub, err)
 						}
@@ -249,3 +266,28 @@ func (m *WorkspaceModule) pathUsable(rc *RunContext, path string) bool {
 func (m *WorkspaceModule) targetReachable(rc *RunContext, path string) bool {
 	return rc.Runner.FileExists(path) || rc.Runner.IsDir(path)
 }
+
+// removeDanglingSymlink removes path only when it is a symlink whose target is
+// missing, so the Symlink call that follows can create the link Apply planned.
+// A real file or directory, and a symlink whose target exists, are left alone.
+// Removal goes through Runner.Remove, so a dry run removes nothing.
+//
+// Only a target confirmed missing (fs.ErrNotExist) counts as dangling. Any other
+// stat failure, such as a target behind an untraversable directory or an
+// unavailable mount, leaves the link in place and is returned as an error.
+func (m *WorkspaceModule) removeDanglingSymlink(rc *RunContext, path string) error {
+	if !rc.Runner.IsSymlink(path) {
+		return nil
+	}
+	_, statErr := os.Stat(path)
+	if statErr == nil {
+		return nil
+	}
+	if !errors.Is(statErr, fs.ErrNotExist) {
+		return fmt.Errorf("cannot tell whether the target of symlink %s exists (%v); left it in place, check the link target's permissions or mount", path, statErr)
+	}
+	if err := rc.Runner.Remove(path); err != nil && !os.IsNotExist(err) {
+		return err
+	}
+	return nil
+}

internal/module/workspace_test.go

@@ -0,0 +1,160 @@
+package module
+
+import (
+	"context"
+	"io"
+	"log/slog"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/STAIxBWLB/dotfiles/internal/config"
+	dotexec "github.com/STAIxBWLB/dotfiles/internal/exec"
+	"github.com/STAIxBWLB/dotfiles/internal/template"
+)
+
+// workspaceApplyFixture builds a workspace whose cloud root already holds
+// work/inbox/{downloads,incoming}, so Apply only has to federate symlinks.
+func workspaceApplyFixture(t *testing.T) (*RunContext, string) {
+	t.Helper()
+	home := t.TempDir()
+	drive := filepath.Join(home, "drive")
+	for _, dir := range []string{
+		filepath.Join(drive, "work", "inbox", "downloads"),
+		filepath.Join(drive, "work", "inbox", "incoming"),
+		filepath.Join(home, "workspace", "work"),
+	} {
+		if err := os.MkdirAll(dir, 0o755); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	rc := &RunContext{
+		Config: &config.Config{Modules: config.ModulesConfig{Workspace: config.WorkConfig{
+			Enabled:       true,
+			Path:          filepath.Join(home, "workspace"),
+			Gdrive:        drive,
+			GdriveSymlink: filepath.Join(home, "gdrive-workspace"),
+		}}},
+		Runner:   dotexec.NewRunner(false, slog.New(slog.NewTextHandler(io.Discard, nil))),
+		Template: template.NewEngine(),
+		HomeDir:  home,
+		Out:      io.Discard,
+	}
+	return rc, home
+}
+
+// A link whose target is gone is what a moved cloud mirror leaves behind:
+// Check plans the relink and Apply must remove the stale link before creating it.
+func TestWorkspaceApplyReplacesDanglingGdriveLink(t *testing.T) {
+	rc, home := workspaceApplyFixture(t)
+	gdriveXref := filepath.Join(home, "workspace", "work", ".gdrive")
+	if err := os.Symlink(filepath.Join(home, "old-drive", "work"), gdriveXref); err != nil {
+		t.Fatal(err)
+	}
+
+	if _, err := (&WorkspaceModule{}).Apply(context.Background(), rc); err != nil {
+		t.Fatalf("Apply: %v", err)
+	}
+
+	want := filepath.Join(home, "gdrive-workspace", "work")
+	got, err := os.Readlink(gdriveXref)
+	if err != nil {
+		t.Fatalf("work/.gdrive is not a symlink after Apply: %v", err)
+	}
+	if got != want {
+		t.Errorf("work/.gdrive -> %q, want %q", got, want)
+	}
+	if info, err := os.Stat(gdriveXref); err != nil || !info.IsDir() {
+		t.Errorf("work/.gdrive does not resolve to a directory: info=%v err=%v", info, err)
+	}
+
+	check, err := (&WorkspaceModule{}).Check(context.Background(), rc)
+	if err != nil {
+		t.Fatalf("Check after Apply: %v", err)
+	}
+	if !check.Satisfied {
+		t.Errorf("Check after Apply is not satisfied: %+v", check.Changes)
+	}
+}
+
+// A link whose target cannot be stat'ed for a reason other than "missing"
+// (here an untraversable parent) is not dangling: it stays, and Apply errors.
+func TestWorkspaceApplyKeepsSymlinkWithUnreadableTarget(t *testing.T) {
+	if os.Geteuid() == 0 {
+		t.Skip("root traverses a 0o000 directory")
+	}
+	rc, home := workspaceApplyFixture(t)
+	locked := filepath.Join(home, "locked")
+	target := filepath.Join(locked, "work")
+	if err := os.MkdirAll(target, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Chmod(locked, 0o000); err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { _ = os.Chmod(locked, 0o755) })
+	gdriveXref := filepath.Join(home, "workspace", "work", ".gdrive")
+	if err := os.Symlink(target, gdriveXref); err != nil {
+		t.Fatal(err)
+	}
+
+	_, err := (&WorkspaceModule{}).Apply(context.Background(), rc)
+	if err == nil || !strings.Contains(err.Error(), gdriveXref) || !strings.Contains(err.Error(), "permissions or mount") {
+		t.Fatalf("Apply error = %v, want one naming %s and the permissions/mount hint", err, gdriveXref)
+	}
+	got, lerr := os.Readlink(gdriveXref)
+	if lerr != nil || got != target {
+		t.Fatalf("link was changed: target=%q err=%v, want %q", got, lerr, target)
+	}
+}
+
+// A link whose target exists is the healthy state and must not be touched.
+func TestWorkspaceApplyLeavesLiveSymlinkAlone(t *testing.T) {
+	rc, home := workspaceApplyFixture(t)
+	target := filepath.Join(home, "live-target")
+	if err := os.MkdirAll(target, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	gdriveXref := filepath.Join(home, "workspace", "work", ".gdrive")
+	if err := os.Symlink(target, gdriveXref); err != nil {
+		t.Fatal(err)
+	}
+
+	if _, err := (&WorkspaceModule{}).Apply(context.Background(), rc); err != nil {
+		t.Fatalf("Apply: %v", err)
+	}
+
+	got, err := os.Readlink(gdriveXref)
+	if err != nil {
+		t.Fatalf("work/.gdrive is not a symlink after Apply: %v", err)
+	}
+	if got != target {
+		t.Errorf("work/.gdrive -> %q, want unchanged %q", got, target)
+	}
+}
+
+// A real directory at the link path is user data: Apply must never replace it.
+func TestWorkspaceApplyLeavesRealDirectoryAlone(t *testing.T) {
+	rc, home := workspaceApplyFixture(t)
+	gdriveXref := filepath.Join(home, "workspace", "work", ".gdrive")
+	if err := os.MkdirAll(gdriveXref, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	marker := filepath.Join(gdriveXref, "keep.txt")
+	if err := os.WriteFile(marker, []byte("user data\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	if _, err := (&WorkspaceModule{}).Apply(context.Background(), rc); err != nil {
+		t.Fatalf("Apply: %v", err)
+	}
+
+	if info, err := os.Lstat(gdriveXref); err != nil || !info.IsDir() {
+		t.Fatalf("work/.gdrive is no longer a real directory: info=%v err=%v", info, err)
+	}
+	if _, err := os.Stat(marker); err != nil {
+		t.Errorf("file inside the real directory was lost: %v", err)
+	}
+}