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

PolyPilot.IntegrationTests/AdvancedCliConfigTests.cs

@@ -0,0 +1,58 @@
+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.True(pageText.Length > 0, "Advanced section should contain text content");
+        Assert.Contains("Compact Paste", pageText, StringComparison.OrdinalIgnoreCase);
+
+        await ScreenshotAsync("settings-advanced-toggles");
+    }
+}

PolyPilot.Tests/ConnectionSettingsTests.cs

@@ -686,4 +686,216 @@ 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_AbortsOnCorruptFile()
+    {
+        var tempDir = Path.Combine(Path.GetTempPath(), $"copilot-test-{Guid.NewGuid():N}");
+        try
+        {
+            Directory.CreateDirectory(tempDir);
+            var configPath = Path.Combine(tempDir, "config.json");
+            // Write corrupt JSON with an existing key that should be preserved
+            File.WriteAllText(configPath, "NOT VALID JSON {{{");
+
+            var settings = new ConnectionSettings { CompactPaste = true };
+            settings.SyncCliConfig(tempDir);
+
+            // File should remain unchanged — abort on parse failure
+            var content = File.ReadAllText(configPath);
+            Assert.Equal("NOT VALID JSON {{{", content);
+        }
+        finally
+        {
+            try { Directory.Delete(tempDir, true); } catch { }
+        }
+    }
+
+    [Fact]
+    public void SyncCliConfig_AtomicWrite_UsesRename()
+    {
+        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 be created");
+            // Temp file should be cleaned up
+            Assert.False(File.Exists(configPath + ".tmp"), "Temp file should not remain");
+
+            using var doc = JsonDocument.Parse(File.ReadAllText(configPath));
+            Assert.True(doc.RootElement.GetProperty("compactPaste").GetBoolean());
+        }
+        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, "someOtherKey": 42}""");
+
+            var settings = new ConnectionSettings();
+            Assert.False(settings.CompactPaste);
+            Assert.False(settings.RespectGitignore);
+
+            settings.ImportCliConfigValues(tempDir);
+
+            Assert.True(settings.CompactPaste);
+            Assert.True(settings.RespectGitignore);
+            Assert.False(settings.DisableAllHooks); // not in config.json
+        }
+        finally
+        {
+            try { Directory.Delete(tempDir, true); } catch { }
+        }
+    }
+
+    [Fact]
+    public void ImportCliConfigValues_IgnoresCorruptFile()
+    {
+        var tempDir = Path.Combine(Path.GetTempPath(), $"copilot-test-{Guid.NewGuid():N}");
+        try
+        {
+            Directory.CreateDirectory(tempDir);
+            File.WriteAllText(Path.Combine(tempDir, "config.json"), "NOT VALID JSON");
+
+            var settings = new ConnectionSettings();
+            settings.ImportCliConfigValues(tempDir);
+
+            // Should remain at defaults — no crash
+            Assert.False(settings.CompactPaste);
+            Assert.False(settings.RespectGitignore);
+            Assert.False(settings.DisableAllHooks);
+        }
+        finally
+        {
+            try { Directory.Delete(tempDir, true); } catch { }
+        }
+    }
+
+    [Fact]
+    public void ImportCliConfigValues_NoFileNoCrash()
+    {
+        var tempDir = Path.Combine(Path.GetTempPath(), $"copilot-test-{Guid.NewGuid():N}");
+        try
+        {
+            Directory.CreateDirectory(tempDir);
+            var settings = new ConnectionSettings();
+            settings.ImportCliConfigValues(tempDir);
+
+            // Should remain at defaults — no crash
+            Assert.False(settings.CompactPaste);
+        }
+        finally
+        {
+            try { Directory.Delete(tempDir, true); } catch { }
+        }
+    }
 }

PolyPilot.Tests/SettingsRegistryTests.cs

@@ -358,4 +358,112 @@ public void Editor_VisibleOnDesktopOnly()
         ctx.IsDesktop = false;
         Assert.False(desc.IsVisible!(ctx));
     }
+
+    // ── Advanced CLI config tests ───────────────────────────────────
+
+    [Fact]
+    public void Categories_ContainsAdvanced()
+    {
+        Assert.Contains("Advanced", SettingsRegistry.Categories);
+    }
+
+    [Fact]
+    public void Advanced_CompactPaste_DefaultFalse()
+    {
+        var ctx = CreateContext();
+        var desc = SettingsRegistry.All.First(s => s.Id == "advanced.compactPaste");
+        Assert.False((bool)desc.GetValue!(ctx)!);
+    }
+
+    [Fact]
+    public void Advanced_CompactPaste_ToggleValue()
+    {
+        var settings = new ConnectionSettings { CompactPaste = false };
+        var ctx = CreateContext(settings);
+        var desc = SettingsRegistry.All.First(s => s.Id == "advanced.compactPaste");
+        desc.SetValue!(ctx, true);
+        Assert.True(settings.CompactPaste);
+    }
+
+    [Fact]
+    public void Advanced_RespectGitignore_ToggleValue()
+    {
+        var settings = new ConnectionSettings();
+        var ctx = CreateContext(settings);
+        var desc = SettingsRegistry.All.First(s => s.Id == "advanced.respectGitignore");
+        Assert.False((bool)desc.GetValue!(ctx)!);
+        desc.SetValue!(ctx, true);
+        Assert.True(settings.RespectGitignore);
+    }
+
+    [Fact]
+    public void Advanced_DisableAllHooks_ToggleValue()
+    {
+        var settings = new ConnectionSettings();
+        var ctx = CreateContext(settings);
+        var desc = SettingsRegistry.All.First(s => s.Id == "advanced.disableAllHooks");
+        Assert.False((bool)desc.GetValue!(ctx)!);
+        desc.SetValue!(ctx, true);
+        Assert.True(settings.DisableAllHooks);
+    }
+
+    [Fact]
+    public void Advanced_HiddenInRemoteMode()
+    {
+        var settings = new ConnectionSettings { Mode = ConnectionMode.Remote };
+        var ctx = CreateContext(settings);
+        var desc = SettingsRegistry.All.First(s => s.Id == "advanced.compactPaste");
+        Assert.False(desc.IsVisible!(ctx));
+    }
+
+    [Fact]
+    public void Advanced_HiddenInDemoMode()
+    {
+        var settings = new ConnectionSettings { Mode = ConnectionMode.Demo };
+        var ctx = CreateContext(settings);
+        var desc = SettingsRegistry.All.First(s => s.Id == "advanced.respectGitignore");
+        Assert.False(desc.IsVisible!(ctx));
+    }
+
+    [Fact]
+    public void Advanced_VisibleInPersistentMode()
+    {
+        var settings = new ConnectionSettings { Mode = ConnectionMode.Persistent };
+        var ctx = CreateContext(settings);
+        var desc = SettingsRegistry.All.First(s => s.Id == "advanced.disableAllHooks");
+        Assert.True(desc.IsVisible!(ctx));
+    }
+
+    [Fact]
+    public void Advanced_VisibleInEmbeddedMode()
+    {
+        var settings = new ConnectionSettings { Mode = ConnectionMode.Embedded };
+        var ctx = CreateContext(settings);
+        var compactPaste = SettingsRegistry.All.First(s => s.Id == "advanced.compactPaste");
+        Assert.True(compactPaste.IsVisible!(ctx));
+    }
+
+    [Fact]
+    public void Search_FindsAdvancedByKeyword()
+    {
+        var ctx = CreateContext();
+        var results = SettingsRegistry.Search("compact paste", ctx).ToList();
+        Assert.Contains(results, s => s.Id == "advanced.compactPaste");
+    }
+
+    [Fact]
+    public void Search_FindsAdvancedByGitignore()
+    {
+        var ctx = CreateContext();
+        var results = SettingsRegistry.Search("gitignore", ctx).ToList();
+        Assert.Contains(results, s => s.Id == "advanced.respectGitignore");
+    }
+
+    [Fact]
+    public void Search_FindsAdvancedByHooks()
+    {
+        var ctx = CreateContext();
+        var results = SettingsRegistry.Search("hooks", ctx).ToList();
+        Assert.Contains(results, s => s.Id == "advanced.disableAllHooks");
+    }
 }