Skip to content

Commit f35dd0d

Browse files
committed
refactor(Squad.Agents.AI): adopt DelegatingAIAgent base + MAF ctor pattern (PR github#1207 r2)
Addresses westey-m's review feedback: - Extend Microsoft.Agents.AI.DelegatingAIAgent — drops ~70 lines of manual Core* overrides; pass-through is provided by the base class. - Adopt MAF constructor pattern: \(string squadFolderPath, SquadAgentOptions? options = null, ILoggerFactory? loggerFactory = null)\. Required settings on the constructor, options optional, ILoggerFactory stays on ctor for DI injection. - Routing-guard, IAsyncDisposable, and security posture preserved. - Add Squad.Agents.AI.slnx solution for easy IDE open. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent de05707 commit f35dd0d

5 files changed

Lines changed: 110 additions & 113 deletions

File tree

‎Squad.Agents.AI.slnx‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<Solution>
2+
<Folder Name="/src/">
3+
<Project Path="src/Squad.Agents.AI/Squad.Agents.AI.csproj" />
4+
</Folder>
5+
<Folder Name="/test/">
6+
<Project Path="test/Squad.Agents.AI.Tests/Squad.Agents.AI.Tests.csproj" />
7+
</Folder>
8+
<Folder Name="/samples/">
9+
<Project Path="src/Squad.Agents.AI/samples/Squad.Agents.AI.Sample/Squad.Agents.AI.Sample.csproj" />
10+
</Folder>
11+
</Solution>

‎src/Squad.Agents.AI/SquadAgent.cs‎

Lines changed: 70 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
using GitHub.Copilot.SDK;
22
using Microsoft.Agents.AI;
3-
using System.Text.Json;
43
using Microsoft.Extensions.Logging;
54
using Microsoft.Extensions.Options;
65

76
namespace Squad.Agents.AI;
87

98
/// <summary>
109
/// Microsoft Agent Framework agent that delegates to a GitHub Copilot SDK client configured for a Squad team root.
10+
/// Extends <see cref="DelegatingAIAgent"/> — Core* pass-through overrides are provided by the base class.
1111
/// </summary>
1212
/// <remarks>
1313
/// <para>
@@ -16,53 +16,94 @@ namespace Squad.Agents.AI;
1616
/// </para>
1717
/// <example>
1818
/// <code>
19-
/// var agent = new SquadAgent(new SquadAgentOptions { SquadFolderPath = @"C:\repo" });
19+
/// var agent = new SquadAgent(@"C:\repo");
2020
/// var response = await agent.RunAsync("Summarize the team.");
2121
/// </code>
2222
/// </example>
2323
/// </remarks>
24-
public sealed class SquadAgent : AIAgent, IAsyncDisposable
24+
public sealed class SquadAgent : DelegatingAIAgent, IAsyncDisposable
2525
{
2626
private readonly CopilotClient _copilotClient;
27-
private readonly AIAgent _inner;
2827
private readonly ILogger? _logger;
2928
private readonly bool _ownsClient;
3029
private readonly SquadAgentOptions _options;
3130

31+
// State-bag to thread pre-base-ctor state through chain constructors.
32+
// DelegatingAIAgent requires the inner AIAgent at base() call time, so we
33+
// build everything via static factory methods before invoking base().
34+
private readonly record struct SquadAgentState(
35+
AIAgent Inner,
36+
CopilotClient CopilotClient,
37+
ILogger? Logger,
38+
bool OwnsClient,
39+
SquadAgentOptions Options);
40+
3241
/// <summary>
33-
/// Initializes a new <see cref="SquadAgent"/> from fully-resolved options and owns the created Copilot client.
42+
/// Initializes a new <see cref="SquadAgent"/> with the Squad team root as the primary required parameter.
3443
/// </summary>
35-
/// <param name="options">Options that describe the Squad team root, CLI process, authentication, logging, and instructions.</param>
36-
/// <param name="loggerFactory">Optional logger factory used for wrapper logs and SDK trace logging when enabled.</param>
37-
public SquadAgent(SquadAgentOptions options, ILoggerFactory? loggerFactory = null)
38-
: this(CreateCopilotClient(options, loggerFactory), options, loggerFactory, ownsClient: true)
44+
/// <param name="squadFolderPath">Path to the initialized Squad team root. Required.</param>
45+
/// <param name="options">
46+
/// Optional options. When <see langword="null"/> a default <see cref="SquadAgentOptions"/> is used.
47+
/// <see cref="SquadAgentOptions.SquadFolderPath"/> is overwritten by <paramref name="squadFolderPath"/>
48+
/// so there is exactly one source of truth for the team root.
49+
/// </param>
50+
/// <param name="loggerFactory">Optional logger factory; inject via DI or supply directly.</param>
51+
public SquadAgent(string squadFolderPath, SquadAgentOptions? options = null, ILoggerFactory? loggerFactory = null)
52+
: this(BuildState(squadFolderPath, options, loggerFactory))
3953
{
4054
}
4155

4256
/// <summary>
43-
/// Initializes a new <see cref="SquadAgent"/> from the options pattern.
57+
/// Initializes a new <see cref="SquadAgent"/> from the options pattern (DI-friendly).
4458
/// </summary>
45-
/// <param name="options">Options wrapper containing the resolved <see cref="SquadAgentOptions"/>.</param>
46-
/// <param name="loggerFactory">Optional logger factory used for wrapper logs and SDK trace logging when enabled.</param>
59+
/// <param name="options">
60+
/// Options wrapper. <see cref="SquadAgentOptions.SquadFolderPath"/> must be set via a configure callback
61+
/// or connection string before the agent is resolved from DI.
62+
/// </param>
63+
/// <param name="loggerFactory">Optional logger factory.</param>
64+
/// <exception cref="ArgumentException">
65+
/// Thrown when <see cref="SquadAgentOptions.SquadFolderPath"/> is <see langword="null"/> or whitespace.
66+
/// </exception>
4767
public SquadAgent(IOptions<SquadAgentOptions> options, ILoggerFactory? loggerFactory = null)
48-
: this(options.Value, loggerFactory)
68+
: this(BuildStateFromOptions(options, loggerFactory))
4969
{
5070
}
5171

52-
private SquadAgent(CopilotClient copilotClient, SquadAgentOptions options, ILoggerFactory? loggerFactory, bool ownsClient)
72+
private SquadAgent(SquadAgentState state)
73+
: base(state.Inner)
74+
{
75+
_copilotClient = state.CopilotClient;
76+
_logger = state.Logger;
77+
_ownsClient = state.OwnsClient;
78+
_options = state.Options;
79+
_logger?.LogInformation("SquadAgent initialized with name '{AgentName}', team root '{TeamRoot}'",
80+
state.Options.AgentName, state.Options.SquadFolderPath);
81+
}
82+
83+
private static SquadAgentState BuildState(string squadFolderPath, SquadAgentOptions? options, ILoggerFactory? lf)
5384
{
54-
_copilotClient = copilotClient ?? throw new ArgumentNullException(nameof(copilotClient));
55-
_logger = loggerFactory?.CreateLogger<SquadAgent>();
56-
_ownsClient = ownsClient;
57-
_options = options;
85+
ArgumentException.ThrowIfNullOrWhiteSpace(squadFolderPath);
86+
var resolved = options ?? new SquadAgentOptions();
87+
resolved.SquadFolderPath = squadFolderPath; // ctor param wins
88+
return BuildStateInternal(resolved, lf);
89+
}
5890

59-
// Create the inner AIAgent via AsAIAgent extension — NO CAST
60-
_inner = _copilotClient.AsAIAgent(
61-
instructions: options.Instructions,
62-
name: options.AgentName ?? "Squad");
91+
private static SquadAgentState BuildStateFromOptions(IOptions<SquadAgentOptions> options, ILoggerFactory? lf)
92+
{
93+
ArgumentNullException.ThrowIfNull(options);
94+
var opts = options.Value ?? throw new ArgumentException("Options.Value cannot be null.", nameof(options));
95+
if (string.IsNullOrWhiteSpace(opts.SquadFolderPath))
96+
throw new ArgumentException(
97+
"SquadAgentOptions.SquadFolderPath must be set (via configure callback or connection string).",
98+
nameof(options));
99+
return BuildStateInternal(opts, lf);
100+
}
63101

64-
_logger?.LogInformation("SquadAgent initialized with name '{AgentName}', team root '{TeamRoot}'",
65-
options.AgentName, options.SquadFolderPath);
102+
private static SquadAgentState BuildStateInternal(SquadAgentOptions options, ILoggerFactory? lf)
103+
{
104+
var client = CreateCopilotClient(options, lf);
105+
var inner = client.AsAIAgent(instructions: options.Instructions, name: options.AgentName ?? "Squad");
106+
return new SquadAgentState(inner, client, lf?.CreateLogger<SquadAgent>(), true, options);
66107
}
67108

68109
// ── Extensibility seam ──────────────────────────────────────────────
@@ -183,87 +224,14 @@ private static CopilotClient CreateCopilotClient(SquadAgentOptions options, ILog
183224

184225
/// <summary>
185226
/// Gets the display name exposed by this agent.
227+
/// Prefers <see cref="SquadAgentOptions.AgentName"/> over the inner agent's name.
186228
/// </summary>
187-
public override string? Name => _options.AgentName ?? _inner.Name;
229+
public override string? Name => _options.AgentName ?? base.Name;
188230

189231
/// <summary>
190-
/// Gets the agent description exposed by the inner Copilot-backed agent, or a Squad fallback description.
232+
/// Gets the agent description, falling back to a Squad-specific default when the inner agent provides none.
191233
/// </summary>
192-
public override string? Description => _inner.Description ?? "Squad multi-agent CLI participant";
193-
194-
/// <summary>
195-
/// Creates a new MAF session by delegating to the inner agent.
196-
/// </summary>
197-
/// <param name="cancellationToken">Token that cancels session creation.</param>
198-
/// <returns>A newly-created agent session.</returns>
199-
protected override ValueTask<AgentSession> CreateSessionCoreAsync(CancellationToken cancellationToken = default)
200-
{
201-
return _inner.CreateSessionAsync(cancellationToken);
202-
}
203-
204-
/// <summary>
205-
/// Runs a non-streaming agent turn by delegating to the inner agent.
206-
/// </summary>
207-
/// <param name="messages">Messages to send to the agent.</param>
208-
/// <param name="session">Optional session for multi-turn continuity.</param>
209-
/// <param name="options">Optional run options.</param>
210-
/// <param name="cancellationToken">Token that cancels the run.</param>
211-
/// <returns>The complete agent response.</returns>
212-
protected override Task<AgentResponse> RunCoreAsync(
213-
IEnumerable<Microsoft.Extensions.AI.ChatMessage> messages,
214-
AgentSession? session = null,
215-
AgentRunOptions? options = null,
216-
CancellationToken cancellationToken = default)
217-
{
218-
return _inner.RunAsync(messages, session, options, cancellationToken);
219-
}
220-
221-
/// <summary>
222-
/// Runs a streaming agent turn by delegating to the inner agent.
223-
/// </summary>
224-
/// <param name="messages">Messages to send to the agent.</param>
225-
/// <param name="session">Optional session for multi-turn continuity.</param>
226-
/// <param name="options">Optional run options.</param>
227-
/// <param name="cancellationToken">Token that cancels the run.</param>
228-
/// <returns>An async stream of response updates.</returns>
229-
protected override IAsyncEnumerable<AgentResponseUpdate> RunCoreStreamingAsync(
230-
IEnumerable<Microsoft.Extensions.AI.ChatMessage> messages,
231-
AgentSession? session = null,
232-
AgentRunOptions? options = null,
233-
CancellationToken cancellationToken = default)
234-
{
235-
return _inner.RunStreamingAsync(messages, session, options, cancellationToken);
236-
}
237-
238-
/// <summary>
239-
/// Serializes a session by delegating to the inner agent.
240-
/// </summary>
241-
/// <param name="session">Session to serialize.</param>
242-
/// <param name="jsonSerializerOptions">Optional JSON serializer settings.</param>
243-
/// <param name="cancellationToken">Token that cancels serialization.</param>
244-
/// <returns>The serialized session state.</returns>
245-
protected override ValueTask<JsonElement> SerializeSessionCoreAsync(
246-
AgentSession session,
247-
JsonSerializerOptions? jsonSerializerOptions = null,
248-
CancellationToken cancellationToken = default)
249-
{
250-
return _inner.SerializeSessionAsync(session, jsonSerializerOptions, cancellationToken);
251-
}
252-
253-
/// <summary>
254-
/// Deserializes a session by delegating to the inner agent.
255-
/// </summary>
256-
/// <param name="sessionState">Serialized session state.</param>
257-
/// <param name="jsonSerializerOptions">Optional JSON serializer settings.</param>
258-
/// <param name="cancellationToken">Token that cancels deserialization.</param>
259-
/// <returns>The deserialized session.</returns>
260-
protected override ValueTask<AgentSession> DeserializeSessionCoreAsync(
261-
JsonElement sessionState,
262-
JsonSerializerOptions? jsonSerializerOptions = null,
263-
CancellationToken cancellationToken = default)
264-
{
265-
return _inner.DeserializeSessionAsync(sessionState, jsonSerializerOptions, cancellationToken);
266-
}
234+
public override string? Description => base.Description ?? "Squad multi-agent CLI participant";
267235

268236
/// <summary>
269237
/// Disposes the owned Copilot client and disposable inner agent resources.
@@ -272,14 +240,10 @@ protected override ValueTask<AgentSession> DeserializeSessionCoreAsync(
272240
public async ValueTask DisposeAsync()
273241
{
274242
if (_ownsClient)
275-
{
276243
await _copilotClient.DisposeAsync().ConfigureAwait(false);
277-
}
278244

279-
if (_inner is IAsyncDisposable innerDisposable)
280-
{
245+
if (InnerAgent is IAsyncDisposable innerDisposable)
281246
await innerDisposable.DisposeAsync().ConfigureAwait(false);
282-
}
283247

284248
_logger?.LogDebug("SquadAgent disposed");
285249
}

‎src/Squad.Agents.AI/SquadServiceCollectionExtensions.cs‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,12 @@ private static IServiceCollection AddSquadAgentCore(
176176
sp =>
177177
{
178178
var options = sp.GetRequiredService<IOptionsMonitor<SquadAgentOptions>>().Get(optionsName);
179-
return ActivatorUtilities.CreateInstance<SquadAgent>(sp, options);
179+
var loggerFactory = sp.GetService<ILoggerFactory>();
180+
var folderPath = options.SquadFolderPath;
181+
if (string.IsNullOrWhiteSpace(folderPath))
182+
throw new InvalidOperationException(
183+
"SquadAgentOptions.SquadFolderPath must be configured (via configure callback or connection string) before resolving SquadAgent.");
184+
return new SquadAgent(folderPath, options, loggerFactory);
180185
},
181186
lifetime));
182187

@@ -214,7 +219,12 @@ private static IServiceCollection AddKeyedSquadAgentCore(
214219
(sp, _) =>
215220
{
216221
var options = sp.GetRequiredService<IOptionsMonitor<SquadAgentOptions>>().Get(optionsName);
217-
return ActivatorUtilities.CreateInstance<SquadAgent>(sp, options);
222+
var loggerFactory = sp.GetService<ILoggerFactory>();
223+
var folderPath = options.SquadFolderPath;
224+
if (string.IsNullOrWhiteSpace(folderPath))
225+
throw new InvalidOperationException(
226+
"SquadAgentOptions.SquadFolderPath must be configured (via configure callback or connection string) before resolving SquadAgent.");
227+
return new SquadAgent(folderPath, options, loggerFactory);
218228
},
219229
lifetime));
220230

‎test/Squad.Agents.AI.Tests/SquadAgentRoutingTests.cs‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ public void AddSquadAgent_RoutesThroughCopilotClientOptionsNotAgentName()
7777
options.Environment["SQUAD_ROUTE"] = "enabled";
7878
});
7979

80-
var inner = GetRequiredField<object>(agent, "_inner");
80+
var inner = GetInnerAgent(agent);
8181
var sessionConfig = GetInnerSessionConfig(agent);
8282
var clientOptions = GetCopilotClientOptions(agent);
8383
var cliArgs = GetRequiredProperty<string[]>(clientOptions, "CliArgs");
@@ -153,9 +153,21 @@ private static object GetCopilotClientOptions(SquadAgent agent)
153153
return GetRequiredField<object>(client, "_options");
154154
}
155155

156+
private static object GetInnerAgent(SquadAgent agent)
157+
{
158+
// InnerAgent is protected on DelegatingAIAgent; search the declaring base type.
159+
var prop = typeof(DelegatingAIAgent).GetProperty(
160+
"InnerAgent",
161+
BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic);
162+
Assert.NotNull(prop);
163+
var value = prop.GetValue(agent);
164+
Assert.NotNull(value);
165+
return value!;
166+
}
167+
156168
private static object GetInnerSessionConfig(SquadAgent agent)
157169
{
158-
var inner = GetRequiredField<object>(agent, "_inner");
170+
var inner = GetInnerAgent(agent);
159171
return GetRequiredField<object>(inner, "_sessionConfig");
160172
}
161173

‎test/Squad.Agents.AI.Tests/SquadServiceCollectionExtensionsTests.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ namespace Squad.Agents.AI.Tests;
99
public class SquadServiceCollectionExtensionsTests
1010
{
1111
[Fact]
12-
public void AddSquadAgent_WithoutConfig_RegistersAIAgent()
12+
public void AddSquadAgent_WithoutConfig_ThrowsOnResolution()
1313
{
1414
var services = new ServiceCollection();
1515
var configuration = new ConfigurationBuilder().Build();
@@ -18,9 +18,9 @@ public void AddSquadAgent_WithoutConfig_RegistersAIAgent()
1818
services.AddSquadAgent();
1919

2020
var provider = services.BuildServiceProvider();
21-
var agent = provider.GetService<SquadAgent>();
2221

23-
Assert.NotNull(agent);
22+
// SquadFolderPath not set → InvalidOperationException on resolution (MAF ctor pattern).
23+
Assert.Throws<InvalidOperationException>(() => provider.GetRequiredService<SquadAgent>());
2424
}
2525

2626
[Fact]

0 commit comments

Comments
 (0)