Skip to content

Commit a95b69a

Browse files
SteveSandersonMSCopilot
SteveSandersonMS
and
Copilot
committed
Replace hand-written PermissionRequestResult with PermissionDecision (.NET, Go)
Python (#1376) drove out its own hand-written ``PermissionRequestResult`` wrapper in favour of the generated discriminated ``PermissionDecision`` union plus a small ``PermissionNoResult`` sentinel. This commit lands the same refactor for the .NET and Go SDKs. **.NET** The old ``PermissionRequestResult`` struct (just ``Kind`` + optional ``Feedback``) collapsed every decision variant into a flat string-tagged DTO, losing the rich per-variant payloads — ``Feedback`` on rejection, per-kind ``Approval`` lists on ``ApproveForSession`` / ``ApproveForLocation``, ``Domain`` on ``ApprovePermanently``, etc. The generated ``PermissionDecision`` (``Rpc.cs:4760``) is already a proper polymorphic hierarchy with ``[JsonDerivedType]`` wired up for every variant. Switch ``OnPermissionRequest`` to return ``Task<PermissionDecision>`` and route the variant straight onto the wire — StreamJsonRpc handles the discriminator via the existing attributes. Additions: - ``PermissionDecisionNoResult`` — hand-written subclass of ``PermissionDecision`` used when the handler declines to respond so another connected client can answer. The SDK suppresses the wire response when it sees this variant. - Static factories on ``PermissionDecision`` for discoverability: ``ApproveOnce()``, ``Reject(feedback)``, ``UserNotAvailable()``, ``NoResult()``. For richer decisions that need an ``Approval`` payload, instantiate the variant class directly. Deletions: - ``PermissionRequestResult`` class (``Types.cs``) - ``PermissionRequestResultKind`` struct + obsolete enum-like wrappers - ``PermissionRequestResultKindTests.cs`` - ``PermissionRequestResult`` JSON serialization metadata **Go** Same shape: drop ``PermissionRequestResult`` + ``PermissionRequestResultKind`` in favour of ``rpc.PermissionDecision`` (already a sealed interface implemented by every variant). Added ``rpc.PermissionDecisionNoResult`` as a hand-written variant that satisfies the unexported ``permissionDecision()`` method — kept inside the ``rpc`` package since the sealing method is unexported. Handler signature changes from ``(PermissionRequestResult, error)`` to ``(rpc.PermissionDecision, error)``. ``PermissionHandler.ApproveAll`` now returns ``&rpc.PermissionDecisionApproveOnce{}``. Removed the ``rpcPermissionDecisionFromKind`` helper used to convert the flat kind back to a variant — no longer needed when the handler already returns the variant directly. **Tests / scenarios** All E2E tests and scenarios across .NET and Go updated to construct ``rpc.PermissionDecision*`` variants directly. The Go interface return type means explicit ``Task.FromResult<PermissionDecision>(...)`` casts are needed in C# where lambdas previously inferred the wrapper type. **Doc style** Per repo convention, public docs do not reference protocol v1/v2 or internal transport details. The README copy for each SDK describes the behavioural semantics ("decline to respond so another client can answer") rather than the wire mechanism. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 85f1780 commit a95b69a

36 files changed

Lines changed: 274 additions & 631 deletions

‎dotnet/README.md‎

Lines changed: 16 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -749,51 +749,37 @@ var session = await client.CreateSessionAsync(new SessionConfig
749749

750750
### Custom Permission Handler
751751

752-
Provide your own permission handler (`Func<PermissionRequest, PermissionInvocation, Task<PermissionRequestResult>>`) to inspect each request and apply custom logic:
752+
Provide your own permission handler (`Func<PermissionRequest, PermissionInvocation, Task<PermissionDecision>>`) to inspect each request and apply custom logic:
753753

754754
```csharp
755755
var session = await client.CreateSessionAsync(new SessionConfig
756756
{
757757
Model = "gpt-5",
758758
OnPermissionRequest = async (request, invocation) =>
759759
{
760-
// request.Kind — string discriminator for the type of operation being requested:
761-
// "shell" — executing a shell command
762-
// "write" — writing or editing a file
763-
// "read" — reading a file
764-
// "mcp" — calling an MCP tool
765-
// "custom_tool" — calling one of your registered tools
766-
// "url" — fetching a URL
767-
// "memory" — accessing or modifying assistant memory
768-
// "hook" — invoking a registered hook
769-
// request.ToolCallId — the tool call that triggered this request
770-
// request.ToolName — name of the tool (for custom-tool / mcp)
771-
// request.FileName — file being written (for write)
772-
// request.FullCommandText — full shell command text (for shell)
773-
774-
if (request.Kind == "shell")
760+
// Pattern-match on the discriminated PermissionRequest union to access
761+
// per-kind fields (FullCommandText, Path, ToolName, …).
762+
return request switch
775763
{
776-
// Deny shell commands
777-
return new PermissionRequestResult { Kind = PermissionRequestResultKind.Rejected };
778-
}
779-
780-
return new PermissionRequestResult { Kind = PermissionRequestResultKind.Approved };
764+
PermissionRequestShell s => PermissionDecision.Reject($"Refusing shell: {s.FullCommandText}"),
765+
_ => PermissionDecision.ApproveOnce(),
766+
};
781767
}
782768
});
783769
```
784770

785-
### Permission Result Kinds
771+
### Permission Decisions
786772

787-
The `Kind` property must be one of the canonical `PermissionRequestResultKind` values. Approval decisions are present-tense — they describe the decision to apply, not the past-tense outcome reported back on `permission.completed` session events.
773+
The handler returns a `PermissionDecision`. Use the static factories for common cases (returned types are the strongly-typed variant classes — full IntelliSense via `PermissionDecision.<dot>`):
788774

789-
| Value | Wire value | Meaning |
790-
| ------------------------------------------- | ---------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------ |
791-
| `PermissionRequestResultKind.Approved` | `"approve-once"` | Allow this single request |
792-
| `PermissionRequestResultKind.Rejected` | `"reject"` | Deny the request |
793-
| `PermissionRequestResultKind.UserNotAvailable` | `"user-not-available"` | Deny the request because no user is available to confirm it |
794-
| `PermissionRequestResultKind.NoResult` | `"no-result"` | Leave the permission request unanswered (the SDK returns without calling the RPC). Not allowed for protocol v2 permission requests (will be rejected). |
775+
| Factory | Meaning |
776+
| -------------------------------------- | -------------------------------------------------------------------------------------------- |
777+
| `PermissionDecision.ApproveOnce()` | Allow this single request |
778+
| `PermissionDecision.Reject(feedback)` | Deny the request, optionally forwarding feedback to the LLM |
779+
| `PermissionDecision.UserNotAvailable()`| Deny the request because no user is available to confirm it |
780+
| `PermissionDecision.NoResult()` | Decline to respond, allowing another connected client to answer instead |
795781

796-
> The past-tense names `PermissionRequestResultKind.DeniedInteractivelyByUser`, `PermissionRequestResultKind.DeniedCouldNotRequestFromUser`, and `PermissionRequestResultKind.DeniedByRules` remain as `[Obsolete]` aliases for backward compatibility — prefer the canonical members above in new code.
782+
For richer decisions that need an `Approval` payload — `PermissionDecisionApproveForSession`, `PermissionDecisionApproveForLocation`, `PermissionDecisionApprovePermanently` — instantiate the variant class directly.
797783

798784
### Resuming Sessions
799785

‎dotnet/src/Client.cs‎

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@ namespace GitHub.Copilot;
5353
/// </example>
5454
public sealed partial class CopilotClient : IDisposable, IAsyncDisposable
5555
{
56-
internal const string NoResultPermissionV2ErrorMessage =
57-
"Permission handlers cannot return 'no-result' when connected to a protocol v2 server.";
56+
internal const string NoResultPermissionDirectRpcErrorMessage =
57+
"Permission handlers cannot return PermissionDecision.NoResult() for this permission request.";
5858

5959
/// <summary>
6060
/// Minimum protocol version this SDK can communicate with.
@@ -1885,23 +1885,20 @@ public async ValueTask<PermissionRequestResponseV2> OnPermissionRequestV2(string
18851885

18861886
try
18871887
{
1888-
var result = await session.HandlePermissionRequestAsync(permissionRequest);
1889-
if (result.Kind == new PermissionRequestResultKind("no-result"))
1888+
var decision = await session.HandlePermissionRequestAsync(permissionRequest);
1889+
if (decision is PermissionDecisionNoResult)
18901890
{
1891-
throw new InvalidOperationException(NoResultPermissionV2ErrorMessage);
1891+
throw new InvalidOperationException(NoResultPermissionDirectRpcErrorMessage);
18921892
}
1893-
return new PermissionRequestResponseV2(result);
1893+
return new PermissionRequestResponseV2(decision);
18941894
}
1895-
catch (InvalidOperationException ex) when (ex.Message == NoResultPermissionV2ErrorMessage)
1895+
catch (InvalidOperationException ex) when (ex.Message == NoResultPermissionDirectRpcErrorMessage)
18961896
{
18971897
throw;
18981898
}
18991899
catch (Exception)
19001900
{
1901-
return new PermissionRequestResponseV2(new PermissionRequestResult
1902-
{
1903-
Kind = PermissionRequestResultKind.UserNotAvailable
1904-
});
1901+
return new PermissionRequestResponseV2(PermissionDecision.UserNotAvailable());
19051902
}
19061903
}
19071904
}
@@ -2079,7 +2076,7 @@ internal record ToolCallResponseV2(
20792076
ToolResultObject Result);
20802077

20812078
internal record PermissionRequestResponseV2(
2082-
PermissionRequestResult Result);
2079+
PermissionDecision Result);
20832080

20842081
[JsonSourceGenerationOptions(
20852082
JsonSerializerDefaults.Web,
@@ -2103,8 +2100,6 @@ internal record PermissionRequestResponseV2(
21032100
[JsonSerializable(typeof(GetSessionMetadataRequest))]
21042101
[JsonSerializable(typeof(GetSessionMetadataResponse))]
21052102
[JsonSerializable(typeof(ModelCapabilitiesOverride))]
2106-
[JsonSerializable(typeof(PermissionRequestResult))]
2107-
[JsonSerializable(typeof(PermissionRequestResultKind))]
21082103
[JsonSerializable(typeof(PermissionRequestResponseV2))]
21092104
[JsonSerializable(typeof(ProviderConfig))]
21102105
[JsonSerializable(typeof(ResumeSessionRequest))]

‎dotnet/src/PermissionDecision.cs‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
*--------------------------------------------------------------------------------------------*/
4+
5+
using System.Text.Json.Serialization;
6+
7+
namespace GitHub.Copilot.Rpc;
8+
9+
/// <summary>
10+
/// SDK-only <see cref="PermissionDecision"/> value indicating the handler
11+
/// declines to respond to this permission request. The SDK then suppresses
12+
/// the response so another connected client can answer instead.
13+
/// </summary>
14+
public sealed class PermissionDecisionNoResult : PermissionDecision
15+
{
16+
/// <inheritdoc />
17+
[JsonIgnore]
18+
public override string Kind => "no-result";
19+
}
20+
21+
/// <summary>
22+
/// Static factories for the common <see cref="PermissionDecision"/> variants
23+
/// returned by <c>OnPermissionRequest</c> handlers. Use these for quick
24+
/// discoverability via <c>PermissionDecision.&lt;dot&gt;</c>. For richer
25+
/// decisions (per-session, per-location, permanent) that need an
26+
/// <c>Approval</c> payload, instantiate the variant class directly.
27+
/// </summary>
28+
[JsonDerivedType(typeof(PermissionDecisionNoResult), "no-result")]
29+
public partial class PermissionDecision
30+
{
31+
/// <summary>Approve this single request.</summary>
32+
public static PermissionDecisionApproveOnce ApproveOnce() => new();
33+
34+
/// <summary>Reject the request, optionally forwarding feedback to the LLM.</summary>
35+
public static PermissionDecisionReject Reject(string? feedback = null) =>
36+
new() { Feedback = feedback };
37+
38+
/// <summary>Deny the request because no user is available to confirm it.</summary>
39+
public static PermissionDecisionUserNotAvailable UserNotAvailable() => new();
40+
41+
/// <summary>
42+
/// Decline to respond to this permission request, allowing another
43+
/// connected client to answer instead.
44+
/// </summary>
45+
public static PermissionDecisionNoResult NoResult() => new();
46+
}

‎dotnet/src/PermissionHandlers.cs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,14 @@
22
* Copyright (c) Microsoft Corporation. All rights reserved.
33
*--------------------------------------------------------------------------------------------*/
44

5+
using GitHub.Copilot.Rpc;
6+
57
namespace GitHub.Copilot;
68

79
/// <summary>Provides pre-built permission request handlers.</summary>
810
public static class PermissionHandler
911
{
1012
/// <summary>A permission handler that approves all permission requests.</summary>
11-
public static Func<PermissionRequest, PermissionInvocation, Task<PermissionRequestResult>> ApproveAll { get; } =
12-
(_, _) => Task.FromResult(new PermissionRequestResult { Kind = PermissionRequestResultKind.Approved });
13+
public static Func<PermissionRequest, PermissionInvocation, Task<PermissionDecision>> ApproveAll { get; } =
14+
(_, _) => Task.FromResult<PermissionDecision>(PermissionDecision.ApproveOnce());
1315
}

‎dotnet/src/Session.cs‎

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ public sealed partial class CopilotSession : IAsyncDisposable
6060
private readonly ILogger _logger;
6161
private readonly CopilotClient _parentClient;
6262

63-
private volatile Func<PermissionRequest, PermissionInvocation, Task<PermissionRequestResult>>? _permissionHandler;
63+
private volatile Func<PermissionRequest, PermissionInvocation, Task<PermissionDecision>>? _permissionHandler;
6464
private volatile Func<UserInputRequest, UserInputInvocation, Task<UserInputResponse>>? _userInputHandler;
6565
private volatile Func<ElicitationContext, Task<ElicitationResult>>? _elicitationHandler;
6666
private volatile Func<ExitPlanModeRequest, ExitPlanModeInvocation, Task<ExitPlanModeResult>>? _exitPlanModeHandler;
@@ -535,7 +535,7 @@ internal void RegisterTools(ICollection<AIFunctionDeclaration> tools)
535535
/// When the assistant needs permission to perform certain actions (e.g., file operations),
536536
/// this handler is called to approve or deny the request.
537537
/// </remarks>
538-
internal void RegisterPermissionHandler(Func<PermissionRequest, PermissionInvocation, Task<PermissionRequestResult>>? handler)
538+
internal void RegisterPermissionHandler(Func<PermissionRequest, PermissionInvocation, Task<PermissionDecision>>? handler)
539539
{
540540
_permissionHandler = handler;
541541
}
@@ -545,16 +545,13 @@ internal void RegisterPermissionHandler(Func<PermissionRequest, PermissionInvoca
545545
/// </summary>
546546
/// <param name="permissionRequestData">The permission request data from the CLI.</param>
547547
/// <returns>A task that resolves with the permission decision.</returns>
548-
internal async Task<PermissionRequestResult> HandlePermissionRequestAsync(JsonElement permissionRequestData)
548+
internal async Task<PermissionDecision> HandlePermissionRequestAsync(JsonElement permissionRequestData)
549549
{
550550
var handler = _permissionHandler;
551551

552552
if (handler == null)
553553
{
554-
return new PermissionRequestResult
555-
{
556-
Kind = PermissionRequestResultKind.UserNotAvailable
557-
};
554+
return PermissionDecision.UserNotAvailable();
558555
}
559556

560557
var request = JsonSerializer.Deserialize(permissionRequestData.GetRawText(), SessionEventsJsonContext.Default.PermissionRequest)
@@ -765,7 +762,7 @@ private async Task ExecuteToolAndRespondAsync(string requestId, string toolName,
765762
/// <summary>
766763
/// Executes a permission handler and sends the result back via the HandlePendingPermissionRequest RPC.
767764
/// </summary>
768-
private async Task ExecutePermissionAndRespondAsync(string requestId, PermissionRequest permissionRequest, Func<PermissionRequest, PermissionInvocation, Task<PermissionRequestResult>> handler)
765+
private async Task ExecutePermissionAndRespondAsync(string requestId, PermissionRequest permissionRequest, Func<PermissionRequest, PermissionInvocation, Task<PermissionDecision>> handler)
769766
{
770767
try
771768
{
@@ -775,20 +772,17 @@ private async Task ExecutePermissionAndRespondAsync(string requestId, Permission
775772
};
776773

777774
var permissionTimestamp = Stopwatch.GetTimestamp();
778-
var result = await handler(permissionRequest, invocation);
775+
var decision = await handler(permissionRequest, invocation);
779776
LoggingHelpers.LogTiming(_logger, LogLevel.Debug, null,
780777
"CopilotSession.ExecutePermissionAndRespondAsync dispatch. Elapsed={Elapsed}, SessionId={SessionId}, RequestId={RequestId}",
781778
permissionTimestamp,
782779
SessionId,
783780
requestId);
784-
if (result.Kind == new PermissionRequestResultKind("no-result"))
781+
if (decision is PermissionDecisionNoResult)
785782
{
786783
return;
787784
}
788785
var responseRpcTimestamp = Stopwatch.GetTimestamp();
789-
PermissionDecision decision = result.Kind == PermissionRequestResultKind.Rejected
790-
? new PermissionDecisionReject { Feedback = result.Feedback }
791-
: new PermissionDecision { Kind = result.Kind.Value };
792786
await Rpc.Permissions.HandlePendingPermissionRequestAsync(requestId, decision);
793787
LoggingHelpers.LogTiming(_logger, LogLevel.Debug, null,
794788
"CopilotSession.ExecutePermissionAndRespondAsync response sent successfully. Elapsed={Elapsed}, SessionId={SessionId}, RequestId={RequestId}",
@@ -800,10 +794,7 @@ private async Task ExecutePermissionAndRespondAsync(string requestId, Permission
800794
{
801795
try
802796
{
803-
await Rpc.Permissions.HandlePendingPermissionRequestAsync(requestId, new PermissionDecision
804-
{
805-
Kind = PermissionRequestResultKind.UserNotAvailable.Value
806-
});
797+
await Rpc.Permissions.HandlePendingPermissionRequestAsync(requestId, PermissionDecision.UserNotAvailable());
807798
}
808799
catch (IOException)
809800
{

0 commit comments

Comments
 (0)