fix: address review findings round 3 (PR #806)

PolyPilot.IntegrationTests/AdvancedCliConfigTests.cs

@@ -0,0 +1,59 @@
+using PolyPilot.IntegrationTests.Fixtures;
+
+namespace PolyPilot.IntegrationTests;
+
+/// <summary>
+/// Integration tests for the Advanced CLI config settings in the Settings page.
+/// Verifies that the Advanced section renders with the expected toggles
+/// (CompactPaste, RespectGitignore, DisableAllHooks) end-to-end through
+/// the live Blazor UI via DevFlow CDP.
+/// Related to issue #698 (Expose additional CLI config options in Settings UI).
+/// </summary>
+[Collection("PolyPilot")]
+[Trait("Category", "AdvancedCliConfig")]
+public class AdvancedCliConfigTests : IntegrationTestBase
+{
+    public AdvancedCliConfigTests(AppFixture app, ITestOutputHelper output)
+        : base(app, output) { }
+
+    [Fact]
+    public async Task SettingsPage_ShowsAdvancedSection()
+    {
+        await WaitForCdpReadyAsync();
+
+        // Navigate to settings
+        await ClickAsync("[href='/settings'], .settings-link, a[title='Settings']");
+        await WaitForAsync("#settings-page, .settings-container", TimeSpan.FromSeconds(10));
+
+        // The Advanced section should be present in the page
+        var hasAdvanced = await ExistsAsync("#settings-advanced");
+        Output.WriteLine($"Advanced section visible: {hasAdvanced}");
+        Assert.True(hasAdvanced, "Advanced section (#settings-advanced) should be visible on the Settings page");
+
+        await ScreenshotAsync("settings-advanced-section");
+    }
+
+    [Fact]
+    public async Task AdvancedSection_HasCliConfigToggles()
+    {
+        await WaitForCdpReadyAsync();
+
+        // Navigate to settings
+        await ClickAsync("[href='/settings'], .settings-link, a[title='Settings']");
+        await WaitForAsync("#settings-page, .settings-container", TimeSpan.FromSeconds(10));
+
+        // Scroll to and check for the Advanced navigation item
+        var navVisible = await ExistsAsync(".settings-nav-item");
+        Output.WriteLine($"Nav items visible: {navVisible}");
+        Assert.True(navVisible, "Settings nav items should be visible");
+
+        // Check the page text contains our setting labels
+        var pageText = await GetTextAsync("#settings-advanced") ?? "";
+        Output.WriteLine($"Advanced section text length: {pageText.Length}");
+        Assert.Contains("Compact Paste", pageText);
+        Assert.Contains("Respect .gitignore", pageText);
+        Assert.Contains("Disable All Hooks", pageText);
+
+        await ScreenshotAsync("settings-advanced-toggles");
+    }
+}

PolyPilot.Tests/ConnectionSettingsTests.cs

@@ -686,4 +686,231 @@ private void Dispose()
     {
         try { Directory.Delete(_testDir, true); } catch { }
     }
+
+    // ── Advanced CLI config tests ───────────────────────────────────
+
+    [Fact]
+    public void DefaultValues_AdvancedCliConfig_AreFalse()
+    {
+        var settings = new ConnectionSettings();
+        Assert.False(settings.CompactPaste);
+        Assert.False(settings.RespectGitignore);
+        Assert.False(settings.DisableAllHooks);
+    }
+
+    [Fact]
+    public void RoundTrip_AdvancedCliConfig()
+    {
+        var original = new ConnectionSettings
+        {
+            CompactPaste = true,
+            RespectGitignore = true,
+            DisableAllHooks = true
+        };
+
+        var json = JsonSerializer.Serialize(original);
+        var loaded = JsonSerializer.Deserialize<ConnectionSettings>(json);
+
+        Assert.NotNull(loaded);
+        Assert.True(loaded!.CompactPaste);
+        Assert.True(loaded.RespectGitignore);
+        Assert.True(loaded.DisableAllHooks);
+    }
+
+    [Fact]
+    public void BackwardCompatibility_OldJson_AdvancedCliConfigDefaultsFalse()
+    {
+        var json = """{"Mode":0,"Host":"localhost","Port":4321}""";
+        var loaded = JsonSerializer.Deserialize<ConnectionSettings>(json);
+
+        Assert.NotNull(loaded);
+        Assert.False(loaded!.CompactPaste);
+        Assert.False(loaded.RespectGitignore);
+        Assert.False(loaded.DisableAllHooks);
+    }
+
+    [Fact]
+    public void SyncCliConfig_WritesConfigFile()
+    {
+        var tempDir = Path.Combine(Path.GetTempPath(), $"copilot-test-{Guid.NewGuid():N}");
+        try
+        {
+            var settings = new ConnectionSettings
+            {
+                CompactPaste = true,
+                RespectGitignore = false,
+                DisableAllHooks = true
+            };
+            settings.SyncCliConfig(tempDir);
+
+            var configPath = Path.Combine(tempDir, "config.json");
+            Assert.True(File.Exists(configPath), "config.json should be created");
+
+            using var doc = JsonDocument.Parse(File.ReadAllText(configPath));
+            Assert.True(doc.RootElement.GetProperty("compactPaste").GetBoolean());
+            Assert.False(doc.RootElement.GetProperty("respectGitignore").GetBoolean());
+            Assert.True(doc.RootElement.GetProperty("disableAllHooks").GetBoolean());
+        }
+        finally
+        {
+            try { Directory.Delete(tempDir, true); } catch { }
+        }
+    }
+
+    [Fact]
+    public void SyncCliConfig_PreservesExistingKeys()
+    {
+        var tempDir = Path.Combine(Path.GetTempPath(), $"copilot-test-{Guid.NewGuid():N}");
+        try
+        {
+            // Pre-populate config with an existing key
+            Directory.CreateDirectory(tempDir);
+            File.WriteAllText(
+                Path.Combine(tempDir, "config.json"),
+                """{"existingKey": "existingValue", "compactPaste": false}""");
+
+            var settings = new ConnectionSettings { CompactPaste = true };
+            settings.SyncCliConfig(tempDir);
+
+            var configPath = Path.Combine(tempDir, "config.json");
+            using var doc = JsonDocument.Parse(File.ReadAllText(configPath));
+            Assert.Equal("existingValue", doc.RootElement.GetProperty("existingKey").GetString());
+            Assert.True(doc.RootElement.GetProperty("compactPaste").GetBoolean());
+        }
+        finally
+        {
+            try { Directory.Delete(tempDir, true); } catch { }
+        }
+    }
+
+    [Fact]
+    public void SyncCliConfig_CorruptFile_AbortsWithoutOverwriting()
+    {
+        var tempDir = Path.Combine(Path.GetTempPath(), $"copilot-test-{Guid.NewGuid():N}");
+        try
+        {
+            Directory.CreateDirectory(tempDir);
+            var configPath = Path.Combine(tempDir, "config.json");
+            var corruptContent = "{ not valid json !!!";
+            File.WriteAllText(configPath, corruptContent);
+
+            var settings = new ConnectionSettings { CompactPaste = true };
+            settings.SyncCliConfig(tempDir);
+
+            // The corrupt file should NOT be overwritten
+            var afterContent = File.ReadAllText(configPath);
+            Assert.Equal(corruptContent, afterContent);
+        }
+        finally
+        {
+            try { Directory.Delete(tempDir, true); } catch { }
+        }
+    }
+
+    [Fact]
+    public void SyncCliConfig_AtomicWrite_UsesTempFile()
+    {
+        var tempDir = Path.Combine(Path.GetTempPath(), $"copilot-test-{Guid.NewGuid():N}");
+        try
+        {
+            var settings = new ConnectionSettings { CompactPaste = true };
+            settings.SyncCliConfig(tempDir);
+
+            var configPath = Path.Combine(tempDir, "config.json");
+            Assert.True(File.Exists(configPath), "config.json should exist after sync");
+
+            // Verify the temp file was cleaned up (rename happened)
+            Assert.False(File.Exists(configPath + ".tmp"), "Temp file should not remain after atomic write");
+        }
+        finally
+        {
+            try { Directory.Delete(tempDir, true); } catch { }
+        }
+    }
+
+    [Fact]
+    public void ImportCliConfigValues_ImportsFromConfigJson()
+    {
+        var tempDir = Path.Combine(Path.GetTempPath(), $"copilot-test-{Guid.NewGuid():N}");
+        try
+        {
+            Directory.CreateDirectory(tempDir);
+            File.WriteAllText(
+                Path.Combine(tempDir, "config.json"),
+                """{"compactPaste": true, "respectGitignore": true, "disableAllHooks": false}""");
+
+            var settings = new ConnectionSettings();
+            Assert.False(settings.CompactPaste);
+            Assert.False(settings.RespectGitignore);
+
+            bool changed = settings.ImportCliConfigValues(tempDir);
+
+            Assert.True(changed);
+            Assert.True(settings.CompactPaste);
+            Assert.True(settings.RespectGitignore);
+            Assert.False(settings.DisableAllHooks);
+        }
+        finally
+        {
+            try { Directory.Delete(tempDir, true); } catch { }
+        }
+    }
+
+    [Fact]
+    public void ImportCliConfigValues_NoFile_ReturnsFalse()
+    {
+        var tempDir = Path.Combine(Path.GetTempPath(), $"copilot-test-{Guid.NewGuid():N}");
+        try
+        {
+            Directory.CreateDirectory(tempDir);
+            var settings = new ConnectionSettings();
+            bool changed = settings.ImportCliConfigValues(tempDir);
+            Assert.False(changed);
+        }
+        finally
+        {
+            try { Directory.Delete(tempDir, true); } catch { }
+        }
+    }
+
+    [Fact]
+    public void ImportCliConfigValues_CorruptFile_ReturnsFalse()
+    {
+        var tempDir = Path.Combine(Path.GetTempPath(), $"copilot-test-{Guid.NewGuid():N}");
+        try
+        {
+            Directory.CreateDirectory(tempDir);
+            File.WriteAllText(Path.Combine(tempDir, "config.json"), "not json!");
+
+            var settings = new ConnectionSettings();
+            bool changed = settings.ImportCliConfigValues(tempDir);
+            Assert.False(changed);
+            Assert.False(settings.CompactPaste);
+        }
+        finally
+        {
+            try { Directory.Delete(tempDir, true); } catch { }
+        }
+    }
+
+    [Fact]
+    public void ImportCliConfigValues_NoChange_ReturnsFalse()
+    {
+        var tempDir = Path.Combine(Path.GetTempPath(), $"copilot-test-{Guid.NewGuid():N}");
+        try
+        {
+            Directory.CreateDirectory(tempDir);
+            File.WriteAllText(
+                Path.Combine(tempDir, "config.json"),
+                """{"compactPaste": false, "respectGitignore": false, "disableAllHooks": false}""");
+
+            var settings = new ConnectionSettings();
+            bool changed = settings.ImportCliConfigValues(tempDir);
+            Assert.False(changed); // defaults match file
+        }
+        finally
+        {
+            try { Directory.Delete(tempDir, true); } catch { }
+        }
+    }
 }