diff --git a/PolyPilot.Tests/RepoManagerTests.cs b/PolyPilot.Tests/RepoManagerTests.cs index 4eb3f6083a..0814955ca7 100644 --- a/PolyPilot.Tests/RepoManagerTests.cs +++ b/PolyPilot.Tests/RepoManagerTests.cs @@ -3,6 +3,7 @@ namespace PolyPilot.Tests; +[Collection("BaseDir")] public class RepoManagerTests { [Theory] @@ -99,10 +100,8 @@ public void Save_AfterFailedLoad_DoesNotOverwriteWithEmptyState() SetField(rm, "_loadedSuccessfully", false); SetField(rm, "_state", new RepositoryState()); - // Override StateFile to our temp path - var stateFileField = typeof(RepoManager).GetField("_stateFile", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static)!; - var originalValue = stateFileField.GetValue(null); - stateFileField.SetValue(null, stateFile); + // Redirect RepoManager to our temp dir (safe — uses the lock-protected setter) + RepoManager.SetBaseDirForTesting(tempDir); try { // Save should be blocked — empty state after failed load @@ -114,7 +113,7 @@ public void Save_AfterFailedLoad_DoesNotOverwriteWithEmptyState() } finally { - stateFileField.SetValue(null, originalValue); + RepoManager.SetBaseDirForTesting(TestSetup.TestBaseDir); } } finally @@ -129,7 +128,6 @@ public void Save_AfterSuccessfulLoad_PersistsEmptyState() var rm = new RepoManager(); var tempDir = Path.Combine(Path.GetTempPath(), $"repomgr-test-{Guid.NewGuid():N}"); Directory.CreateDirectory(tempDir); - var stateFile = Path.Combine(tempDir, "repos.json"); try { @@ -138,21 +136,21 @@ public void Save_AfterSuccessfulLoad_PersistsEmptyState() SetField(rm, "_loadedSuccessfully", true); SetField(rm, "_state", new RepositoryState()); - var stateFileField = typeof(RepoManager).GetField("_stateFile", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static)!; - var originalValue = stateFileField.GetValue(null); - stateFileField.SetValue(null, stateFile); + // Redirect RepoManager to our temp dir (safe — uses the lock-protected setter) + RepoManager.SetBaseDirForTesting(tempDir); try { // Save should proceed — load was successful, intentional empty state InvokeSave(rm); + var stateFile = Path.Combine(tempDir, "repos.json"); var content = File.ReadAllText(stateFile); Assert.Contains("Repositories", content); Assert.DoesNotContain("test-1", content); } finally { - stateFileField.SetValue(null, originalValue); + RepoManager.SetBaseDirForTesting(TestSetup.TestBaseDir); } } finally diff --git a/PolyPilot.Tests/TestIsolationGuardTests.cs b/PolyPilot.Tests/TestIsolationGuardTests.cs index 1ce64308ce..2daa29eb15 100644 --- a/PolyPilot.Tests/TestIsolationGuardTests.cs +++ b/PolyPilot.Tests/TestIsolationGuardTests.cs @@ -40,6 +40,38 @@ public void BaseDir_PointsToTempDirectory() Assert.Contains("polypilot-tests-", CopilotService.BaseDir); } + [Fact] + public void RepoManager_StateFile_IsNotRealPolypilotDir() + { + // Access the static StateFile via reflection to verify it doesn't point to real path + var stateFileField = typeof(RepoManager).GetField("_stateFile", + System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static)!; + // Force resolution by accessing Repositories (which calls EnsureLoaded -> Load -> StateFile) + var rm = new RepoManager(); + _ = rm.Repositories; + + var stateFile = (string?)stateFileField.GetValue(null); + var realReposJson = Path.Combine( + Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), + ".polypilot", "repos.json"); + + Assert.NotNull(stateFile); + Assert.NotEqual(realReposJson, stateFile); + Assert.DoesNotContain(Path.Combine(".polypilot", "repos.json"), stateFile); + } + + [Fact] + public void RepoManager_BaseDir_MatchesTestSetupDir() + { + // Verify RepoManager resolves to the same test directory as CopilotService + var baseDirOverride = typeof(RepoManager).GetField("_baseDirOverride", + System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static)!; + var overrideValue = (string?)baseDirOverride.GetValue(null); + + Assert.NotNull(overrideValue); + Assert.Equal(TestSetup.TestBaseDir, overrideValue); + } + [Fact] public void TestSetup_ModuleInitializer_HasRun() { diff --git a/PolyPilot/Services/CopilotService.Organization.cs b/PolyPilot/Services/CopilotService.Organization.cs index 0a55976f04..2a448edadf 100644 --- a/PolyPilot/Services/CopilotService.Organization.cs +++ b/PolyPilot/Services/CopilotService.Organization.cs @@ -1108,7 +1108,7 @@ private void AddOrchestratorSystemMessage(string sessionName, string message) #region Orchestration Persistence (relaunch resilience) private static string? _pendingOrchestrationFile; - private static string PendingOrchestrationFile => _pendingOrchestrationFile ??= Path.Combine(PolyPilotBaseDir, "pending-orchestration.json"); + private static string PendingOrchestrationFile { get { lock (_pathLock) return _pendingOrchestrationFile ??= Path.Combine(PolyPilotBaseDir, "pending-orchestration.json"); } } internal void SavePendingOrchestration(PendingOrchestration pending) { diff --git a/PolyPilot/Services/CopilotService.cs b/PolyPilot/Services/CopilotService.cs index 94a09a8229..2f88a0f9fd 100644 --- a/PolyPilot/Services/CopilotService.cs +++ b/PolyPilot/Services/CopilotService.cs @@ -40,11 +40,12 @@ public partial class CopilotService : IAsyncDisposable private string? _activeSessionName; private SynchronizationContext? _syncContext; + private static readonly object _pathLock = new(); private static string? _copilotBaseDir; - private static string CopilotBaseDir => LazyInitializer.EnsureInitialized(ref _copilotBaseDir, GetCopilotBaseDir); + private static string CopilotBaseDir { get { lock (_pathLock) return _copilotBaseDir ??= GetCopilotBaseDir(); } } private static string? _polyPilotBaseDir; - private static string PolyPilotBaseDir => LazyInitializer.EnsureInitialized(ref _polyPilotBaseDir, GetPolyPilotBaseDir); + private static string PolyPilotBaseDir { get { lock (_pathLock) return _polyPilotBaseDir ??= GetPolyPilotBaseDir(); } } internal static string BaseDir => PolyPilotBaseDir; private static string GetCopilotBaseDir() @@ -100,19 +101,19 @@ private static string GetPolyPilotBaseDir() } private static string? _sessionStatePath; - private static string SessionStatePath => _sessionStatePath ??= Path.Combine(CopilotBaseDir, "session-state"); + private static string SessionStatePath { get { lock (_pathLock) return _sessionStatePath ??= Path.Combine(CopilotBaseDir, "session-state"); } } private static string? _activeSessionsFile; - private static string ActiveSessionsFile => _activeSessionsFile ??= Path.Combine(PolyPilotBaseDir, "active-sessions.json"); + private static string ActiveSessionsFile { get { lock (_pathLock) return _activeSessionsFile ??= Path.Combine(PolyPilotBaseDir, "active-sessions.json"); } } private static string? _sessionAliasesFile; - private static string SessionAliasesFile => _sessionAliasesFile ??= Path.Combine(PolyPilotBaseDir, "session-aliases.json"); + private static string SessionAliasesFile { get { lock (_pathLock) return _sessionAliasesFile ??= Path.Combine(PolyPilotBaseDir, "session-aliases.json"); } } private static string? _uiStateFile; - private static string UiStateFile => _uiStateFile ??= Path.Combine(PolyPilotBaseDir, "ui-state.json"); + private static string UiStateFile { get { lock (_pathLock) return _uiStateFile ??= Path.Combine(PolyPilotBaseDir, "ui-state.json"); } } private static string? _organizationFile; - private static string OrganizationFile => _organizationFile ??= Path.Combine(PolyPilotBaseDir, "organization.json"); + private static string OrganizationFile { get { lock (_pathLock) return _organizationFile ??= Path.Combine(PolyPilotBaseDir, "organization.json"); } } /// /// Override base directory for tests to prevent writing to real ~/.polypilot/. @@ -120,18 +121,21 @@ private static string GetPolyPilotBaseDir() /// internal static void SetBaseDirForTesting(string path) { - Volatile.Write(ref _polyPilotBaseDir, path); - _activeSessionsFile = null; - _sessionAliasesFile = null; - _uiStateFile = null; - _organizationFile = null; - Volatile.Write(ref _copilotBaseDir, null); - _sessionStatePath = null; - _pendingOrchestrationFile = null; + lock (_pathLock) + { + _polyPilotBaseDir = path; + _activeSessionsFile = null; + _sessionAliasesFile = null; + _uiStateFile = null; + _organizationFile = null; + _copilotBaseDir = null; + _sessionStatePath = null; + _pendingOrchestrationFile = null; + } } private static string? _projectDir; - private static string ProjectDir => _projectDir ??= FindProjectDir(); + private static string ProjectDir { get { lock (_pathLock) return _projectDir ??= FindProjectDir(); } } private static string FindProjectDir() { diff --git a/PolyPilot/Services/RepoManager.cs b/PolyPilot/Services/RepoManager.cs index 740f5d990a..51b3193987 100644 --- a/PolyPilot/Services/RepoManager.cs +++ b/PolyPilot/Services/RepoManager.cs @@ -11,12 +11,13 @@ namespace PolyPilot.Services; public class RepoManager { private static string? _baseDirOverride; + private static readonly object _pathLock = new(); private static string? _reposDir; - private static string ReposDir => _reposDir ??= GetReposDir(); + private static string ReposDir { get { lock (_pathLock) return _reposDir ??= GetReposDir(); } } private static string? _worktreesDir; - private static string WorktreesDir => _worktreesDir ??= GetWorktreesDir(); + private static string WorktreesDir { get { lock (_pathLock) return _worktreesDir ??= GetWorktreesDir(); } } private static string? _stateFile; - private static string StateFile => _stateFile ??= GetStateFile(); + private static string StateFile { get { lock (_pathLock) return _stateFile ??= GetStateFile(); } } /// /// Redirect all RepoManager paths to a test directory. @@ -24,10 +25,13 @@ public class RepoManager /// internal static void SetBaseDirForTesting(string? path) { - Volatile.Write(ref _baseDirOverride, path); - Volatile.Write(ref _reposDir, null); - Volatile.Write(ref _worktreesDir, null); - Volatile.Write(ref _stateFile, null); + lock (_pathLock) + { + _baseDirOverride = path; + _reposDir = null; + _worktreesDir = null; + _stateFile = null; + } } private RepositoryState _state = new(); @@ -46,7 +50,8 @@ private void EnsureLoaded() private static string GetBaseDir() { - var over = Volatile.Read(ref _baseDirOverride); + // Called from within _pathLock — no Volatile.Read needed + var over = _baseDirOverride; if (over != null) return over; try { @@ -71,9 +76,10 @@ public void Load() _loadedSuccessfully = false; try { - if (File.Exists(StateFile)) + var stateFile = StateFile; // resolve once + if (File.Exists(stateFile)) { - var json = File.ReadAllText(StateFile); + var json = File.ReadAllText(stateFile); _state = JsonSerializer.Deserialize(json) ?? new RepositoryState(); } _loadedSuccessfully = true; @@ -98,9 +104,10 @@ private void Save() _loadedSuccessfully = true; try { - Directory.CreateDirectory(Path.GetDirectoryName(StateFile)!); + var stateFile = StateFile; // resolve once + Directory.CreateDirectory(Path.GetDirectoryName(stateFile)!); var json = JsonSerializer.Serialize(_state, new JsonSerializerOptions { WriteIndented = true }); - File.WriteAllText(StateFile, json); + File.WriteAllText(stateFile, json); } catch (Exception ex) {