diff --git a/.gitignore b/.gitignore index 4cb4dbff8..a3153c598 100644 --- a/.gitignore +++ b/.gitignore @@ -42,3 +42,9 @@ docs/tests/screenshots/ bin/ obj/ artifacts/ + +# Visual Studio IDE artifacts +.vs/ +*.user +*.userprefs + diff --git a/src/Squad.Agents.AI/Squad.Agents.AI.csproj b/src/Squad.Agents.AI/Squad.Agents.AI.csproj index 0aae5bd11..7e0343c50 100644 --- a/src/Squad.Agents.AI/Squad.Agents.AI.csproj +++ b/src/Squad.Agents.AI/Squad.Agents.AI.csproj @@ -17,13 +17,22 @@ MIT README.md true - 0.1.0-preview + 0.2.0 - + + + diff --git a/src/Squad.Agents.AI/SquadAgent.cs b/src/Squad.Agents.AI/SquadAgent.cs index 2c5911c0a..b457b7e92 100644 --- a/src/Squad.Agents.AI/SquadAgent.cs +++ b/src/Squad.Agents.AI/SquadAgent.cs @@ -1,4 +1,4 @@ -using GitHub.Copilot.SDK; +using GitHub.Copilot; using Microsoft.Agents.AI; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -103,14 +103,33 @@ private static SquadAgentState BuildStateInternal(SquadAgentOptions options, ILo { var client = CreateCopilotClient(options, lf); - // Build a SessionConfig with a sensible default permission handler so the - // resulting AIAgent can call CreateSession without throwing. Callers can override - // via SquadAgentOptions.ConfigureSession (e.g., to inject a stricter handler or - // tweak instructions / available tools / model on the inner session). + // Build the SessionConfig. Two non-obvious settings are critical here for the + // SquadAgent to actually work end-to-end against a `.squad/`-initialised team: + // + // 1. ConfigDir + EnableConfigDiscovery = true + // Points the Copilot CLI at the .squad/ folder so it auto-discovers the + // team's agents, skills, instructions, and MCP servers at session start. + // Without these, the agent has to read .squad/team.md (and per-agent + // charters) via runtime file tools — every read becomes a permission + // request, the agent eventually gives up and reports "permission errors". + // + // 2. OnPermissionRequest = PermissionHandler.ApproveAll + // Required by the SDK (CreateSessionAsync throws without it). This is the + // SDK-protocol layer; the CLI also has its own per-tool gate which we + // open with --allow-all in CreateCopilotClient. + // + // Callers can override any of this via SquadAgentOptions.ConfigureSession. + var teamRoot = options.Cwd ?? options.SquadFolderPath; + var squadConfigDir = !string.IsNullOrEmpty(teamRoot) + ? Path.Combine(teamRoot, ".squad") + : null; + var sessionConfig = new SessionConfig { OnPermissionRequest = PermissionHandler.ApproveAll, - WorkingDirectory = options.Cwd ?? options.SquadFolderPath, + WorkingDirectory = teamRoot, + ConfigDirectory = squadConfigDir, + EnableConfigDiscovery = true, }; if (!string.IsNullOrEmpty(options.Instructions)) { @@ -155,17 +174,64 @@ private static CopilotClient CreateCopilotClient(SquadAgentOptions options, ILog resolvedToken = options.GitHubToken; } + // ── CLI path resolution ──────────────────────────────────────────── + // When this package is consumed via NuGet, GitHub.Copilot.SDK's build + // targets download the copilot CLI binary and copy it to bin/.../runtimes/ + // {rid}/native/copilot.exe. The SDK runtime looks there by default, so + // most consumers don't need to set anything. + // + // (Squad.Agents.AI keeps a *direct* PackageReference to GitHub.Copilot.SDK + // exactly to force those build targets to fire — without it the SDK is only + // a transitive dependency of Microsoft.Agents.AI.GitHub.Copilot and its + // build/ targets don't propagate. Once microsoft/agent-framework#6457 + // merges, this dance will no longer be required.) + // + // SquadAgentOptions.CliPath / CliArgs remain explicit overrides for advanced + // scenarios: custom CLI builds, sandboxed runners, air-gapped environments. + // SDK 1.0.0 expresses both via RuntimeConnection.ForStdio(path, args), so we + // build a single Connection when either is supplied. + // ─────────────────────────────────────────────────────────────────── var clientOptions = new CopilotClientOptions { - CliPath = options.CliPath, - Cwd = options.Cwd ?? options.SquadFolderPath, - GitHubToken = resolvedToken + WorkingDirectory = options.Cwd ?? options.SquadFolderPath, + GitHubToken = resolvedToken, }; - // Preserve CLI args parsed from connection strings or supplied by user code for the SDK's CLI invocation. - if (options.CliArgs.Count > 0) + // ── CLI permission flags ─────────────────────────────────────────── + // The Copilot CLI enforces THREE independent permission gates: + // 1. Tools (--allow-all-tools) — which tool kinds may run + // 2. Paths (--allow-all-paths) — which filesystem paths may be read/written + // 3. URLs (--allow-all-urls) — which URLs may be fetched + // + // The SDK-level `OnPermissionRequest` handler only covers the SDK protocol; + // each CLI gate is a separate verification step. We pass `--allow-all` by + // default so the SquadAgent can actually drive tool calls end-to-end against + // the entire Squad workspace. Hosts that want stricter behavior can replace + // this via SquadAgentOptions.CliArgs / the ConfigureCopilotClient delegate. + // ─────────────────────────────────────────────────────────────────── + // Skip our `--allow-all` default if the host already opted in via any of the + // CLI's permission-opening flags (or the omnibus `--yolo` alias). Comparison is + // case-insensitive because `copilot --help` documents the flags in lowercase but + // CLI argument parsers on Windows are commonly forgiving of case differences. + var combinedCliArgs = new List(); + bool hostAlreadyOpenedPermissions = options.CliArgs.Any(a => + string.Equals(a, "--allow-all", StringComparison.OrdinalIgnoreCase) || + string.Equals(a, "--allow-all-tools", StringComparison.OrdinalIgnoreCase) || + string.Equals(a, "--allow-all-paths", StringComparison.OrdinalIgnoreCase) || + string.Equals(a, "--allow-all-urls", StringComparison.OrdinalIgnoreCase) || + string.Equals(a, "--yolo", StringComparison.OrdinalIgnoreCase)); + if (!hostAlreadyOpenedPermissions) + { + combinedCliArgs.Add("--allow-all"); + } + combinedCliArgs.AddRange(options.CliArgs); + + // Only override the SDK's default child-process connection when the consumer + // supplied a custom CLI path or any extra CLI args. Otherwise let the SDK + // resolve its own bundled binary (downloaded via the build/ targets). + if (!string.IsNullOrEmpty(options.CliPath) || combinedCliArgs.Count > 0) { - clientOptions.CliArgs = options.CliArgs.ToArray(); + clientOptions.Connection = RuntimeConnection.ForStdio(options.CliPath, combinedCliArgs); } // Copy environment variables @@ -192,44 +258,34 @@ private static CopilotClient CreateCopilotClient(SquadAgentOptions options, ILog // restore routing-critical properties to prevent accidental or // malicious changes that would route the agent to a different CLI // process. (Picard Condition 1: hard routing gate.) + // + // SDK 1.0.0 collapsed CliPath/CliArgs into Connection (RuntimeConnection), + // so the routing gate now snapshots WorkingDirectory and Connection. if (options.ConfigureCopilotClient is not null) { - // Snapshot routing properties before the delegate runs - var snapshotCwd = clientOptions.Cwd; - var snapshotCliPath = clientOptions.CliPath; - // snapshot is a clone — in-place CliArgs mutation by SDK consumers is also caught - var cliArgsSnapshot = clientOptions.CliArgs?.ToArray(); + var snapshotWorkingDirectory = clientOptions.WorkingDirectory; + var snapshotConnection = clientOptions.Connection; options.ConfigureCopilotClient(clientOptions); - // Restore routing properties if changed (SC-3: post-delegate warning) bool restored = false; - if (!string.Equals(clientOptions.Cwd, snapshotCwd, StringComparison.Ordinal)) - { - logger?.LogWarning( - "ConfigureCopilotClient delegate changed Cwd from '{Original}' to '{Changed}'; " + - "restoring original value to preserve Squad routing.", - snapshotCwd, clientOptions.Cwd); - clientOptions.Cwd = snapshotCwd; - restored = true; - } - - if (!string.Equals(clientOptions.CliPath, snapshotCliPath, StringComparison.Ordinal)) + if (!string.Equals(clientOptions.WorkingDirectory, snapshotWorkingDirectory, StringComparison.Ordinal)) { logger?.LogWarning( - "ConfigureCopilotClient delegate changed CliPath from '{Original}' to '{Changed}'; " + + "ConfigureCopilotClient delegate changed WorkingDirectory from '{Original}' to '{Changed}'; " + "restoring original value to preserve Squad routing.", - snapshotCliPath, clientOptions.CliPath); - clientOptions.CliPath = snapshotCliPath; + snapshotWorkingDirectory, clientOptions.WorkingDirectory); + clientOptions.WorkingDirectory = snapshotWorkingDirectory; restored = true; } - if (!(clientOptions.CliArgs ?? Array.Empty()).SequenceEqual(cliArgsSnapshot ?? Array.Empty())) + if (!ReferenceEquals(clientOptions.Connection, snapshotConnection)) { logger?.LogWarning( - "ConfigureCopilotClient delegate changed CliArgs; " + - "restoring original value to preserve Squad routing."); - clientOptions.CliArgs = cliArgsSnapshot; + "ConfigureCopilotClient delegate replaced Connection; " + + "restoring original value to preserve Squad routing. " + + "Configure CLI path / args via SquadAgentOptions.CliPath / CliArgs instead."); + clientOptions.Connection = snapshotConnection; restored = true; } @@ -237,7 +293,7 @@ private static CopilotClient CreateCopilotClient(SquadAgentOptions options, ILog { logger?.LogWarning( "One or more routing properties were restored after ConfigureCopilotClient delegate ran. " + - "To set Cwd, CliPath, or CliArgs, configure them on SquadAgentOptions instead."); + "To set the team root, CLI path, or CLI args, configure them on SquadAgentOptions instead."); } } diff --git a/src/Squad.Agents.AI/SquadAgentOptions.cs b/src/Squad.Agents.AI/SquadAgentOptions.cs index eb4f796d0..a308b7ae3 100644 --- a/src/Squad.Agents.AI/SquadAgentOptions.cs +++ b/src/Squad.Agents.AI/SquadAgentOptions.cs @@ -1,5 +1,5 @@ using System.Text.Json.Serialization; -using GitHub.Copilot.SDK; +using GitHub.Copilot; namespace Squad.Agents.AI; diff --git a/src/Squad.Agents.AI/samples/Squad.Agents.AI.Sample/Program.cs b/src/Squad.Agents.AI/samples/Squad.Agents.AI.Sample/Program.cs index 9b6c3824c..6c49f490e 100644 --- a/src/Squad.Agents.AI/samples/Squad.Agents.AI.Sample/Program.cs +++ b/src/Squad.Agents.AI/samples/Squad.Agents.AI.Sample/Program.cs @@ -35,7 +35,6 @@ // Real-world usage: set SQUAD_TEAM_ROOT to the path of an initialized Squad team. var teamRoot = System.Environment.GetEnvironmentVariable("SQUAD_TEAM_ROOT") ?? System.IO.Directory.GetCurrentDirectory(); - PrintBanner($"Squad.Agents.AI v0.1 — sample run (team root: {teamRoot})"); Console.WriteLine(); @@ -278,7 +277,10 @@ static void PrintDone(string flow) catch (Exception ex) { Console.ForegroundColor = ConsoleColor.Red; - Console.WriteLine($"[ERROR] {ex.GetType().Name}: {ex.Message}"); + Console.WriteLine($"[ERROR] {ex.GetType().FullName}: {ex.Message}"); + if (ex.InnerException is not null) + Console.WriteLine($" inner: {ex.InnerException.GetType().FullName}: {ex.InnerException.Message}"); + Console.WriteLine(ex.StackTrace); Console.ResetColor(); return null; } @@ -307,12 +309,12 @@ static async Task RunStreamingWithErrorHandlingAsync(Func action) static bool IsCliMissingException(Exception ex) { - var msg = ex.Message; - return msg.Contains("copilot", StringComparison.OrdinalIgnoreCase) || - msg.Contains("not found", StringComparison.OrdinalIgnoreCase) || - msg.Contains("cannot find", StringComparison.OrdinalIgnoreCase) || - msg.Contains("No such file", StringComparison.OrdinalIgnoreCase) || - ex is System.ComponentModel.Win32Exception; + // Only treat *startup* failures (CLI binary missing) as the friendly CLI-not-found case. + // Everything else — auth, RPC, agent runtime errors — should bubble up as a real error + // so the user can actually see what went wrong instead of a misleading "install copilot" banner. + return ex is System.ComponentModel.Win32Exception w32 && + (w32.NativeErrorCode == 2 /* ERROR_FILE_NOT_FOUND */ || + w32.NativeErrorCode == 3 /* ERROR_PATH_NOT_FOUND */); } static void PrintCliError() @@ -335,3 +337,7 @@ static async ValueTask DisposeIfNeeded(SquadAgent agent) if (agent is IAsyncDisposable d) await d.DisposeAsync(); } + + + + diff --git a/test/Squad.Agents.AI.Tests/SquadAgentRoutingTests.cs b/test/Squad.Agents.AI.Tests/SquadAgentRoutingTests.cs index 5beff8142..c71d715fc 100644 --- a/test/Squad.Agents.AI.Tests/SquadAgentRoutingTests.cs +++ b/test/Squad.Agents.AI.Tests/SquadAgentRoutingTests.cs @@ -48,7 +48,7 @@ public void AddSquadAgent_UsesExplicitCwdForCopilotClientRouting() var clientOptions = GetCopilotClientOptions(agent); - Assert.Equal(@"C:\isolated-working-directory", GetRequiredProperty(clientOptions, "Cwd")); + Assert.Equal(@"C:\isolated-working-directory", GetRequiredProperty(clientOptions, "WorkingDirectory")); } [Fact] @@ -62,7 +62,7 @@ public void AddSquadAgent_DefaultsCopilotClientCwdToSquadFolderPath() var clientOptions = GetCopilotClientOptions(agent); - Assert.Equal(@"C:\squad-team-root", GetRequiredProperty(clientOptions, "Cwd")); + Assert.Equal(@"C:\squad-team-root", GetRequiredProperty(clientOptions, "WorkingDirectory")); } [Fact] @@ -80,7 +80,7 @@ public void AddSquadAgent_RoutesThroughCopilotClientOptionsNotAgentName() var inner = GetInnerAgent(agent); var sessionConfig = GetInnerSessionConfig(agent); var clientOptions = GetCopilotClientOptions(agent); - var cliArgs = GetRequiredProperty(clientOptions, "CliArgs"); + var cliArgs = GetConnectionArgs(clientOptions); var environment = GetRequiredProperty>(clientOptions, "Environment"); Assert.Equal("Custom Persona", GetRequiredProperty(inner, "Name")); @@ -112,7 +112,7 @@ public void AddSquadAgent_CopiesConnectionStringCliArgsToCopilotClientOptions() var provider = services.BuildServiceProvider(); var agent = provider.GetRequiredService(); var clientOptions = GetCopilotClientOptions(agent); - var cliArgs = GetRequiredProperty(clientOptions, "CliArgs"); + var cliArgs = GetConnectionArgs(clientOptions); Assert.Equal(new[] { "--yolo", "--model", "gpt-5" }, cliArgs); } @@ -153,6 +153,20 @@ private static object GetCopilotClientOptions(SquadAgent agent) return GetRequiredField(client, "_options"); } + /// + /// In SDK 1.0.0 the CLI args live inside CopilotClientOptions.Connection (a + /// RuntimeConnection — concrete type ChildProcessRuntimeConnection — exposing + /// .Path and .Args). Older tests asserted against a flat clientOptions.CliArgs + /// property; this helper hides the unwrap so each test stays readable. + /// + private static IList GetConnectionArgs(object clientOptions) + { + var connection = GetRequiredProperty(clientOptions, "Connection"); + var args = GetProperty>(connection, "Args"); + Assert.NotNull(args); + return args!; + } + private static object GetInnerAgent(SquadAgent agent) { // InnerAgent is protected on DelegatingAIAgent; search the declaring base type. diff --git a/test/Squad.Agents.AI.Tests/SquadAgentSessionConfigTests.cs b/test/Squad.Agents.AI.Tests/SquadAgentSessionConfigTests.cs index 22377f836..7d6f51707 100644 --- a/test/Squad.Agents.AI.Tests/SquadAgentSessionConfigTests.cs +++ b/test/Squad.Agents.AI.Tests/SquadAgentSessionConfigTests.cs @@ -1,5 +1,4 @@ using GitHub.Copilot; -using GitHub.Copilot.SDK; using Xunit; namespace Squad.Agents.AI.Tests; @@ -53,7 +52,7 @@ public void ConfigureSession_CanReplacePermissionHandler() var options = new SquadAgentOptions(); // Build a no-op handler so we just verify the property can be reassigned. // The exact PermissionRequestResult shape isn't relevant to this test. - PermissionRequestHandler customHandler = PermissionHandler.ApproveAll; + var customHandler = PermissionHandler.ApproveAll; options.ConfigureSession = config => config.OnPermissionRequest = customHandler; diff --git a/test/Squad.Agents.AI.Tests/SquadBYOKTests.cs b/test/Squad.Agents.AI.Tests/SquadBYOKTests.cs index 26ab0b102..c589a4542 100644 --- a/test/Squad.Agents.AI.Tests/SquadBYOKTests.cs +++ b/test/Squad.Agents.AI.Tests/SquadBYOKTests.cs @@ -1,4 +1,5 @@ using System.Reflection; +using GitHub.Copilot; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Xunit; @@ -47,7 +48,7 @@ public void ConfigureCopilotClient_CanSetEnvironmentOnCopilotClient() } [Fact] - public void ConfigureCopilotClient_RestoresCwdAfterDelegate() + public void ConfigureCopilotClient_RestoresWorkingDirectoryAfterDelegate() { var agent = CreateAgent(opts => { @@ -55,53 +56,42 @@ public void ConfigureCopilotClient_RestoresCwdAfterDelegate() opts.ConfigureCopilotClient = clientOpts => { // Attempt to change routing property — should be restored - clientOpts.Cwd = @"C:\hijacked-cwd"; + clientOpts.WorkingDirectory = @"C:\hijacked-cwd"; }; }); var clientOptions = GetCopilotClientOptions(agent); - var cwd = GetRequiredProperty(clientOptions, "Cwd"); + var workingDirectory = GetRequiredProperty(clientOptions, "WorkingDirectory"); - Assert.Equal(@"C:\original-cwd", cwd); + Assert.Equal(@"C:\original-cwd", workingDirectory); } [Fact] - public void ConfigureCopilotClient_RestoresCliPathAfterDelegate() + public void ConfigureCopilotClient_RestoresConnectionAfterDelegate() { + // SDK 1.0.0 collapsed CliPath/CliArgs into Connection (RuntimeConnection). + // The routing gate now snapshots and restores the Connection reference. var agent = CreateAgent(opts => { opts.CliPath = @"C:\original\copilot.exe"; - opts.ConfigureCopilotClient = clientOpts => - { - clientOpts.CliPath = @"C:\hijacked\evil.exe"; - }; - }); - - var clientOptions = GetCopilotClientOptions(agent); - var cliPath = GetProperty(clientOptions, "CliPath"); - - Assert.Equal(@"C:\original\copilot.exe", cliPath); - } - - [Fact] - public void ConfigureCopilotClient_RestoresCliArgsAfterDelegate() - { - var agent = CreateAgent(opts => - { opts.CliArgs.Add("--extension"); opts.CliArgs.Add("squad"); opts.ConfigureCopilotClient = clientOpts => { - clientOpts.CliArgs = new[] { "--hijacked" }; + clientOpts.Connection = RuntimeConnection.ForStdio(@"C:\hijacked\evil.exe", new List { "--hijacked" }); }; }); var clientOptions = GetCopilotClientOptions(agent); - var cliArgs = GetRequiredProperty(clientOptions, "CliArgs"); - - Assert.Contains("--extension", cliArgs); - Assert.Contains("squad", cliArgs); - Assert.DoesNotContain("--hijacked", cliArgs); + var connection = GetRequiredProperty(clientOptions, "Connection"); + var path = GetProperty(connection, "Path"); + var args = GetProperty>(connection, "Args"); + + Assert.Equal(@"C:\original\copilot.exe", path); + Assert.NotNull(args); + Assert.Contains("--extension", args); + Assert.Contains("squad", args); + Assert.DoesNotContain("--hijacked", args); } [Fact]