Streamline session+worktree creation: quick-create and inline branch input

PolyPilot.Tests/CloseSessionIconTests.cs

@@ -3,8 +3,9 @@
 namespace PolyPilot.Tests;
 
 /// <summary>
-/// Ensures the "Close Session" button uses a non-destructive icon (not trash/wastebasket).
-/// The trash icon (🗑) implies permanent deletion, but closing a session is reversible.
+/// Ensures the "Close Session" button has an appropriate icon.
+/// SessionCard uses ✕ (non-destructive inline close).
+/// SessionListItem uses 🗑 (opens a dialog with destructive options like delete worktree/branch).
 /// </summary>
 public class CloseSessionIconTests
 {
@@ -22,17 +23,19 @@ public void SessionCard_CloseButton_DoesNotUseTrashIcon()
         var file = Path.Combine(GetRepoRoot(), "PolyPilot", "Components", "SessionCard.razor");
         var content = File.ReadAllText(file);
 
-        // The close session button must not use the trash/wastebasket emoji
+        // SessionCard close is a simple inline close — no destructive options
         Assert.DoesNotContain("🗑", content.Substring(content.IndexOf("Close Session") - 5, 10));
     }
 
     [Fact]
-    public void SessionListItem_CloseButton_DoesNotUseTrashIcon()
+    public void SessionListItem_CloseButton_UsesTrashIcon()
     {
+        // SessionListItem's close opens a dialog with destructive options
+        // (delete worktree, delete branch) so the trash icon is appropriate.
         var file = Path.Combine(GetRepoRoot(), "PolyPilot", "Components", "Layout", "SessionListItem.razor");
         var content = File.ReadAllText(file);
 
-        Assert.DoesNotContain("🗑", content.Substring(content.IndexOf("Close Session") - 5, 10));
+        Assert.Contains("🗑 Close Session", content);
     }
 
     [Fact]
@@ -45,11 +48,13 @@ public void SessionCard_CloseButton_UsesCloseIcon()
     }
 
     [Fact]
-    public void SessionListItem_CloseButton_UsesCloseIcon()
+    public void SessionListItem_CloseButton_IsDestructiveStyle()
     {
+        // The close menu button should be marked as destructive since it can delete worktrees/branches
         var file = Path.Combine(GetRepoRoot(), "PolyPilot", "Components", "Layout", "SessionListItem.razor");
         var content = File.ReadAllText(file);
 
-        Assert.Contains("✕ Close Session", content);
+        // Find the menu button line (contains 🗑 Close Session)
+        Assert.Contains("class=\"menu-item destructive\" @onclick=\"ShowCloseConfirm\"", content);
     }
 }

PolyPilot.Tests/InputSelectionTests.cs

@@ -71,7 +71,6 @@ public void ValueBoundInputs_MustNotUse_OnKeyDown()
     /// </summary>
     [Theory]
     [InlineData("Layout/CreateSessionForm.razor", "ns-name", "@onkeyup")]
-    [InlineData("Layout/CreateSessionForm.razor", "wt-branch-input", "@onkeyup")]
     [InlineData("Layout/SessionListItem.razor", "rename-input", "@onkeyup")]
     [InlineData("SessionCard.razor", "card-rename-input", "@onkeyup")]
     public void SpecificInputs_UseOnKeyUp(string relativePath, string cssClass, string expectedEvent)

PolyPilot.Tests/RepoManagerTests.cs

@@ -1,3 +1,4 @@
+using PolyPilot.Models;
 using PolyPilot.Services;
 
 namespace PolyPilot.Tests;
@@ -55,4 +56,134 @@ public void NormalizeRepoUrl_NonShorthand_PassesThrough(string input)
     {
         Assert.Equal(input, RepoManager.NormalizeRepoUrl(input));
     }
+
+    #region Save Guard Tests (Review Finding #9)
+
+    private static readonly System.Reflection.BindingFlags NonPublic =
+        System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance;
+
+    private static void SetField(object obj, string name, object value)
+    {
+        var field = obj.GetType().GetField(name, NonPublic)!;
+        field.SetValue(obj, value);
+    }
+
+    private static T GetField<T>(object obj, string name)
+    {
+        var field = obj.GetType().GetField(name, NonPublic)!;
+        return (T)field.GetValue(obj)!;
+    }
+
+    private static void InvokeSave(RepoManager rm)
+    {
+        var method = typeof(RepoManager).GetMethod("Save", NonPublic)!;
+        method.Invoke(rm, null);
+    }
+
+    [Fact]
+    public void Save_AfterFailedLoad_DoesNotOverwriteWithEmptyState()
+    {
+        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
+        {
+            // Write valid state to file
+            var validJson = """{"Repositories":[{"Id":"test-1","Name":"TestRepo","Url":"https://example.com","BareClonePath":"","AddedAt":"2026-01-01T00:00:00Z"}],"Worktrees":[]}""";
+            File.WriteAllText(stateFile, validJson);
+
+            // Simulate failed load: _loaded=true, _loadedSuccessfully=false, empty state
+            SetField(rm, "_loaded", true);
+            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);
+            try
+            {
+                // Save should be blocked — empty state after failed load
+                InvokeSave(rm);
+
+                // Original file should still have our repo
+                var content = File.ReadAllText(stateFile);
+                Assert.Contains("test-1", content);
+            }
+            finally
+            {
+                stateFileField.SetValue(null, originalValue);
+            }
+        }
+        finally
+        {
+            Directory.Delete(tempDir, true);
+        }
+    }
+
+    [Fact]
+    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
+        {
+            // Simulate successful load then all repos removed
+            SetField(rm, "_loaded", true);
+            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);
+            try
+            {
+                // Save should proceed — load was successful, intentional empty state
+                InvokeSave(rm);
+
+                var content = File.ReadAllText(stateFile);
+                Assert.Contains("Repositories", content);
+                Assert.DoesNotContain("test-1", content);
+            }
+            finally
+            {
+                stateFileField.SetValue(null, originalValue);
+            }
+        }
+        finally
+        {
+            Directory.Delete(tempDir, true);
+        }
+    }
+
+    [Fact]
+    public void Repositories_ReturnsCopy_ThreadSafe()
+    {
+        var rm = new RepoManager();
+        // Inject state with some repos
+        SetField(rm, "_loaded", true);
+        SetField(rm, "_loadedSuccessfully", true);
+        var state = new RepositoryState
+        {
+            Repositories = new() { new() { Id = "r1", Name = "R1" }, new() { Id = "r2", Name = "R2" } }
+        };
+        SetField(rm, "_state", state);
+
+        // Get a snapshot
+        var repos = rm.Repositories;
+        Assert.Equal(2, repos.Count);
+
+        // Mutate the underlying state
+        state.Repositories.RemoveAll(r => r.Id == "r1");
+
+        // Snapshot should be unaffected (it's a copy)
+        Assert.Equal(2, repos.Count);
+    }
+
+    #endregion
 }