fix(windows): retry atomic file renames on Access is denied / Sharing violation

internal/cron/store.go

@@ -7,7 +7,9 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"runtime"
 	"strings"
+	"syscall"
 	"time"
 )
 
@@ -144,7 +146,7 @@ func (s *Store) writeJob(job Job) error {
 	if err := os.WriteFile(tmp, data, 0o600); err != nil {
 		return err
 	}
-	return os.Rename(tmp, filepath.Join(dir, "metadata.json"))
+	return renameWithRetry(tmp, filepath.Join(dir, "metadata.json"))
 }
 
 func (s *Store) Get(id string) (Job, error) {
@@ -322,3 +324,30 @@ func (s *Store) Runs(id string) ([]RunRecord, error) {
 	}
 	return runs, scanner.Err()
 }
+
+func renameWithRetry(src, dst string) error {
+	var err error
+	for i := 0; i < 10; i++ {
+		err = os.Rename(src, dst)
+		if err == nil {
+			return nil
+		}
+		if runtime.GOOS == "windows" {
+			if os.IsPermission(err) || isWindowsSharingViolation(err) {
+				time.Sleep(10 * time.Millisecond)
+				continue
+			}
+		}
+		break
+	}
+	return err
+}
+
+func isWindowsSharingViolation(err error) bool {
+	var errno syscall.Errno
+	if errors.As(err, &errno) {
+		const ERROR_SHARING_VIOLATION syscall.Errno = 32
+		return errno == ERROR_SHARING_VIOLATION
+	}
+	return false
+}

internal/sandbox/grants.go

@@ -600,7 +600,7 @@ func (store *GrantStore) writeState(state grantFile) error {
 	if err := os.WriteFile(tempPath, append(data, '\n'), 0o600); err != nil {
 		return err
 	}
-	if err := os.Rename(tempPath, store.filePath); err != nil {
+	if err := renameWithRetry(tempPath, store.filePath); err != nil {
 		_ = os.Remove(tempPath)
 		return err
 	}

internal/sandbox/windows_runner.go

@@ -663,7 +663,7 @@ func saveWindowsCapabilitySIDs(path string, caps WindowsCapabilitySIDs) error {
 		_ = os.Remove(tmpPath)
 		return fmt.Errorf("close windows capability SID temp file: %w", err)
 	}
-	if err := os.Rename(tmpPath, path); err != nil {
+	if err := renameWithRetry(tmpPath, path); err != nil {
 		_ = os.Remove(tmpPath)
 		return fmt.Errorf("replace windows capability SID file: %w", err)
 	}

internal/sandbox/windows_setup.go

@@ -239,7 +239,7 @@ func WriteWindowsSandboxSetupMarker(config WindowsSandboxSetupConfig) (WindowsSa
 		_ = os.Remove(tmpPath)
 		return WindowsSandboxSetupMarker{}, fmt.Errorf("close windows sandbox setup marker temp file: %w", err)
 	}
-	if err := os.Rename(tmpPath, path); err != nil {
+	if err := renameWithRetry(tmpPath, path); err != nil {
 		_ = os.Remove(tmpPath)
 		return WindowsSandboxSetupMarker{}, fmt.Errorf("replace windows sandbox setup marker: %w", err)
 	}

internal/sandbox/windows_unelevated.go

@@ -6,7 +6,10 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"runtime"
 	"strings"
+	"syscall"
+	"time"
 )
 
 const windowsUnelevatedSetupMarkerSchemaVersion = 1
@@ -138,9 +141,36 @@ func recordWindowsUnelevatedAppliedPlan(sandboxHome string, applied WindowsUnele
 		_ = os.Remove(tmpPath)
 		return fmt.Errorf("close windows unelevated setup marker temp file: %w", err)
 	}
-	if err := os.Rename(tmpPath, path); err != nil {
+	if err := renameWithRetry(tmpPath, path); err != nil {
 		_ = os.Remove(tmpPath)
 		return fmt.Errorf("replace windows unelevated setup marker: %w", err)
 	}
 	return nil
 }
+
+func renameWithRetry(src, dst string) error {
+	var err error
+	for i := 0; i < 10; i++ {
+		err = os.Rename(src, dst)
+		if err == nil {
+			return nil
+		}
+		if runtime.GOOS == "windows" {
+			if os.IsPermission(err) || isWindowsSharingViolation(err) {
+				time.Sleep(10 * time.Millisecond)
+				continue
+			}
+		}
+		break
+	}
+	return err
+}
+
+func isWindowsSharingViolation(err error) bool {
+	var errno syscall.Errno
+	if errors.As(err, &errno) {
+		const ERROR_SHARING_VIOLATION syscall.Errno = 32
+		return errno == ERROR_SHARING_VIOLATION
+	}
+	return false
+}

internal/sessions/rewind.go

@@ -290,7 +290,7 @@ func (store *Store) writeFileAtomic(path string, content []byte, mode uint32) er
 		_ = os.Remove(tmp)
 		return err
 	}
-	if err := os.Rename(tmp, path); err != nil {
+	if err := renameWithRetry(tmp, path); err != nil {
 		_ = os.Remove(tmp)
 		return err
 	}

internal/sessions/store.go

@@ -13,6 +13,7 @@ import (
 	"strings"
 	"sync"
 	"sync/atomic"
+	"syscall"
 	"time"
 )
 
@@ -841,7 +842,7 @@ func (store *Store) writeMetadata(session Metadata) error {
 	if err := writeFileSync(tmp, append(data, '\n'), 0o600); err != nil {
 		return fmt.Errorf("write zero session metadata: %w", err)
 	}
-	if err := os.Rename(tmp, path); err != nil {
+	if err := renameWithRetry(tmp, path); err != nil {
 		_ = os.Remove(tmp)
 		return fmt.Errorf("replace zero session metadata: %w", err)
 	}
@@ -904,7 +905,7 @@ func (store *Store) writeFileAtomicSync(path string, content []byte, perm os.Fil
 	if err := writeFileSync(tmp, content, perm); err != nil {
 		return err
 	}
-	if err := os.Rename(tmp, path); err != nil {
+	if err := renameWithRetry(tmp, path); err != nil {
 		_ = os.Remove(tmp)
 		return err
 	}
@@ -1091,3 +1092,30 @@ func applySpecRecord(session *Metadata, input RecordSpecInput, status SpecStatus
 		session.SpecImplSessionID = implID
 	}
 }
+
+func renameWithRetry(src, dst string) error {
+	var err error
+	for i := 0; i < 10; i++ {
+		err = os.Rename(src, dst)
+		if err == nil {
+			return nil
+		}
+		if runtime.GOOS == "windows" {
+			if os.IsPermission(err) || isWindowsSharingViolation(err) {
+				time.Sleep(10 * time.Millisecond)
+				continue
+			}
+		}
+		break
+	}
+	return err
+}
+
+func isWindowsSharingViolation(err error) bool {
+	var errno syscall.Errno
+	if errors.As(err, &errno) {
+		const ERROR_SHARING_VIOLATION syscall.Errno = 32
+		return errno == ERROR_SHARING_VIOLATION
+	}
+	return false
+}

internal/swarm/mailbox.go

@@ -7,8 +7,10 @@ import (
 	"os"
 	"path/filepath"
 	"regexp"
+	"runtime"
 	"strings"
 	"sync/atomic"
+	"syscall"
 	"time"
 )
 
@@ -32,6 +34,9 @@ type Mailbox struct {
 	MaxMessages int
 	// LockTimeout bounds how long Send/MarkRead wait for the inbox lock.
 	LockTimeout time.Duration
+
+	// rename is used to override the rename operation in tests.
+	rename func(src, dst string) error
 }
 
 const (
@@ -214,7 +219,7 @@ func (m *Mailbox) Send(team, recipient string, msg Message) error {
 		return fmt.Errorf("%w: %d messages", ErrMailboxFull, len(messages))
 	}
 	messages = append(messages, msg)
-	return atomicWriteJSON(path, messages)
+	return m.atomicWriteJSON(path, messages)
 }
 
 // ReadAndConsume reads the recipient's inbox and marks every previously-unread
@@ -256,7 +261,7 @@ func (m *Mailbox) ReadAndConsume(team, recipient string) ([]Message, error) {
 		}
 	}
 	if changed {
-		if err := atomicWriteJSON(path, messages); err != nil {
+		if err := m.atomicWriteJSON(path, messages); err != nil {
 			return nil, err
 		}
 	}
@@ -299,7 +304,7 @@ func (m *Mailbox) readLocked(path string) ([]Message, error) {
 
 // atomicWriteJSON writes data as pretty JSON to a sibling temp file (0600) then
 // renames it over path, so a reader never observes a partial write.
-func atomicWriteJSON(path string, data any) error {
+func (m *Mailbox) atomicWriteJSON(path string, data any) error {
 	encoded, err := json.MarshalIndent(data, "", "  ")
 	if err != nil {
 		return fmt.Errorf("swarm: encode inbox: %w", err)
@@ -322,7 +327,7 @@ func atomicWriteJSON(path string, data any) error {
 	if err := tmp.Close(); err != nil {
 		return fmt.Errorf("swarm: close temp inbox: %w", err)
 	}
-	if err := os.Rename(tmpName, path); err != nil {
+	if err := m.renameWithRetry(tmpName, path); err != nil {
 		return fmt.Errorf("swarm: commit inbox: %w", err)
 	}
 	return nil
@@ -392,3 +397,34 @@ func acquireLock(lockPath string, timeout time.Duration) (func(), error) {
 		time.Sleep(2 * time.Millisecond)
 	}
 }
+
+func (m *Mailbox) renameWithRetry(src, dst string) error {
+	var err error
+	for i := 0; i < 10; i++ {
+		rename := m.rename
+		if rename == nil {
+			rename = os.Rename
+		}
+		err = rename(src, dst)
+		if err == nil {
+			return nil
+		}
+		if runtime.GOOS == "windows" {
+			if os.IsPermission(err) || isWindowsSharingViolation(err) {
+				time.Sleep(10 * time.Millisecond)
+				continue
+			}
+		}
+		break
+	}
+	return err
+}
+
+func isWindowsSharingViolation(err error) bool {
+	var errno syscall.Errno
+	if errors.As(err, &errno) {
+		const ERROR_SHARING_VIOLATION syscall.Errno = 32
+		return errno == ERROR_SHARING_VIOLATION
+	}
+	return false
+}

internal/swarm/mailbox_test.go

@@ -8,6 +8,7 @@ import (
 	"strings"
 	"sync"
 	"sync/atomic"
+	"syscall"
 	"testing"
 	"time"
 )
@@ -295,6 +296,7 @@ func TestMailboxConcurrentSends(t *testing.T) {
 			defer wg.Done()
 			if err := mb.Send("team", "bob", Message{From: "a", Body: "concurrent"}); err != nil {
 				failures.Add(1)
+				t.Logf("Send error: %v", err)
 			}
 		}()
 	}
@@ -310,3 +312,51 @@ func TestMailboxConcurrentSends(t *testing.T) {
 		t.Fatalf("concurrent sends lost messages: got %d, want %d", len(msgs), n)
 	}
 }
+
+func TestMailboxRenameRetry(t *testing.T) {
+	if runtime.GOOS != "windows" {
+		t.Skip("renameWithRetry only retries on Windows")
+	}
+	mb := newTestMailbox(t)
+
+	var attempts int
+	mb.rename = func(src, dst string) error {
+		attempts++
+		if attempts < 3 {
+			// Return a retryable Windows sharing violation error
+			return syscall.Errno(32)
+		}
+		// Then succeed
+		return os.Rename(src, dst)
+	}
+
+	err := mb.Send("team", "alice", Message{Body: "retry-test"})
+	if err != nil {
+		t.Fatalf("expected Send to succeed after retries, got: %v", err)
+	}
+	if attempts < 3 {
+		t.Errorf("expected at least 3 attempts, got %d", attempts)
+	}
+}
+
+func TestMailboxRenameNonRetryableError(t *testing.T) {
+	mb := newTestMailbox(t)
+
+	var attempts int
+	expectedErr := errors.New("non-retryable error")
+	mb.rename = func(src, dst string) error {
+		attempts++
+		return expectedErr
+	}
+
+	err := mb.Send("team", "alice", Message{Body: "retry-test"})
+	if err == nil {
+		t.Fatal("expected Send to fail immediately")
+	}
+	if !errors.Is(err, expectedErr) && !strings.Contains(err.Error(), expectedErr.Error()) {
+		t.Errorf("expected error %v, got %v", expectedErr, err)
+	}
+	if attempts != 1 {
+		t.Errorf("expected only 1 attempt for non-retryable error, got %d", attempts)
+	}
+}