From b85c9cf69b6e578c3d58b2ee3517737f4e0c02cc Mon Sep 17 00:00:00 2001 From: euxaristia <25621994+euxaristia@users.noreply.github.com> Date: Fri, 10 Jul 2026 10:47:59 -0400 Subject: [PATCH 1/5] fix(windows): retry atomic file renames on Access is denied / Sharing violation --- internal/cron/store.go | 31 ++++++++++++++++++++++++- internal/sandbox/windows_unelevated.go | 32 +++++++++++++++++++++++++- internal/sessions/store.go | 32 ++++++++++++++++++++++++-- internal/swarm/mailbox.go | 31 ++++++++++++++++++++++++- internal/swarm/mailbox_test.go | 1 + 5 files changed, 122 insertions(+), 5 deletions(-) diff --git a/internal/cron/store.go b/internal/cron/store.go index cee2fa0f2..2e5a78f75 100644 --- a/internal/cron/store.go +++ b/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 +} diff --git a/internal/sandbox/windows_unelevated.go b/internal/sandbox/windows_unelevated.go index 6934071de..5d387b17e 100644 --- a/internal/sandbox/windows_unelevated.go +++ b/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 +} diff --git a/internal/sessions/store.go b/internal/sessions/store.go index b0da699aa..3e44501db 100644 --- a/internal/sessions/store.go +++ b/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 +} diff --git a/internal/swarm/mailbox.go b/internal/swarm/mailbox.go index fef0caf69..d435afa45 100644 --- a/internal/swarm/mailbox.go +++ b/internal/swarm/mailbox.go @@ -7,8 +7,10 @@ import ( "os" "path/filepath" "regexp" + "runtime" "strings" "sync/atomic" + "syscall" "time" ) @@ -322,7 +324,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 := renameWithRetry(tmpName, path); err != nil { return fmt.Errorf("swarm: commit inbox: %w", err) } return nil @@ -392,3 +394,30 @@ func acquireLock(lockPath string, timeout time.Duration) (func(), error) { time.Sleep(2 * time.Millisecond) } } + +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 +} diff --git a/internal/swarm/mailbox_test.go b/internal/swarm/mailbox_test.go index 2164eb911..ebb79cd85 100644 --- a/internal/swarm/mailbox_test.go +++ b/internal/swarm/mailbox_test.go @@ -295,6 +295,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) } }() } From 631f9fe643cf68884758767d94985641f0f12af8 Mon Sep 17 00:00:00 2001 From: euxaristia <25621994+euxaristia@users.noreply.github.com> Date: Sat, 11 Jul 2026 13:00:38 -0400 Subject: [PATCH 2/5] test(swarm): add unit tests for rename retry and non-retryable errors --- internal/swarm/mailbox.go | 19 +++++++++----- internal/swarm/mailbox_test.go | 47 ++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 6 deletions(-) diff --git a/internal/swarm/mailbox.go b/internal/swarm/mailbox.go index d435afa45..6efc1a8de 100644 --- a/internal/swarm/mailbox.go +++ b/internal/swarm/mailbox.go @@ -34,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 ( @@ -216,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 @@ -258,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 } } @@ -301,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) @@ -324,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 := renameWithRetry(tmpName, path); err != nil { + if err := m.renameWithRetry(tmpName, path); err != nil { return fmt.Errorf("swarm: commit inbox: %w", err) } return nil @@ -395,10 +398,14 @@ func acquireLock(lockPath string, timeout time.Duration) (func(), error) { } } -func renameWithRetry(src, dst string) error { +func (m *Mailbox) renameWithRetry(src, dst string) error { var err error for i := 0; i < 10; i++ { - err = os.Rename(src, dst) + rename := m.rename + if rename == nil { + rename = os.Rename + } + err = rename(src, dst) if err == nil { return nil } diff --git a/internal/swarm/mailbox_test.go b/internal/swarm/mailbox_test.go index ebb79cd85..7436b7bc1 100644 --- a/internal/swarm/mailbox_test.go +++ b/internal/swarm/mailbox_test.go @@ -8,6 +8,7 @@ import ( "strings" "sync" "sync/atomic" + "syscall" "testing" "time" ) @@ -311,3 +312,49 @@ func TestMailboxConcurrentSends(t *testing.T) { t.Fatalf("concurrent sends lost messages: got %d, want %d", len(msgs), n) } } + +func TestMailboxRenameRetry(t *testing.T) { + 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) + } +} + From b48bcb7f66905aed274f4e753235b07e87ee974b Mon Sep 17 00:00:00 2001 From: euxaristia <25621994+euxaristia@users.noreply.github.com> Date: Sat, 11 Jul 2026 16:19:45 -0400 Subject: [PATCH 3/5] test(swarm): skip TestMailboxRenameRetry on non-Windows platforms --- internal/swarm/mailbox_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/swarm/mailbox_test.go b/internal/swarm/mailbox_test.go index 7436b7bc1..6925ddb03 100644 --- a/internal/swarm/mailbox_test.go +++ b/internal/swarm/mailbox_test.go @@ -314,6 +314,9 @@ func TestMailboxConcurrentSends(t *testing.T) { } func TestMailboxRenameRetry(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skip("renameWithRetry only retries on Windows") + } mb := newTestMailbox(t) var attempts int From 2af6ab3acc3976d80134d9190128c51930b6c850 Mon Sep 17 00:00:00 2001 From: euxaristia <25621994+euxaristia@users.noreply.github.com> Date: Sat, 11 Jul 2026 18:22:50 -0400 Subject: [PATCH 4/5] style(swarm): remove blank line at EOF flagged by gofmt --- internal/swarm/mailbox_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/swarm/mailbox_test.go b/internal/swarm/mailbox_test.go index 6925ddb03..2e59e4a54 100644 --- a/internal/swarm/mailbox_test.go +++ b/internal/swarm/mailbox_test.go @@ -360,4 +360,3 @@ func TestMailboxRenameNonRetryableError(t *testing.T) { t.Errorf("expected only 1 attempt for non-retryable error, got %d", attempts) } } - From e8f5b7f1796f395c389f33a2b8e80838b4d6c061 Mon Sep 17 00:00:00 2001 From: euxaristia <25621994+euxaristia@users.noreply.github.com> Date: Mon, 13 Jul 2026 23:00:16 -0400 Subject: [PATCH 5/5] fix(windows): route remaining atomic-rename sites through the retry helper Convert the 3 remaining sandbox marker/state writers and the sessions rewind writeFileAtomic method to use the existing package-level renameWithRetry helper instead of a bare os.Rename, so a transient Windows Access is denied / Sharing violation no longer rolls back the write. --- internal/sandbox/grants.go | 2 +- internal/sandbox/windows_runner.go | 2 +- internal/sandbox/windows_setup.go | 2 +- internal/sessions/rewind.go | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/sandbox/grants.go b/internal/sandbox/grants.go index ad2170c87..197454320 100644 --- a/internal/sandbox/grants.go +++ b/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 } diff --git a/internal/sandbox/windows_runner.go b/internal/sandbox/windows_runner.go index b783910ce..9eaa38f68 100644 --- a/internal/sandbox/windows_runner.go +++ b/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) } diff --git a/internal/sandbox/windows_setup.go b/internal/sandbox/windows_setup.go index 0e9b93bdd..67c00337b 100644 --- a/internal/sandbox/windows_setup.go +++ b/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) } diff --git a/internal/sessions/rewind.go b/internal/sessions/rewind.go index 6bfcf731e..90204d945 100644 --- a/internal/sessions/rewind.go +++ b/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 }