Skip to content

Commit f2f9373

Browse files
SteveSandersonMSSteveSandersonMS
committed
Improve discoverability of permission handler being required
1 parent f0909a7 commit f2f9373

21 files changed

Lines changed: 415 additions & 251 deletions

‎dotnet/src/Client.cs‎

Lines changed: 74 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ namespace GitHub.Copilot.SDK;
3838
/// await using var client = new CopilotClient();
3939
///
4040
/// // Create a session
41-
/// await using var session = await client.CreateSessionAsync(new SessionConfig { Model = "gpt-4" });
41+
/// await using var session = await client.CreateSessionAsync(new() { OnPermissionRequest = PermissionHandler.ApproveAll, Model = "gpt-4" });
4242
///
4343
/// // Handle events
4444
/// using var subscription = session.On(evt =>
@@ -340,10 +340,9 @@ private async Task CleanupConnectionAsync(List<Exception>? errors)
340340
/// <summary>
341341
/// Creates a new Copilot session with the specified configuration.
342342
/// </summary>
343-
/// <param name="config">Configuration for the session. If null, default settings are used.</param>
343+
/// <param name="config">Configuration for the session, including the required <see cref="SessionConfig.OnPermissionRequest"/> handler.</param>
344344
/// <param name="cancellationToken">A <see cref="CancellationToken"/> that can be used to cancel the operation.</param>
345345
/// <returns>A task that resolves to provide the <see cref="CopilotSession"/>.</returns>
346-
/// <exception cref="InvalidOperationException">Thrown when the client is not connected and AutoStart is disabled, or when a session with the same ID already exists.</exception>
347346
/// <remarks>
348347
/// Sessions maintain conversation state, handle events, and manage tool execution.
349348
/// If the client is not connected and <see cref="CopilotClientOptions.AutoStart"/> is enabled (default),
@@ -352,21 +351,29 @@ private async Task CleanupConnectionAsync(List<Exception>? errors)
352351
/// <example>
353352
/// <code>
354353
/// // Basic session
355-
/// var session = await client.CreateSessionAsync();
354+
/// var session = await client.CreateSessionAsync(new() { OnPermissionRequest = PermissionHandler.ApproveAll });
356355
///
357356
/// // Session with model and tools
358-
/// var session = await client.CreateSessionAsync(new SessionConfig
357+
/// var session = await client.CreateSessionAsync(new()
359358
/// {
359+
/// OnPermissionRequest = PermissionHandler.ApproveAll,
360360
/// Model = "gpt-4",
361361
/// Tools = [AIFunctionFactory.Create(MyToolMethod)]
362362
/// });
363363
/// </code>
364364
/// </example>
365-
public async Task<CopilotSession> CreateSessionAsync(SessionConfig? config = null, CancellationToken cancellationToken = default)
365+
public async Task<CopilotSession> CreateSessionAsync(SessionConfig config, CancellationToken cancellationToken = default)
366366
{
367+
if (config.OnPermissionRequest == null)
368+
{
369+
throw new ArgumentException(
370+
"An OnPermissionRequest handler is required when creating a session. " +
371+
"For example, to allow all permissions, use CreateSessionAsync(new() { OnPermissionRequest = PermissionHandler.ApproveAll });");
372+
}
373+
367374
var connection = await EnsureConnectedAsync(cancellationToken);
368375

369-
var hasHooks = config?.Hooks != null && (
376+
var hasHooks = config.Hooks != null && (
370377
config.Hooks.OnPreToolUse != null ||
371378
config.Hooks.OnPostToolUse != null ||
372379
config.Hooks.OnUserPromptSubmitted != null ||
@@ -375,42 +382,39 @@ public async Task<CopilotSession> CreateSessionAsync(SessionConfig? config = nul
375382
config.Hooks.OnErrorOccurred != null);
376383

377384
var request = new CreateSessionRequest(
378-
config?.Model,
379-
config?.SessionId,
380-
config?.ClientName,
381-
config?.ReasoningEffort,
382-
config?.Tools?.Select(ToolDefinition.FromAIFunction).ToList(),
383-
config?.SystemMessage,
384-
config?.AvailableTools,
385-
config?.ExcludedTools,
386-
config?.Provider,
385+
config.Model,
386+
config.SessionId,
387+
config.ClientName,
388+
config.ReasoningEffort,
389+
config.Tools?.Select(ToolDefinition.FromAIFunction).ToList(),
390+
config.SystemMessage,
391+
config.AvailableTools,
392+
config.ExcludedTools,
393+
config.Provider,
387394
(bool?)true,
388-
config?.OnUserInputRequest != null ? true : null,
395+
config.OnUserInputRequest != null ? true : null,
389396
hasHooks ? true : null,
390-
config?.WorkingDirectory,
391-
config?.Streaming == true ? true : null,
392-
config?.McpServers,
397+
config.WorkingDirectory,
398+
config.Streaming == true ? true : null,
399+
config.McpServers,
393400
"direct",
394-
config?.CustomAgents,
395-
config?.ConfigDir,
396-
config?.SkillDirectories,
397-
config?.DisabledSkills,
398-
config?.InfiniteSessions);
401+
config.CustomAgents,
402+
config.ConfigDir,
403+
config.SkillDirectories,
404+
config.DisabledSkills,
405+
config.InfiniteSessions);
399406

400407
var response = await InvokeRpcAsync<CreateSessionResponse>(
401408
connection.Rpc, "session.create", [request], cancellationToken);
402409

403410
var session = new CopilotSession(response.SessionId, connection.Rpc, response.WorkspacePath);
404-
session.RegisterTools(config?.Tools ?? []);
405-
if (config?.OnPermissionRequest != null)
406-
{
407-
session.RegisterPermissionHandler(config.OnPermissionRequest);
408-
}
409-
if (config?.OnUserInputRequest != null)
411+
session.RegisterTools(config.Tools ?? []);
412+
session.RegisterPermissionHandler(config.OnPermissionRequest);
413+
if (config.OnUserInputRequest != null)
410414
{
411415
session.RegisterUserInputHandler(config.OnUserInputRequest);
412416
}
413-
if (config?.Hooks != null)
417+
if (config.Hooks != null)
414418
{
415419
session.RegisterHooks(config.Hooks);
416420
}
@@ -427,9 +431,10 @@ public async Task<CopilotSession> CreateSessionAsync(SessionConfig? config = nul
427431
/// Resumes an existing Copilot session with the specified configuration.
428432
/// </summary>
429433
/// <param name="sessionId">The ID of the session to resume.</param>
430-
/// <param name="config">Configuration for the resumed session. If null, default settings are used.</param>
434+
/// <param name="config">Configuration for the resumed session, including the required <see cref="ResumeSessionConfig.OnPermissionRequest"/> handler.</param>
431435
/// <param name="cancellationToken">A <see cref="CancellationToken"/> that can be used to cancel the operation.</param>
432436
/// <returns>A task that resolves to provide the <see cref="CopilotSession"/>.</returns>
437+
/// <exception cref="ArgumentException">Thrown when <see cref="ResumeSessionConfig.OnPermissionRequest"/> is not set.</exception>
433438
/// <exception cref="InvalidOperationException">Thrown when the session does not exist or the client is not connected.</exception>
434439
/// <remarks>
435440
/// This allows you to continue a previous conversation, maintaining all conversation history.
@@ -438,20 +443,28 @@ public async Task<CopilotSession> CreateSessionAsync(SessionConfig? config = nul
438443
/// <example>
439444
/// <code>
440445
/// // Resume a previous session
441-
/// var session = await client.ResumeSessionAsync("session-123");
446+
/// var session = await client.ResumeSessionAsync("session-123", new() { OnPermissionRequest = PermissionHandler.ApproveAll });
442447
///
443448
/// // Resume with new tools
444-
/// var session = await client.ResumeSessionAsync("session-123", new ResumeSessionConfig
449+
/// var session = await client.ResumeSessionAsync("session-123", new()
445450
/// {
451+
/// OnPermissionRequest = PermissionHandler.ApproveAll,
446452
/// Tools = [AIFunctionFactory.Create(MyNewToolMethod)]
447453
/// });
448454
/// </code>
449455
/// </example>
450-
public async Task<CopilotSession> ResumeSessionAsync(string sessionId, ResumeSessionConfig? config = null, CancellationToken cancellationToken = default)
456+
public async Task<CopilotSession> ResumeSessionAsync(string sessionId, ResumeSessionConfig config, CancellationToken cancellationToken = default)
451457
{
458+
if (config.OnPermissionRequest == null)
459+
{
460+
throw new ArgumentException(
461+
"An OnPermissionRequest handler is required when resuming a session. " +
462+
"For example, to allow all permissions, use new() { OnPermissionRequest = PermissionHandler.ApproveAll }.");
463+
}
464+
452465
var connection = await EnsureConnectedAsync(cancellationToken);
453466

454-
var hasHooks = config?.Hooks != null && (
467+
var hasHooks = config.Hooks != null && (
455468
config.Hooks.OnPreToolUse != null ||
456469
config.Hooks.OnPostToolUse != null ||
457470
config.Hooks.OnUserPromptSubmitted != null ||
@@ -461,42 +474,39 @@ public async Task<CopilotSession> ResumeSessionAsync(string sessionId, ResumeSes
461474

462475
var request = new ResumeSessionRequest(
463476
sessionId,
464-
config?.ClientName,
465-
config?.Model,
466-
config?.ReasoningEffort,
467-
config?.Tools?.Select(ToolDefinition.FromAIFunction).ToList(),
468-
config?.SystemMessage,
469-
config?.AvailableTools,
470-
config?.ExcludedTools,
471-
config?.Provider,
477+
config.ClientName,
478+
config.Model,
479+
config.ReasoningEffort,
480+
config.Tools?.Select(ToolDefinition.FromAIFunction).ToList(),
481+
config.SystemMessage,
482+
config.AvailableTools,
483+
config.ExcludedTools,
484+
config.Provider,
472485
(bool?)true,
473-
config?.OnUserInputRequest != null ? true : null,
486+
config.OnUserInputRequest != null ? true : null,
474487
hasHooks ? true : null,
475-
config?.WorkingDirectory,
476-
config?.ConfigDir,
477-
config?.DisableResume == true ? true : null,
478-
config?.Streaming == true ? true : null,
479-
config?.McpServers,
488+
config.WorkingDirectory,
489+
config.ConfigDir,
490+
config.DisableResume == true ? true : null,
491+
config.Streaming == true ? true : null,
492+
config.McpServers,
480493
"direct",
481-
config?.CustomAgents,
482-
config?.SkillDirectories,
483-
config?.DisabledSkills,
484-
config?.InfiniteSessions);
494+
config.CustomAgents,
495+
config.SkillDirectories,
496+
config.DisabledSkills,
497+
config.InfiniteSessions);
485498

486499
var response = await InvokeRpcAsync<ResumeSessionResponse>(
487500
connection.Rpc, "session.resume", [request], cancellationToken);
488501

489502
var session = new CopilotSession(response.SessionId, connection.Rpc, response.WorkspacePath);
490-
session.RegisterTools(config?.Tools ?? []);
491-
if (config?.OnPermissionRequest != null)
492-
{
493-
session.RegisterPermissionHandler(config.OnPermissionRequest);
494-
}
495-
if (config?.OnUserInputRequest != null)
503+
session.RegisterTools(config.Tools ?? []);
504+
session.RegisterPermissionHandler(config.OnPermissionRequest);
505+
if (config.OnUserInputRequest != null)
496506
{
497507
session.RegisterUserInputHandler(config.OnUserInputRequest);
498508
}
499-
if (config?.Hooks != null)
509+
if (config.Hooks != null)
500510
{
501511
session.RegisterHooks(config.Hooks);
502512
}
@@ -516,7 +526,7 @@ public async Task<CopilotSession> ResumeSessionAsync(string sessionId, ResumeSes
516526
/// <code>
517527
/// if (client.State == ConnectionState.Connected)
518528
/// {
519-
/// var session = await client.CreateSessionAsync();
529+
/// var session = await client.CreateSessionAsync(new() { OnPermissionRequest = PermissionHandler.ApproveAll });
520530
/// }
521531
/// </code>
522532
/// </example>
@@ -630,7 +640,7 @@ public async Task<List<ModelInfo>> ListModelsAsync(CancellationToken cancellatio
630640
/// var lastId = await client.GetLastSessionIdAsync();
631641
/// if (lastId != null)
632642
/// {
633-
/// var session = await client.ResumeSessionAsync(lastId);
643+
/// var session = await client.ResumeSessionAsync(lastId, new() { OnPermissionRequest = PermissionHandler.ApproveAll });
634644
/// }
635645
/// </code>
636646
/// </example>

‎dotnet/src/Session.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ namespace GitHub.Copilot.SDK;
2727
/// </remarks>
2828
/// <example>
2929
/// <code>
30-
/// await using var session = await client.CreateSessionAsync(new SessionConfig { Model = "gpt-4" });
30+
/// await using var session = await client.CreateSessionAsync(new() { OnPermissionRequest = PermissionHandler.ApproveAll, Model = "gpt-4" });
3131
///
3232
/// // Subscribe to events
3333
/// using var subscription = session.On(evt =>
@@ -557,10 +557,10 @@ await InvokeRpcAsync<object>(
557557
/// <example>
558558
/// <code>
559559
/// // Using 'await using' for automatic disposal
560-
/// await using var session = await client.CreateSessionAsync();
560+
/// await using var session = await client.CreateSessionAsync(new() { OnPermissionRequest = PermissionHandler.ApproveAll });
561561
///
562562
/// // Or manually dispose
563-
/// var session2 = await client.CreateSessionAsync();
563+
/// var session2 = await client.CreateSessionAsync(new() { OnPermissionRequest = PermissionHandler.ApproveAll });
564564
/// // ... use the session ...
565565
/// await session2.DisposeAsync();
566566
/// </code>

‎dotnet/test/AskUserTests.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ public async Task Should_Invoke_User_Input_Handler_When_Model_Uses_Ask_User_Tool
1515
{
1616
var userInputRequests = new List<UserInputRequest>();
1717
CopilotSession? session = null;
18-
session = await Client.CreateSessionAsync(new SessionConfig
18+
session = await CreateSessionAsync(new SessionConfig
1919
{
2020
OnUserInputRequest = (request, invocation) =>
2121
{
@@ -49,7 +49,7 @@ public async Task Should_Receive_Choices_In_User_Input_Request()
4949
{
5050
var userInputRequests = new List<UserInputRequest>();
5151

52-
var session = await Client.CreateSessionAsync(new SessionConfig
52+
var session = await CreateSessionAsync(new SessionConfig
5353
{
5454
OnUserInputRequest = (request, invocation) =>
5555
{
@@ -82,7 +82,7 @@ public async Task Should_Handle_Freeform_User_Input_Response()
8282
var userInputRequests = new List<UserInputRequest>();
8383
var freeformAnswer = "This is my custom freeform answer that was not in the choices";
8484

85-
var session = await Client.CreateSessionAsync(new SessionConfig
85+
var session = await CreateSessionAsync(new SessionConfig
8686
{
8787
OnUserInputRequest = (request, invocation) =>
8888
{

‎dotnet/test/ClientTests.cs‎

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public async Task Should_Force_Stop_Without_Cleanup()
5959
{
6060
using var client = new CopilotClient(new CopilotClientOptions());
6161

62-
await client.CreateSessionAsync();
62+
await client.CreateSessionAsync(new SessionConfig { OnPermissionRequest = PermissionHandler.ApproveAll });
6363
await client.ForceStopAsync();
6464

6565
Assert.Equal(ConnectionState.Disconnected, client.State);
@@ -220,7 +220,7 @@ public void Should_Throw_When_UseLoggedInUser_Used_With_CliUrl()
220220
public async Task Should_Not_Throw_When_Disposing_Session_After_Stopping_Client()
221221
{
222222
await using var client = new CopilotClient(new CopilotClientOptions());
223-
await using var session = await client.CreateSessionAsync();
223+
await using var session = await client.CreateSessionAsync(new SessionConfig { OnPermissionRequest = PermissionHandler.ApproveAll });
224224

225225
await client.StopAsync();
226226
}
@@ -247,12 +247,40 @@ public async Task Should_Report_Error_With_Stderr_When_CLI_Fails_To_Start()
247247
// Verify subsequent calls also fail (don't hang)
248248
var ex2 = await Assert.ThrowsAnyAsync<Exception>(async () =>
249249
{
250-
var session = await client.CreateSessionAsync();
250+
var session = await client.CreateSessionAsync(new SessionConfig { OnPermissionRequest = PermissionHandler.ApproveAll });
251251
await session.SendAsync(new MessageOptions { Prompt = "test" });
252252
});
253253
Assert.Contains("exited", ex2.Message, StringComparison.OrdinalIgnoreCase);
254254

255255
// Cleanup - ForceStop should handle the disconnected state gracefully
256256
try { await client.ForceStopAsync(); } catch (Exception) { /* Expected */ }
257257
}
258+
259+
[Fact]
260+
public async Task Should_Throw_When_CreateSession_Called_Without_PermissionHandler()
261+
{
262+
using var client = new CopilotClient(new CopilotClientOptions());
263+
264+
var ex = await Assert.ThrowsAsync<ArgumentException>(async () =>
265+
{
266+
await client.CreateSessionAsync(new SessionConfig());
267+
});
268+
269+
Assert.Contains("OnPermissionRequest", ex.Message);
270+
Assert.Contains("is required", ex.Message);
271+
}
272+
273+
[Fact]
274+
public async Task Should_Throw_When_ResumeSession_Called_Without_PermissionHandler()
275+
{
276+
using var client = new CopilotClient(new CopilotClientOptions());
277+
278+
var ex = await Assert.ThrowsAsync<ArgumentException>(async () =>
279+
{
280+
await client.ResumeSessionAsync("some-session-id", new ResumeSessionConfig());
281+
});
282+
283+
Assert.Contains("OnPermissionRequest", ex.Message);
284+
Assert.Contains("is required", ex.Message);
285+
}
258286
}

‎dotnet/test/CompactionTests.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ public class CompactionTests(E2ETestFixture fixture, ITestOutputHelper output) :
1515
public async Task Should_Trigger_Compaction_With_Low_Threshold_And_Emit_Events()
1616
{
1717
// Create session with very low compaction thresholds to trigger compaction quickly
18-
var session = await Client.CreateSessionAsync(new SessionConfig
18+
var session = await CreateSessionAsync(new SessionConfig
1919
{
2020
InfiniteSessions = new InfiniteSessionConfig
2121
{
@@ -84,7 +84,7 @@ await session.SendAndWaitAsync(new MessageOptions
8484
[Fact]
8585
public async Task Should_Not_Emit_Compaction_Events_When_Infinite_Sessions_Disabled()
8686
{
87-
var session = await Client.CreateSessionAsync(new SessionConfig
87+
var session = await CreateSessionAsync(new SessionConfig
8888
{
8989
InfiniteSessions = new InfiniteSessionConfig
9090
{

0 commit comments

Comments
 (0)