diff --git a/PolyPilot.Tests/CliPathResolutionTests.cs b/PolyPilot.Tests/CliPathResolutionTests.cs new file mode 100644 index 0000000000..4d37a7baf3 --- /dev/null +++ b/PolyPilot.Tests/CliPathResolutionTests.cs @@ -0,0 +1,312 @@ +using System.Runtime.InteropServices; +using GitHub.Copilot.SDK; +using PolyPilot.Models; + +namespace PolyPilot.Tests; + +/// +/// Tests for CLI path resolution logic and CopilotClientOptions behavior +/// around CliPath, CliUrl, and CliSourceMode configuration. +/// +public class CliPathResolutionTests +{ + [Fact] + public void CliSourceMode_BuiltIn_IsDefault() + { + var settings = new ConnectionSettings(); + Assert.Equal(CliSourceMode.BuiltIn, settings.CliSource); + } + + [Fact] + public void CliSourceMode_System_IsOne() + { + Assert.Equal(1, (int)CliSourceMode.System); + } + + [Fact] + public void CopilotClientOptions_CliPath_AcceptsNonExistentPath() + { + // Setting CliPath to a non-existent path doesn't throw at construction time; + // failure is deferred until StartAsync. + var options = new CopilotClientOptions(); + options.CliPath = "/nonexistent/path/to/copilot"; + + var client = new CopilotClient(options); + Assert.NotNull(client); + } + + [Fact] + public void CopilotClientOptions_CliPath_AcceptsNull() + { + var options = new CopilotClientOptions(); + options.CliPath = null; + + Assert.Null(options.CliPath); + } + + [Fact] + public void CopilotClientOptions_DefaultCliPath_AutoDiscoveryState() + { + // The SDK may auto-discover a CliPath or set UseStdio depending on environment. + // This test documents the observed behavior in the test context. + var options = new CopilotClientOptions(); + + bool hasCliPath = !string.IsNullOrEmpty(options.CliPath); + bool hasUseStdio = options.UseStdio; + + // Per PersistentModeTests, at least one should be set. If both are false, + // the SDK has no CLI to launch and embedded mode will fail at StartAsync. + Assert.True(hasCliPath || hasUseStdio, + $"Expected SDK to auto-set CliPath or UseStdio. CliPath='{options.CliPath}', UseStdio={options.UseStdio}"); + } + + [Fact] + public void CopilotClientOptions_CliPath_CanBeOverridden() + { + var options = new CopilotClientOptions(); + options.CliPath = "/custom/path"; + + Assert.Equal("/custom/path", options.CliPath); + } + + [Fact] + public void EmbeddedMode_WithCustomCliPath_CreatesValidClient() + { + // Use the SDK-discovered default path to verify CopilotClient creation works. + var defaultOptions = new CopilotClientOptions(); + var discoveredPath = defaultOptions.CliPath; + + var options = new CopilotClientOptions(); + options.CliPath = discoveredPath; + + var client = new CopilotClient(options); + Assert.NotNull(client); + } + + [Fact] + public void PersistentMode_NullCliPath_RequiredForCliUrl() + { + // CliPath must be null before CliUrl can be set without throwing. + var options = new CopilotClientOptions(); + options.CliPath = null; + options.UseStdio = false; + options.AutoStart = false; + options.CliUrl = "http://localhost:4321"; + + var client = new CopilotClient(options); + Assert.NotNull(client); + + // Confirm the inverse: setting CliUrl with a non-null CliPath throws. + var options2 = new CopilotClientOptions(); + options2.CliUrl = "http://localhost:4321"; + + Assert.Throws(() => new CopilotClient(options2)); + } + + // ================================================================ + // Bundled-only CLI resolution tests + // ================================================================ + // These tests verify that the copilot binary can be found when there + // is NO global install (no homebrew, no npm global). The SDK ships a + // bundled binary under runtimes/{rid}/native/copilot. + + [Fact] + public void SdkAutoDiscoveredCliPath_IsNotNull() + { + // The SDK default CliPath is null — it uses UseStdio instead. + // Our app's ResolveBundledCliPath provides the actual binary path. + // Verify that when we manually construct the bundled path (same logic + // as GetBundledCliPath in CopilotService), the file exists. + var assemblyDir = Path.GetDirectoryName(typeof(CopilotClient).Assembly.Location); + Assert.NotNull(assemblyDir); + + var rid = RuntimeInformation.RuntimeIdentifier; + var bundledPath = Path.Combine(assemblyDir!, "runtimes", rid, "native", "copilot"); + + Assert.True(File.Exists(bundledPath), + $"Bundled copilot binary not found at: {bundledPath} (RID={rid})"); + } + + [Fact] + public void SdkAutoDiscoveredCliPath_PointsToRuntimesDir() + { + // The bundled binary path follows the runtimes/{rid}/native/copilot convention. + // This is the path that ResolveBundledCliPath (via GetBundledCliPath) constructs. + var assemblyDir = Path.GetDirectoryName(typeof(CopilotClient).Assembly.Location); + Assert.NotNull(assemblyDir); + + var rid = RuntimeInformation.RuntimeIdentifier; + var bundledPath = Path.Combine(assemblyDir!, "runtimes", rid, "native", "copilot"); + + Assert.Contains("runtimes", bundledPath); + Assert.Contains(rid, bundledPath); + Assert.EndsWith("copilot", bundledPath); + } + + [Fact] + public void EmbeddedMode_WithNullResolvedPath_StillHasSdkDefault() + { + // When ResolveCopilotCliPath returns null, CopilotService.CreateClient does NOT + // set options.CliPath. The SDK's default CliPath is also null, but UseStdio is true. + // This means the SDK falls back to UseStdio mode, which auto-discovers the binary + // by searching for "copilot" in standard locations including the runtimes/ dir. + var options = new CopilotClientOptions(); + + // SDK default: CliPath is null, UseStdio is true + Assert.Null(options.CliPath); + Assert.True(options.UseStdio, + "SDK default should have UseStdio=true as fallback when CliPath is null"); + + // Simulate what happens when ResolveCopilotCliPath returns null: + // CopilotService.CreateClient does NOT override CliPath → SDK uses UseStdio + string? cliPath = null; + if (cliPath != null) + options.CliPath = cliPath; + + // UseStdio remains true — the SDK will find the binary via its own resolution + Assert.True(options.UseStdio); + } + + [Fact] + public void EmbeddedMode_BuiltIn_PrefersBundled() + { + // CliSourceMode.BuiltIn (default) means the bundled binary is tried before system paths. + // ResolveCopilotCliPath(BuiltIn) calls ResolveBundledCliPath() first, then ResolveSystemCliPath(). + // This ensures users without a global install always get the bundled binary. + var settings = new ConnectionSettings(); + Assert.Equal(CliSourceMode.BuiltIn, settings.CliSource); + + // Verify the bundled path exists and is NOT a system path + var assemblyDir = Path.GetDirectoryName(typeof(CopilotClient).Assembly.Location); + Assert.NotNull(assemblyDir); + + var rid = RuntimeInformation.RuntimeIdentifier; + var bundledPath = Path.Combine(assemblyDir!, "runtimes", rid, "native", "copilot"); + + Assert.True(File.Exists(bundledPath), + $"Bundled copilot binary should exist at: {bundledPath}"); + + // Bundled path should NOT be a system path like /opt/homebrew or /usr/local + Assert.DoesNotContain("/opt/homebrew/", bundledPath); + Assert.DoesNotContain("/usr/local/lib/node_modules/", bundledPath); + } + + [Fact] + public void PersistentMode_BypassesCliPath_UsesCliUrl() + { + // In Persistent mode, CliPath is set to null and CliUrl is used instead. + // Binary resolution doesn't matter for client creation in this mode, + // but ServerManager.FindCopilotBinary() is still used to spawn the server process. + var options = new CopilotClientOptions(); + options.CliPath = null; + options.UseStdio = false; + options.AutoStart = false; + options.CliUrl = "http://localhost:4321"; + + // Client creation succeeds without any CliPath + var client = new CopilotClient(options); + Assert.NotNull(client); + Assert.Null(options.CliPath); + } + + [Fact] + public void ServerManager_WouldUseBundledPath_WhenNoSystem() + { + // ServerManager.FindCopilotBinary() checks system paths first (homebrew, /usr/local) + // but falls back to CopilotService.ResolveBundledCliPath(), which checks: + // 1. runtimes/{rid}/native/copilot (SDK bundled path) + // 2. {assemblyDir}/copilot (MonoBundle fallback for Mac Catalyst) + // + // This means persistent mode works without a global copilot install, + // as long as the SDK's bundled binary exists. + // + // We can't call FindCopilotBinary directly (it's in the MAUI project), + // but we can verify the bundled path it would resolve to. + var assemblyDir = Path.GetDirectoryName(typeof(CopilotClient).Assembly.Location); + Assert.NotNull(assemblyDir); + + var rid = RuntimeInformation.RuntimeIdentifier; + var bundledPath = Path.Combine(assemblyDir!, "runtimes", rid, "native", "copilot"); + + // The path should be well-formed and match the pattern ServerManager expects + Assert.Contains("runtimes", bundledPath); + Assert.Contains("native", bundledPath); + Assert.EndsWith("copilot", bundledPath); + } + + [Fact] + public void BundledBinary_ExistsInBuildOutput() + { + // CRITICAL: Verify the copilot binary actually exists at the SDK-expected path + // relative to the test assembly. The GitHub.Copilot.SDK NuGet package ships the + // binary under runtimes/{rid}/native/copilot. + var assemblyDir = Path.GetDirectoryName(typeof(CopilotClient).Assembly.Location); + Assert.NotNull(assemblyDir); + + var rid = RuntimeInformation.RuntimeIdentifier; + var expectedPath = Path.Combine(assemblyDir!, "runtimes", rid, "native", "copilot"); + + // The test project runs as net10.0 (not maccatalyst), so the RID is typically + // osx-arm64 on Apple Silicon Macs. The SDK packages the binary for this RID. + if (!File.Exists(expectedPath)) + { + // Try common alternative RIDs in case the exact RID doesn't match + var alternativeRids = new[] { "osx-arm64", "osx-x64", "maccatalyst-arm64" }; + var found = false; + var checkedPaths = new List { expectedPath }; + + foreach (var altRid in alternativeRids) + { + if (altRid == rid) continue; + var altPath = Path.Combine(assemblyDir!, "runtimes", altRid, "native", "copilot"); + checkedPaths.Add(altPath); + if (File.Exists(altPath)) + { + found = true; + break; + } + } + + // Also check the SDK's own auto-discovered path + var sdkPath = new CopilotClientOptions().CliPath; + if (sdkPath != null && File.Exists(sdkPath)) + { + found = true; + checkedPaths.Add($"SDK auto-discovered: {sdkPath}"); + } + + Assert.True(found, + $"Bundled copilot binary not found at any expected path. " + + $"RID='{rid}', Checked: [{string.Join(", ", checkedPaths)}]"); + } + else + { + // Binary exists at the exact expected path — verify it's executable + Assert.True(File.Exists(expectedPath), + $"Bundled copilot binary should exist at: {expectedPath}"); + } + } + + [Fact] + public void MonoBundleFallback_PathIsAssemblyDir() + { + // The MonoBundle fallback looks for "copilot" in the same directory as the SDK assembly. + // On Mac Catalyst, MAUI flattens runtimes/ into Contents/MonoBundle/, so the binary + // ends up alongside the SDK DLL. This test documents that fallback path. + var assemblyDir = Path.GetDirectoryName(typeof(CopilotClient).Assembly.Location); + Assert.NotNull(assemblyDir); + + var monoBundlePath = Path.Combine(assemblyDir!, "copilot"); + + // In test context, the MonoBundle path typically doesn't exist (we're not in a .app bundle). + // But the path should be well-formed and point to the assembly directory. + Assert.Equal(assemblyDir, Path.GetDirectoryName(monoBundlePath)); + Assert.Equal("copilot", Path.GetFileName(monoBundlePath)); + + // The runtimes/{rid}/native/ path is the primary bundled path; + // MonoBundle is only a fallback for the Mac Catalyst app bundle layout. + var rid = RuntimeInformation.RuntimeIdentifier; + var primaryPath = Path.Combine(assemblyDir!, "runtimes", rid, "native", "copilot"); + Assert.NotEqual(primaryPath, monoBundlePath); + } +} diff --git a/PolyPilot.Tests/ConnectionSettingsTests.cs b/PolyPilot.Tests/ConnectionSettingsTests.cs index b6601d5d07..0077eed025 100644 --- a/PolyPilot.Tests/ConnectionSettingsTests.cs +++ b/PolyPilot.Tests/ConnectionSettingsTests.cs @@ -134,6 +134,80 @@ public void JsonSerialization_ModeAsInt() Assert.Contains("\"Mode\":1", json); } + [Fact] + public void DefaultValues_NewFields_AreCorrect() + { + var settings = new ConnectionSettings(); + + Assert.Null(settings.ServerPassword); + Assert.False(settings.DirectSharingEnabled); + Assert.Equal(CliSourceMode.BuiltIn, settings.CliSource); + } + + [Fact] + public void Save_Load_RoundTrip_WithNewFields() + { + var original = new ConnectionSettings + { + Mode = ConnectionMode.Embedded, + Host = "localhost", + Port = 4321, + ServerPassword = "mypass", + DirectSharingEnabled = true, + CliSource = CliSourceMode.System + }; + + var json = JsonSerializer.Serialize(original); + var loaded = JsonSerializer.Deserialize(json); + + Assert.NotNull(loaded); + Assert.Equal("mypass", loaded!.ServerPassword); + Assert.True(loaded.DirectSharingEnabled); + Assert.Equal(CliSourceMode.System, loaded.CliSource); + } + + [Fact] + public void BackwardCompatibility_OldJsonWithoutNewFields() + { + var json = """{"Mode":0,"Host":"oldhost","Port":1234}"""; + var loaded = JsonSerializer.Deserialize(json); + + Assert.NotNull(loaded); + Assert.Equal("oldhost", loaded!.Host); + Assert.Equal(1234, loaded.Port); + Assert.Null(loaded.ServerPassword); + Assert.False(loaded.DirectSharingEnabled); + Assert.Equal(CliSourceMode.BuiltIn, loaded.CliSource); + } + + [Fact] + public void ServerPassword_NotInCliUrl() + { + var settings = new ConnectionSettings + { + Host = "myhost", + Port = 5555, + ServerPassword = "secret123" + }; + + Assert.Equal("myhost:5555", settings.CliUrl); + Assert.DoesNotContain("secret123", settings.CliUrl); + } + + [Fact] + public void DirectSharingEnabled_DefaultFalse() + { + var settings = new ConnectionSettings(); + Assert.False(settings.DirectSharingEnabled); + } + + [Fact] + public void CliSourceMode_Enum_HasExpectedValues() + { + Assert.Equal(0, (int)CliSourceMode.BuiltIn); + Assert.Equal(1, (int)CliSourceMode.System); + } + private void Dispose() { try { Directory.Delete(_testDir, true); } catch { } diff --git a/PolyPilot.Tests/InitializationModeTests.cs b/PolyPilot.Tests/InitializationModeTests.cs new file mode 100644 index 0000000000..d369b8e4c3 --- /dev/null +++ b/PolyPilot.Tests/InitializationModeTests.cs @@ -0,0 +1,187 @@ +using GitHub.Copilot.SDK; +using PolyPilot.Models; + +namespace PolyPilot.Tests; + +/// +/// Tests for initialization mode branching logic. +/// Validates that ConnectionSettings and CopilotClientOptions +/// behave correctly for each ConnectionMode path in CopilotService.InitializeAsync. +/// +public class InitializationModeTests +{ + // --- Helper: mirrors CreateClient option-building logic from CopilotService --- + + private static CopilotClientOptions BuildClientOptions(ConnectionSettings settings) + { + var options = new CopilotClientOptions(); + + if (settings.Mode == ConnectionMode.Persistent) + { + options.CliPath = null; + options.UseStdio = false; + options.AutoStart = false; + options.CliUrl = $"http://{settings.Host}:{settings.Port}"; + } + + return options; + } + + [Fact] + public void PersistentMode_RequiresHostAndPort() + { + var settings = new ConnectionSettings + { + Mode = ConnectionMode.Persistent, + Host = "192.168.1.50", + Port = 5555 + }; + + var options = BuildClientOptions(settings); + + Assert.Equal("http://192.168.1.50:5555", options.CliUrl); + Assert.Null(options.CliPath); + Assert.False(options.UseStdio); + } + + [Fact] + public void RemoteMode_WithNoUrl_IsNotReady() + { + var settings = new ConnectionSettings + { + Mode = ConnectionMode.Remote, + RemoteUrl = null + }; + + // CliUrl falls back to Host:Port which is not a valid remote endpoint + var cliUrl = settings.CliUrl; + + Assert.Equal($"{settings.Host}:{settings.Port}", cliUrl); + Assert.DoesNotContain("http", cliUrl); + } + + [Fact] + public void RemoteMode_WithUrl_CliUrl_IsRemoteUrl() + { + var settings = new ConnectionSettings + { + Mode = ConnectionMode.Remote, + RemoteUrl = "https://my-tunnel.devtunnels.ms" + }; + + Assert.Equal("https://my-tunnel.devtunnels.ms", settings.CliUrl); + } + + [Fact] + public void DemoMode_NoNetworkRequired() + { + var settings = new ConnectionSettings + { + Mode = ConnectionMode.Demo, + Host = "", + Port = 0, + RemoteUrl = null + }; + + // Demo mode should not need BuildClientOptions at all; + // verify settings are valid without network config + Assert.Equal(ConnectionMode.Demo, settings.Mode); + Assert.Null(settings.RemoteUrl); + } + + [Fact] + public void PersistentMode_FallbackToEmbedded_CliPathValid() + { + // Simulate fallback: build Embedded options (no CliUrl, CliPath available) + var settings = new ConnectionSettings { Mode = ConnectionMode.Embedded }; + + var options = BuildClientOptions(settings); + + // Embedded mode must NOT set CliUrl (SDK uses CliPath + stdio instead) + Assert.Null(options.CliUrl); + // SDK auto-discovers CliPath or UseStdio + bool hasAutoPath = !string.IsNullOrEmpty(options.CliPath); + bool hasAutoStdio = options.UseStdio; + Assert.True(hasAutoPath || hasAutoStdio, + "Embedded fallback must have CliPath or UseStdio set by SDK defaults"); + } + + [Fact] + public void EmbeddedMode_NoServerNeeded() + { + var settings = new ConnectionSettings + { + Mode = ConnectionMode.Embedded, + Host = "unreachable-host", + Port = 99999 + }; + + var options = BuildClientOptions(settings); + + // Embedded mode ignores Host/Port — no CliUrl set + Assert.Null(options.CliUrl); + // SDK defaults AutoStart=true for Embedded (auto-spawn copilot process) + Assert.True(options.AutoStart); + } + + [Fact] + public void AllModes_HaveDistinctEnumValues() + { + Assert.Equal(0, (int)ConnectionMode.Embedded); + Assert.Equal(1, (int)ConnectionMode.Persistent); + Assert.Equal(2, (int)ConnectionMode.Remote); + Assert.Equal(3, (int)ConnectionMode.Demo); + } + + [Fact] + public void PersistentMode_CliUrl_Format() + { + var settings = new ConnectionSettings + { + Mode = ConnectionMode.Persistent, + Host = "localhost", + Port = 4321 + }; + + var options = BuildClientOptions(settings); + + Assert.NotNull(options.CliUrl); + Assert.StartsWith("http://", options.CliUrl); + Assert.Contains("localhost", options.CliUrl); + Assert.Contains("4321", options.CliUrl); + } + + [Fact] + public void ModeSwitch_PersistentToEmbedded_ChangesOptions() + { + var persistentSettings = new ConnectionSettings + { + Mode = ConnectionMode.Persistent, + Host = "localhost", + Port = 4321 + }; + var embeddedSettings = new ConnectionSettings + { + Mode = ConnectionMode.Embedded + }; + + var persistentOptions = BuildClientOptions(persistentSettings); + var embeddedOptions = BuildClientOptions(embeddedSettings); + + // Persistent sets CliUrl; Embedded does not + Assert.NotNull(persistentOptions.CliUrl); + Assert.Null(embeddedOptions.CliUrl); + + // Persistent clears UseStdio; Embedded keeps SDK default + Assert.False(persistentOptions.UseStdio); + } + + [Fact] + public void PersistentMode_DefaultPort() + { + var settings = new ConnectionSettings(); + + Assert.Equal(4321, settings.Port); + Assert.Equal("localhost", settings.Host); + } +} diff --git a/PolyPilot.Tests/PersistentModeTests.cs b/PolyPilot.Tests/PersistentModeTests.cs new file mode 100644 index 0000000000..70ad1967de --- /dev/null +++ b/PolyPilot.Tests/PersistentModeTests.cs @@ -0,0 +1,403 @@ +using GitHub.Copilot.SDK; +using PolyPilot.Models; + +namespace PolyPilot.Tests; + +/// +/// Tests for Persistent mode client configuration. +/// Validates that CopilotClientOptions are set correctly for each ConnectionMode, +/// respecting the SDK constraint: CliUrl is mutually exclusive with UseStdio and CliPath. +/// +public class PersistentModeTests +{ + // --- SDK default discovery tests --- + + [Fact] + public void CopilotClientOptions_Defaults_DiscoverAutoSetFields() + { + // Discover what the SDK auto-sets so we know what to clear for Persistent mode. + var options = new CopilotClientOptions(); + + // The SDK auto-discovers a bundled CliPath and/or sets UseStdio. + // At least one of these must be true for the CliUrl conflict to occur. + bool hasAutoPath = !string.IsNullOrEmpty(options.CliPath); + bool hasAutoStdio = options.UseStdio; + + // At least one is auto-set (this is why CliUrl alone throws) + Assert.True(hasAutoPath || hasAutoStdio, + $"Expected SDK to auto-set CliPath or UseStdio. CliPath='{options.CliPath}', UseStdio={options.UseStdio}"); + } + + // --- SDK constraint tests --- + + [Fact] + public void CopilotClientOptions_CliUrl_WithClearedDefaults_DoesNotThrow() + { + // When connecting to an existing persistent server, we must: + // 1. Clear CliPath (auto-discovered by SDK) + // 2. Clear UseStdio (may default to true) + // 3. Set AutoStart = false + // 4. Set CliUrl + var options = new CopilotClientOptions(); + options.CliPath = null; + options.UseStdio = false; + options.AutoStart = false; + options.CliUrl = "http://localhost:4321"; + + var client = new CopilotClient(options); + Assert.NotNull(client); + } + + [Fact] + public void CopilotClientOptions_CliUrl_WithoutClearingDefaults_Throws() + { + // Proves that just setting CliUrl on a fresh options object throws + // because of auto-discovered CliPath or UseStdio. + var options = new CopilotClientOptions(); + options.CliUrl = "http://localhost:4321"; + + Assert.Throws(() => new CopilotClient(options)); + } + + [Fact] + public void CopilotClientOptions_CliUrl_WithCliPath_Throws() + { + var options = new CopilotClientOptions(); + options.CliPath = null; + options.UseStdio = false; + options.CliUrl = "http://localhost:4321"; + options.CliPath = "/some/path/to/copilot"; + + Assert.Throws(() => new CopilotClient(options)); + } + + [Fact] + public void CopilotClientOptions_CliUrl_WithUseStdio_Throws() + { + var options = new CopilotClientOptions(); + options.CliPath = null; + options.CliUrl = "http://localhost:4321"; + options.UseStdio = true; + + Assert.Throws(() => new CopilotClient(options)); + } + + // --- ConnectionSettings.CliUrl property tests --- + + [Fact] + public void ConnectionSettings_PersistentMode_CliUrl_ReturnsHostPort() + { + var settings = new ConnectionSettings + { + Mode = ConnectionMode.Persistent, + Host = "localhost", + Port = 4321 + }; + + Assert.Equal("localhost:4321", settings.CliUrl); + } + + [Fact] + public void ConnectionSettings_PersistentMode_CustomPort_CliUrl_ReturnsCorrectly() + { + var settings = new ConnectionSettings + { + Mode = ConnectionMode.Persistent, + Host = "localhost", + Port = 5555 + }; + + Assert.Equal("localhost:5555", settings.CliUrl); + } + + [Fact] + public void ConnectionSettings_EmbeddedMode_CliUrl_ReturnsHostPort() + { + var settings = new ConnectionSettings + { + Mode = ConnectionMode.Embedded, + Host = "localhost", + Port = 4321 + }; + + Assert.Equal("localhost:4321", settings.CliUrl); + } + + // --- Option configuration logic tests --- + + [Fact] + public void PersistentMode_ConfiguresOptionsForTcpConnection() + { + var settings = new ConnectionSettings + { + Mode = ConnectionMode.Persistent, + Host = "localhost", + Port = 4321 + }; + + var options = BuildClientOptions(settings); + + Assert.Equal($"http://{settings.Host}:{settings.Port}", options.CliUrl); + Assert.False(options.AutoStart); + Assert.Null(options.CliPath); + Assert.Null(options.CliArgs); + Assert.False(options.UseStdio); + } + + [Fact] + public void PersistentMode_WithCustomPort_ConfiguresCorrectCliUrl() + { + var settings = new ConnectionSettings + { + Mode = ConnectionMode.Persistent, + Host = "localhost", + Port = 9999 + }; + + var options = BuildClientOptions(settings); + + Assert.Equal("http://localhost:9999", options.CliUrl); + Assert.False(options.AutoStart); + } + + [Fact] + public void PersistentMode_DoesNotSetCliPath() + { + var settings = new ConnectionSettings + { + Mode = ConnectionMode.Persistent, + Host = "localhost", + Port = 4321 + }; + + var options = BuildClientOptions(settings); + + Assert.Null(options.CliPath); + } + + [Fact] + public void PersistentMode_DoesNotSetCliArgs() + { + var settings = new ConnectionSettings + { + Mode = ConnectionMode.Persistent, + Host = "localhost", + Port = 4321 + }; + + var options = BuildClientOptions(settings); + + Assert.Null(options.CliArgs); + } + + [Fact] + public void PersistentMode_ClearsUseStdio() + { + var settings = new ConnectionSettings + { + Mode = ConnectionMode.Persistent, + Host = "localhost", + Port = 4321 + }; + + var options = BuildClientOptions(settings); + + Assert.False(options.UseStdio); + } + + [Fact] + public void EmbeddedMode_DoesNotSetCliUrl() + { + var settings = new ConnectionSettings + { + Mode = ConnectionMode.Embedded, + Host = "localhost", + Port = 4321 + }; + + var options = BuildClientOptions(settings); + + Assert.Null(options.CliUrl); + } + + [Fact] + public void PersistentMode_OptionsAreValidForCopilotClient() + { + // End-to-end: build options for Persistent mode and verify the SDK accepts them. + var settings = new ConnectionSettings + { + Mode = ConnectionMode.Persistent, + Host = "localhost", + Port = 4321 + }; + + var options = BuildClientOptions(settings); + + // This must NOT throw ArgumentException about mutual exclusivity. + var client = new CopilotClient(options); + Assert.NotNull(client); + } + + [Fact] + public void EmbeddedMode_OptionsDoNotConflict() + { + var settings = new ConnectionSettings + { + Mode = ConnectionMode.Embedded, + Host = "localhost", + Port = 4321 + }; + + var options = BuildClientOptions(settings); + + Assert.Null(options.CliUrl); + } + + [Fact] + public void EmbeddedMode_OptionsAreValidForCopilotClient() + { + // Embedded path doesn't set CliUrl, so SDK defaults are fine. + var settings = new ConnectionSettings + { + Mode = ConnectionMode.Embedded, + Host = "localhost", + Port = 4321 + }; + + var options = BuildClientOptions(settings); + + // Must NOT throw — SDK auto-discovers CliPath for embedded mode. + var client = new CopilotClient(options); + Assert.NotNull(client); + } + + [Fact] + public void PersistentMode_HttpPrefixRequired() + { + var settings = new ConnectionSettings + { + Mode = ConnectionMode.Persistent, + Host = "localhost", + Port = 4321 + }; + + var options = BuildClientOptions(settings); + + Assert.StartsWith("http://", options.CliUrl); + } + + [Fact] + public void PersistentMode_NonLocalhostHost() + { + var settings = new ConnectionSettings + { + Mode = ConnectionMode.Persistent, + Host = "192.168.1.100", + Port = 4321 + }; + + var options = BuildClientOptions(settings); + + Assert.Equal("http://192.168.1.100:4321", options.CliUrl); + } + + [Fact] + public void EmbeddedMode_DoesNotClearCliPath() + { + // Verify Embedded mode preserves whatever CliPath the SDK auto-discovers, + // unlike Persistent mode which explicitly nulls it out. + var settings = new ConnectionSettings + { + Mode = ConnectionMode.Embedded, + Host = "localhost", + Port = 4321 + }; + + var defaultOptions = new CopilotClientOptions(); + var builtOptions = BuildClientOptions(settings); + + // CliPath should be unchanged from SDK default (not cleared like Persistent mode does). + Assert.Equal(defaultOptions.CliPath, builtOptions.CliPath); + } + + [Fact] + public void PersistentMode_SetsAutoStartFalse() + { + var settings = new ConnectionSettings + { + Mode = ConnectionMode.Persistent, + Host = "localhost", + Port = 4321 + }; + + var options = BuildClientOptions(settings); + + Assert.False(options.AutoStart); + } + + [Fact] + public void DemoMode_TreatedAsEmbedded() + { + // Demo mode takes the else branch — no CliUrl set. + var settings = new ConnectionSettings + { + Mode = ConnectionMode.Demo, + Host = "localhost", + Port = 4321 + }; + + var options = BuildClientOptions(settings); + + Assert.Null(options.CliUrl); + } + + [Fact] + public void RemoteMode_NotHandledByCreateClient() + { + // Remote mode takes the else branch — handled separately by InitializeRemoteAsync. + var settings = new ConnectionSettings + { + Mode = ConnectionMode.Remote, + Host = "localhost", + Port = 4321 + }; + + var options = BuildClientOptions(settings); + + Assert.Null(options.CliUrl); + } + + // --- Helper: mirrors CreateClient option-building logic from CopilotService --- + + /// + /// Extracted option-building logic from CopilotService.CreateClient(). + /// This must stay in sync with the actual implementation. + /// Only Persistent mode sets CliUrl; all other modes (Embedded, Demo, Remote) + /// take the else branch and leave SDK defaults intact. + /// + private static CopilotClientOptions BuildClientOptions(ConnectionSettings settings) + { + var options = new CopilotClientOptions(); + + if (settings.Mode == ConnectionMode.Persistent) + { + // Connect to existing headless server via TCP. + // Must clear auto-discovered CliPath and UseStdio first — + // CliUrl is mutually exclusive with both. + options.CliPath = null; + options.UseStdio = false; + options.AutoStart = false; + options.CliUrl = $"http://{settings.Host}:{settings.Port}"; + } + else + { + // Embedded, Demo, Remote: CliPath would be set here in the real code, + // but we skip it in tests since binary may not exist. + // The important thing is that CliUrl is NOT set. + // Remote mode is handled separately by InitializeRemoteAsync. + } + + return options; + } +} + diff --git a/PolyPilot.Tests/PolyPilot.Tests.csproj b/PolyPilot.Tests/PolyPilot.Tests.csproj index 2597654552..6908b13951 100644 --- a/PolyPilot.Tests/PolyPilot.Tests.csproj +++ b/PolyPilot.Tests/PolyPilot.Tests.csproj @@ -9,6 +9,7 @@ + diff --git a/PolyPilot.Tests/WsBridgeServerAuthTests.cs b/PolyPilot.Tests/WsBridgeServerAuthTests.cs new file mode 100644 index 0000000000..148686da8f --- /dev/null +++ b/PolyPilot.Tests/WsBridgeServerAuthTests.cs @@ -0,0 +1,142 @@ +using System.Text.Json; +using PolyPilot.Models; + +namespace PolyPilot.Tests; + +public class WsBridgeServerAuthTests +{ + [Fact] + public void ConnectionSettings_ServerPassword_DefaultsToNull() + { + var settings = new ConnectionSettings(); + Assert.Null(settings.ServerPassword); + } + + [Fact] + public void ConnectionSettings_ServerPassword_Serializes() + { + var settings = new ConnectionSettings { ServerPassword = "my-secret-pw" }; + var json = JsonSerializer.Serialize(settings); + var loaded = JsonSerializer.Deserialize(json); + + Assert.NotNull(loaded); + Assert.Equal("my-secret-pw", loaded!.ServerPassword); + } + + [Fact] + public void ConnectionSettings_DirectSharingEnabled_DefaultsFalse() + { + var settings = new ConnectionSettings(); + Assert.False(settings.DirectSharingEnabled); + } + + [Fact] + public void ConnectionSettings_DirectSharingEnabled_Serializes() + { + var settings = new ConnectionSettings { DirectSharingEnabled = true }; + var json = JsonSerializer.Serialize(settings); + var loaded = JsonSerializer.Deserialize(json); + + Assert.NotNull(loaded); + Assert.True(loaded!.DirectSharingEnabled); + } + + [Fact] + public void BridgeAuthContract_PasswordNotInRemoteToken() + { + var settings = new ConnectionSettings + { + ServerPassword = "server-pw", + RemoteToken = "remote-tok" + }; + + Assert.Equal("server-pw", settings.ServerPassword); + Assert.Equal("remote-tok", settings.RemoteToken); + + // Changing one doesn't affect the other + settings.ServerPassword = "changed"; + Assert.Equal("remote-tok", settings.RemoteToken); + + settings.RemoteToken = "changed-tok"; + Assert.Equal("changed", settings.ServerPassword); + } + + [Fact] + public void BridgeAuthContract_PasswordIndependentOfTunnelId() + { + var settings = new ConnectionSettings + { + ServerPassword = "pw123", + TunnelId = "tunnel-xyz" + }; + + Assert.Equal("pw123", settings.ServerPassword); + Assert.Equal("tunnel-xyz", settings.TunnelId); + + settings.TunnelId = "other-tunnel"; + Assert.Equal("pw123", settings.ServerPassword); + + settings.ServerPassword = null; + Assert.Equal("other-tunnel", settings.TunnelId); + } + + [Fact] + public void ConnectionSettings_FullConfig_WithPassword_RoundTrips() + { + var original = new ConnectionSettings + { + Mode = ConnectionMode.Persistent, + Host = "192.168.1.50", + Port = 8080, + ServerPassword = "secret", + DirectSharingEnabled = true, + RemoteUrl = "https://tunnel.example.com", + RemoteToken = "tok-abc", + TunnelId = "tun-001", + AutoStartServer = true, + AutoStartTunnel = true + }; + + var json = JsonSerializer.Serialize(original, new JsonSerializerOptions { WriteIndented = true }); + var loaded = JsonSerializer.Deserialize(json); + + Assert.NotNull(loaded); + Assert.Equal(ConnectionMode.Persistent, loaded!.Mode); + Assert.Equal("192.168.1.50", loaded.Host); + Assert.Equal(8080, loaded.Port); + Assert.Equal("secret", loaded.ServerPassword); + Assert.True(loaded.DirectSharingEnabled); + Assert.Equal("https://tunnel.example.com", loaded.RemoteUrl); + Assert.Equal("tok-abc", loaded.RemoteToken); + Assert.Equal("tun-001", loaded.TunnelId); + Assert.True(loaded.AutoStartServer); + Assert.True(loaded.AutoStartTunnel); + } + + [Fact] + public void ConnectionSettings_BackwardCompat_NoPassword() + { + // JSON from an older version that doesn't have ServerPassword or DirectSharingEnabled + var json = """ + { + "Mode": 1, + "Host": "localhost", + "Port": 4321, + "AutoStartServer": false, + "RemoteUrl": null, + "RemoteToken": null, + "TunnelId": null, + "AutoStartTunnel": false + } + """; + + var loaded = JsonSerializer.Deserialize(json); + + Assert.NotNull(loaded); + Assert.Null(loaded!.ServerPassword); + Assert.False(loaded.DirectSharingEnabled); + Assert.Equal(ConnectionMode.Persistent, loaded.Mode); + Assert.Equal("localhost", loaded.Host); + Assert.Equal(4321, loaded.Port); + } +} diff --git a/PolyPilot/Services/CopilotService.cs b/PolyPilot/Services/CopilotService.cs index 79d9a05c00..0fe6ec27e1 100644 --- a/PolyPilot/Services/CopilotService.cs +++ b/PolyPilot/Services/CopilotService.cs @@ -410,17 +410,30 @@ private CopilotClient CreateClient(ConnectionSettings settings) // Note: Don't set Cwd here - each session sets its own WorkingDirectory in SessionConfig var options = new CopilotClientOptions(); - // Resolve the copilot CLI path based on user preference: - var cliPath = ResolveCopilotCliPath(settings.CliSource); - if (cliPath != null) - options.CliPath = cliPath; - - // Pass additional MCP server configs via CLI args. - // The CLI auto-reads ~/.copilot/mcp-config.json, but mcp-servers.json - // uses a different format that needs to be passed explicitly. - var mcpArgs = GetMcpCliArgs(); - if (mcpArgs.Length > 0) - options.CliArgs = mcpArgs; + if (settings.Mode == ConnectionMode.Persistent) + { + // Connect to the existing headless server via TCP instead of spawning a child process. + // Must clear auto-discovered CliPath and UseStdio first — + // CliUrl is mutually exclusive with both (SDK throws ArgumentException). + options.CliPath = null; + options.UseStdio = false; + options.AutoStart = false; + options.CliUrl = $"http://{settings.Host}:{settings.Port}"; + } + else + { + // Embedded mode: spawn copilot as a child process via stdio + var cliPath = ResolveCopilotCliPath(settings.CliSource); + if (cliPath != null) + options.CliPath = cliPath; + + // Pass additional MCP server configs via CLI args. + // The CLI auto-reads ~/.copilot/mcp-config.json, but mcp-servers.json + // uses a different format that needs to be passed explicitly. + var mcpArgs = GetMcpCliArgs(); + if (mcpArgs.Length > 0) + options.CliArgs = mcpArgs; + } return new CopilotClient(options); } @@ -449,7 +462,7 @@ private CopilotClient CreateClient(ConnectionSettings settings) /// /// Resolves the bundled CLI path (shipped with the app). /// - private static string? ResolveBundledCliPath() + internal static string? ResolveBundledCliPath() { // 1. SDK bundled path (runtimes/{rid}/native/copilot) var bundledPath = GetBundledCliPath(); @@ -957,6 +970,7 @@ public async Task ResumeSessionAsync(string sessionId, string // Load history: always parse events.jsonl as source of truth, then sync to DB List history = LoadHistoryFromDisk(sessionId); + if (history.Count > 0) { // Replace DB contents with fresh parse (events.jsonl may have grown since last DB sync) diff --git a/PolyPilot/Services/ServerManager.cs b/PolyPilot/Services/ServerManager.cs index 6a33bdd58f..e9fb18e440 100644 --- a/PolyPilot/Services/ServerManager.cs +++ b/PolyPilot/Services/ServerManager.cs @@ -241,6 +241,10 @@ private static string FindCopilotBinary() if (File.Exists(path)) return path; } + // Try the bundled binary from the SDK (MonoBundle/copilot or runtimes/{rid}/native/copilot) + var bundledPath = CopilotService.ResolveBundledCliPath(); + if (bundledPath != null) return bundledPath; + // Fallback to node wrapper (works if copilot is on PATH) return OperatingSystem.IsWindows() ? "copilot.cmd" : "copilot"; }