From 37f5ad2b9383b33ba59f5bb9ba905c0bf88da7b0 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Mon, 2 Mar 2026 11:41:58 -0500 Subject: [PATCH 1/3] Add PermissionRequestResultKind type for .NET and Go SDKs Replace magic strings for permission request result kinds with strongly-typed values. .NET uses a readonly struct following the ChatRole pattern from Microsoft.Extensions.AI. Go uses a typed string constant block following the existing ConnectionState pattern. Both remain extensible for custom values while making the well-known kinds (approved, denied-by-rules, denied-interactively-by-user, denied-no-approval-rule-and-could-not-request-from-user) discoverable via IntelliSense/autocomplete. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- dotnet/src/Client.cs | 4 +- dotnet/src/PermissionHandlers.cs | 2 +- dotnet/src/Session.cs | 2 +- dotnet/src/Types.cs | 66 ++++++++- .../test/PermissionRequestResultKindTests.cs | 130 ++++++++++++++++++ dotnet/test/PermissionTests.cs | 14 +- dotnet/test/ToolsTests.cs | 4 +- go/client.go | 2 +- go/internal/e2e/permissions_test.go | 10 +- go/internal/e2e/tools_test.go | 4 +- go/permissions.go | 2 +- go/session.go | 2 +- go/types.go | 22 ++- go/types_test.go | 91 ++++++++++++ 14 files changed, 329 insertions(+), 26 deletions(-) create mode 100644 dotnet/test/PermissionRequestResultKindTests.cs create mode 100644 go/types_test.go diff --git a/dotnet/src/Client.cs b/dotnet/src/Client.cs index 9fc0fe8ac..5e21294fb 100644 --- a/dotnet/src/Client.cs +++ b/dotnet/src/Client.cs @@ -1314,7 +1314,7 @@ public async Task OnPermissionRequest(string sessionI { return new PermissionRequestResponse(new PermissionRequestResult { - Kind = "denied-no-approval-rule-and-could-not-request-from-user" + Kind = PermissionRequestResultKind.DeniedCouldNotRequestFromUser }); } @@ -1328,7 +1328,7 @@ public async Task OnPermissionRequest(string sessionI // If permission handler fails, deny the permission return new PermissionRequestResponse(new PermissionRequestResult { - Kind = "denied-no-approval-rule-and-could-not-request-from-user" + Kind = PermissionRequestResultKind.DeniedCouldNotRequestFromUser }); } } diff --git a/dotnet/src/PermissionHandlers.cs b/dotnet/src/PermissionHandlers.cs index 22e5bdb17..3a40e7244 100644 --- a/dotnet/src/PermissionHandlers.cs +++ b/dotnet/src/PermissionHandlers.cs @@ -9,5 +9,5 @@ public static class PermissionHandler { /// A that approves all permission requests. public static PermissionRequestHandler ApproveAll { get; } = - (_, _) => Task.FromResult(new PermissionRequestResult { Kind = "approved" }); + (_, _) => Task.FromResult(new PermissionRequestResult { Kind = PermissionRequestResultKind.Approved }); } diff --git a/dotnet/src/Session.cs b/dotnet/src/Session.cs index 8798565ff..0466c95a7 100644 --- a/dotnet/src/Session.cs +++ b/dotnet/src/Session.cs @@ -317,7 +317,7 @@ internal async Task HandlePermissionRequestAsync(JsonEl { return new PermissionRequestResult { - Kind = "denied-no-approval-rule-and-could-not-request-from-user" + Kind = PermissionRequestResultKind.DeniedCouldNotRequestFromUser }; } diff --git a/dotnet/src/Types.cs b/dotnet/src/Types.cs index 1b716cd41..e4bc6eadb 100644 --- a/dotnet/src/Types.cs +++ b/dotnet/src/Types.cs @@ -2,6 +2,9 @@ * Copyright (c) Microsoft Corporation. All rights reserved. *--------------------------------------------------------------------------------------------*/ +using System.ComponentModel; +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Text.Json; using System.Text.Json.Serialization; using Microsoft.Extensions.AI; @@ -162,10 +165,71 @@ public class PermissionRequest public Dictionary? ExtensionData { get; set; } } +/// Describes the kind of a permission request result. +[JsonConverter(typeof(PermissionRequestResultKind.Converter))] +[DebuggerDisplay("{Value,nq}")] +public readonly struct PermissionRequestResultKind : IEquatable +{ + /// Gets the kind indicating the permission was approved. + public static PermissionRequestResultKind Approved { get; } = new("approved"); + + /// Gets the kind indicating the permission was denied by rules. + public static PermissionRequestResultKind DeniedByRules { get; } = new("denied-by-rules"); + + /// Gets the kind indicating the permission was denied because no approval rule was found and the user could not be prompted. + public static PermissionRequestResultKind DeniedCouldNotRequestFromUser { get; } = new("denied-no-approval-rule-and-could-not-request-from-user"); + + /// Gets the kind indicating the permission was denied interactively by the user. + public static PermissionRequestResultKind DeniedInteractivelyByUser { get; } = new("denied-interactively-by-user"); + + /// Gets the underlying string value of this . + public string Value { get; } + + /// Initializes a new instance of the struct. + /// The string value for this kind. + [JsonConstructor] + public PermissionRequestResultKind(string value) + { + ArgumentNullException.ThrowIfNull(value); + Value = value; + } + + /// + public static bool operator ==(PermissionRequestResultKind left, PermissionRequestResultKind right) => left.Equals(right); + + /// + public static bool operator !=(PermissionRequestResultKind left, PermissionRequestResultKind right) => !left.Equals(right); + + /// + public override bool Equals([NotNullWhen(true)] object? obj) => obj is PermissionRequestResultKind other && Equals(other); + + /// + public bool Equals(PermissionRequestResultKind other) => string.Equals(Value, other.Value, StringComparison.OrdinalIgnoreCase); + + /// + public override int GetHashCode() => StringComparer.OrdinalIgnoreCase.GetHashCode(Value); + + /// + public override string ToString() => Value; + + /// Provides a for serializing instances. + [EditorBrowsable(EditorBrowsableState.Never)] + public sealed class Converter : JsonConverter + { + /// + public override PermissionRequestResultKind Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) => + new(reader.GetString()!); + + /// + public override void Write(Utf8JsonWriter writer, PermissionRequestResultKind value, JsonSerializerOptions options) => + writer.WriteStringValue(value.Value); + } +} + public class PermissionRequestResult { [JsonPropertyName("kind")] - public string Kind { get; set; } = string.Empty; + public PermissionRequestResultKind Kind { get; set; } [JsonPropertyName("rules")] public List? Rules { get; set; } diff --git a/dotnet/test/PermissionRequestResultKindTests.cs b/dotnet/test/PermissionRequestResultKindTests.cs new file mode 100644 index 000000000..d87b8c7d0 --- /dev/null +++ b/dotnet/test/PermissionRequestResultKindTests.cs @@ -0,0 +1,130 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + *--------------------------------------------------------------------------------------------*/ + +using System.Text.Json; +using Xunit; + +namespace GitHub.Copilot.SDK.Test; + +public class PermissionRequestResultKindTests +{ + private static readonly JsonSerializerOptions s_jsonOptions = new(JsonSerializerDefaults.Web) + { + TypeInfoResolver = TestJsonContext.Default, + }; + + [Fact] + public void WellKnownKinds_HaveExpectedValues() + { + Assert.Equal("approved", PermissionRequestResultKind.Approved.Value); + Assert.Equal("denied-by-rules", PermissionRequestResultKind.DeniedByRules.Value); + Assert.Equal("denied-no-approval-rule-and-could-not-request-from-user", PermissionRequestResultKind.DeniedCouldNotRequestFromUser.Value); + Assert.Equal("denied-interactively-by-user", PermissionRequestResultKind.DeniedInteractivelyByUser.Value); + } + + [Fact] + public void Equals_SameValue_ReturnsTrue() + { + var a = new PermissionRequestResultKind("approved"); + Assert.True(a == PermissionRequestResultKind.Approved); + Assert.True(a.Equals(PermissionRequestResultKind.Approved)); + Assert.True(a.Equals((object)PermissionRequestResultKind.Approved)); + } + + [Fact] + public void Equals_DifferentValue_ReturnsFalse() + { + Assert.True(PermissionRequestResultKind.Approved != PermissionRequestResultKind.DeniedByRules); + Assert.False(PermissionRequestResultKind.Approved.Equals(PermissionRequestResultKind.DeniedByRules)); + } + + [Fact] + public void Equals_IsCaseInsensitive() + { + var upper = new PermissionRequestResultKind("APPROVED"); + Assert.Equal(PermissionRequestResultKind.Approved, upper); + } + + [Fact] + public void GetHashCode_IsCaseInsensitive() + { + var upper = new PermissionRequestResultKind("APPROVED"); + Assert.Equal(PermissionRequestResultKind.Approved.GetHashCode(), upper.GetHashCode()); + } + + [Fact] + public void ToString_ReturnsValue() + { + Assert.Equal("approved", PermissionRequestResultKind.Approved.ToString()); + Assert.Equal("denied-by-rules", PermissionRequestResultKind.DeniedByRules.ToString()); + } + + [Fact] + public void CustomValue_IsPreserved() + { + var custom = new PermissionRequestResultKind("custom-kind"); + Assert.Equal("custom-kind", custom.Value); + Assert.Equal("custom-kind", custom.ToString()); + } + + [Fact] + public void Constructor_NullValue_Throws() + { + Assert.Throws(() => new PermissionRequestResultKind(null!)); + } + + [Fact] + public void Equals_NonPermissionRequestResultKindObject_ReturnsFalse() + { + Assert.False(PermissionRequestResultKind.Approved.Equals("approved")); + } + + [Fact] + public void JsonSerialize_WritesStringValue() + { + var result = new PermissionRequestResult { Kind = PermissionRequestResultKind.Approved }; + var json = JsonSerializer.Serialize(result, s_jsonOptions); + Assert.Contains("\"kind\":\"approved\"", json); + } + + [Fact] + public void JsonDeserialize_ReadsStringValue() + { + var json = """{"kind":"denied-by-rules"}"""; + var result = JsonSerializer.Deserialize(json, s_jsonOptions)!; + Assert.Equal(PermissionRequestResultKind.DeniedByRules, result.Kind); + } + + [Fact] + public void JsonRoundTrip_PreservesAllKinds() + { + var kinds = new[] + { + PermissionRequestResultKind.Approved, + PermissionRequestResultKind.DeniedByRules, + PermissionRequestResultKind.DeniedCouldNotRequestFromUser, + PermissionRequestResultKind.DeniedInteractivelyByUser, + }; + + foreach (var kind in kinds) + { + var result = new PermissionRequestResult { Kind = kind }; + var json = JsonSerializer.Serialize(result, s_jsonOptions); + var deserialized = JsonSerializer.Deserialize(json, s_jsonOptions)!; + Assert.Equal(kind, deserialized.Kind); + } + } + + [Fact] + public void JsonRoundTrip_CustomValue() + { + var result = new PermissionRequestResult { Kind = new PermissionRequestResultKind("custom") }; + var json = JsonSerializer.Serialize(result, s_jsonOptions); + var deserialized = JsonSerializer.Deserialize(json, s_jsonOptions)!; + Assert.Equal("custom", deserialized.Kind.Value); + } +} + +[System.Text.Json.Serialization.JsonSerializable(typeof(PermissionRequestResult))] +internal partial class TestJsonContext : System.Text.Json.Serialization.JsonSerializerContext; diff --git a/dotnet/test/PermissionTests.cs b/dotnet/test/PermissionTests.cs index d2c04d1e8..59a3cb4dd 100644 --- a/dotnet/test/PermissionTests.cs +++ b/dotnet/test/PermissionTests.cs @@ -21,7 +21,7 @@ public async Task Should_Invoke_Permission_Handler_For_Write_Operations() { permissionRequests.Add(request); Assert.Equal(session!.SessionId, invocation.SessionId); - return Task.FromResult(new PermissionRequestResult { Kind = "approved" }); + return Task.FromResult(new PermissionRequestResult { Kind = PermissionRequestResultKind.Approved }); } }); @@ -50,7 +50,7 @@ public async Task Should_Deny_Permission_When_Handler_Returns_Denied() { return Task.FromResult(new PermissionRequestResult { - Kind = "denied-interactively-by-user" + Kind = PermissionRequestResultKind.DeniedInteractivelyByUser }); } }); @@ -76,7 +76,7 @@ public async Task Should_Deny_Tool_Operations_When_Handler_Explicitly_Denies() var session = await CreateSessionAsync(new SessionConfig { OnPermissionRequest = (_, _) => - Task.FromResult(new PermissionRequestResult { Kind = "denied-no-approval-rule-and-could-not-request-from-user" }) + Task.FromResult(new PermissionRequestResult { Kind = PermissionRequestResultKind.DeniedCouldNotRequestFromUser }) }); var permissionDenied = false; @@ -123,7 +123,7 @@ public async Task Should_Handle_Async_Permission_Handler() permissionRequestReceived = true; // Simulate async permission check await Task.Delay(10); - return new PermissionRequestResult { Kind = "approved" }; + return new PermissionRequestResult { Kind = PermissionRequestResultKind.Approved }; } }); @@ -153,7 +153,7 @@ public async Task Should_Resume_Session_With_Permission_Handler() OnPermissionRequest = (request, invocation) => { permissionRequestReceived = true; - return Task.FromResult(new PermissionRequestResult { Kind = "approved" }); + return Task.FromResult(new PermissionRequestResult { Kind = PermissionRequestResultKind.Approved }); } }); @@ -201,7 +201,7 @@ public async Task Should_Deny_Tool_Operations_When_Handler_Explicitly_Denies_Aft var session2 = await ResumeSessionAsync(sessionId, new ResumeSessionConfig { OnPermissionRequest = (_, _) => - Task.FromResult(new PermissionRequestResult { Kind = "denied-no-approval-rule-and-could-not-request-from-user" }) + Task.FromResult(new PermissionRequestResult { Kind = PermissionRequestResultKind.DeniedCouldNotRequestFromUser }) }); var permissionDenied = false; @@ -235,7 +235,7 @@ public async Task Should_Receive_ToolCallId_In_Permission_Requests() { receivedToolCallId = true; } - return Task.FromResult(new PermissionRequestResult { Kind = "approved" }); + return Task.FromResult(new PermissionRequestResult { Kind = PermissionRequestResultKind.Approved }); } }); diff --git a/dotnet/test/ToolsTests.cs b/dotnet/test/ToolsTests.cs index c6449ec8f..9e7ad11d4 100644 --- a/dotnet/test/ToolsTests.cs +++ b/dotnet/test/ToolsTests.cs @@ -194,7 +194,7 @@ public async Task Invokes_Custom_Tool_With_Permission_Handler() OnPermissionRequest = (request, invocation) => { permissionRequests.Add(request); - return Task.FromResult(new PermissionRequestResult { Kind = "approved" }); + return Task.FromResult(new PermissionRequestResult { Kind = PermissionRequestResultKind.Approved }); }, }); @@ -229,7 +229,7 @@ public async Task Denies_Custom_Tool_When_Permission_Denied() Tools = [AIFunctionFactory.Create(EncryptStringDenied, "encrypt_string")], OnPermissionRequest = (request, invocation) => { - return Task.FromResult(new PermissionRequestResult { Kind = "denied-interactively-by-user" }); + return Task.FromResult(new PermissionRequestResult { Kind = PermissionRequestResultKind.DeniedInteractivelyByUser }); }, }); diff --git a/go/client.go b/go/client.go index c88a68ac3..2ceba1f9f 100644 --- a/go/client.go +++ b/go/client.go @@ -1342,7 +1342,7 @@ func (c *Client) handlePermissionRequest(req permissionRequestRequest) (*permiss // Return denial on error return &permissionRequestResponse{ Result: PermissionRequestResult{ - Kind: "denied-no-approval-rule-and-could-not-request-from-user", + Kind: PermissionKindDeniedCouldNotRequestFromUser, }, }, nil } diff --git a/go/internal/e2e/permissions_test.go b/go/internal/e2e/permissions_test.go index d1d9134b1..b5def9178 100644 --- a/go/internal/e2e/permissions_test.go +++ b/go/internal/e2e/permissions_test.go @@ -31,7 +31,7 @@ func TestPermissions(t *testing.T) { t.Error("Expected non-empty session ID in invocation") } - return copilot.PermissionRequestResult{Kind: "approved"}, nil + return copilot.PermissionRequestResult{Kind: copilot.PermissionKindApproved}, nil } session, err := client.CreateSession(t.Context(), &copilot.SessionConfig{ @@ -82,7 +82,7 @@ func TestPermissions(t *testing.T) { permissionRequests = append(permissionRequests, request) mu.Unlock() - return copilot.PermissionRequestResult{Kind: "approved"}, nil + return copilot.PermissionRequestResult{Kind: copilot.PermissionKindApproved}, nil } session, err := client.CreateSession(t.Context(), &copilot.SessionConfig{ @@ -117,7 +117,7 @@ func TestPermissions(t *testing.T) { ctx.ConfigureForTest(t) onPermissionRequest := func(request copilot.PermissionRequest, invocation copilot.PermissionInvocation) (copilot.PermissionRequestResult, error) { - return copilot.PermissionRequestResult{Kind: "denied-interactively-by-user"}, nil + return copilot.PermissionRequestResult{Kind: copilot.PermissionKindDeniedInteractivelyByUser}, nil } session, err := client.CreateSession(t.Context(), &copilot.SessionConfig{ @@ -162,7 +162,7 @@ func TestPermissions(t *testing.T) { session, err := client.CreateSession(t.Context(), &copilot.SessionConfig{ OnPermissionRequest: func(request copilot.PermissionRequest, invocation copilot.PermissionInvocation) (copilot.PermissionRequestResult, error) { - return copilot.PermissionRequestResult{Kind: "denied-no-approval-rule-and-could-not-request-from-user"}, nil + return copilot.PermissionRequestResult{Kind: copilot.PermissionKindDeniedCouldNotRequestFromUser}, nil }, }) if err != nil { @@ -212,7 +212,7 @@ func TestPermissions(t *testing.T) { session2, err := client.ResumeSession(t.Context(), sessionID, &copilot.ResumeSessionConfig{ OnPermissionRequest: func(request copilot.PermissionRequest, invocation copilot.PermissionInvocation) (copilot.PermissionRequestResult, error) { - return copilot.PermissionRequestResult{Kind: "denied-no-approval-rule-and-could-not-request-from-user"}, nil + return copilot.PermissionRequestResult{Kind: copilot.PermissionKindDeniedCouldNotRequestFromUser}, nil }, }) if err != nil { diff --git a/go/internal/e2e/tools_test.go b/go/internal/e2e/tools_test.go index e5b93fa25..6eb95d7c5 100644 --- a/go/internal/e2e/tools_test.go +++ b/go/internal/e2e/tools_test.go @@ -285,7 +285,7 @@ func TestTools(t *testing.T) { mu.Lock() permissionRequests = append(permissionRequests, request) mu.Unlock() - return copilot.PermissionRequestResult{Kind: "approved"}, nil + return copilot.PermissionRequestResult{Kind: copilot.PermissionKindApproved}, nil }, }) if err != nil { @@ -341,7 +341,7 @@ func TestTools(t *testing.T) { }), }, OnPermissionRequest: func(request copilot.PermissionRequest, invocation copilot.PermissionInvocation) (copilot.PermissionRequestResult, error) { - return copilot.PermissionRequestResult{Kind: "denied-interactively-by-user"}, nil + return copilot.PermissionRequestResult{Kind: copilot.PermissionKindDeniedInteractivelyByUser}, nil }, }) if err != nil { diff --git a/go/permissions.go b/go/permissions.go index 91ff776cf..127644de5 100644 --- a/go/permissions.go +++ b/go/permissions.go @@ -6,6 +6,6 @@ var PermissionHandler = struct { ApproveAll PermissionHandlerFunc }{ ApproveAll: func(_ PermissionRequest, _ PermissionInvocation) (PermissionRequestResult, error) { - return PermissionRequestResult{Kind: "approved"}, nil + return PermissionRequestResult{Kind: PermissionKindApproved}, nil }, } diff --git a/go/session.go b/go/session.go index 12d1b1afa..9c3313715 100644 --- a/go/session.go +++ b/go/session.go @@ -310,7 +310,7 @@ func (s *Session) handlePermissionRequest(request PermissionRequest) (Permission if handler == nil { return PermissionRequestResult{ - Kind: "denied-no-approval-rule-and-could-not-request-from-user", + Kind: PermissionKindDeniedCouldNotRequestFromUser, }, nil } diff --git a/go/types.go b/go/types.go index 225cc1266..f48667f65 100644 --- a/go/types.go +++ b/go/types.go @@ -132,10 +132,28 @@ func (p *PermissionRequest) UnmarshalJSON(data []byte) error { return nil } +// PermissionRequestResultKind represents the kind of a permission request result. +type PermissionRequestResultKind string + +const ( + // PermissionKindApproved indicates the permission was approved. + PermissionKindApproved PermissionRequestResultKind = "approved" + + // PermissionKindDeniedByRules indicates the permission was denied by rules. + PermissionKindDeniedByRules PermissionRequestResultKind = "denied-by-rules" + + // PermissionKindDeniedCouldNotRequestFromUser indicates the permission was denied because + // no approval rule was found and the user could not be prompted. + PermissionKindDeniedCouldNotRequestFromUser PermissionRequestResultKind = "denied-no-approval-rule-and-could-not-request-from-user" + + // PermissionKindDeniedInteractivelyByUser indicates the permission was denied interactively by the user. + PermissionKindDeniedInteractivelyByUser PermissionRequestResultKind = "denied-interactively-by-user" +) + // PermissionRequestResult represents the result of a permission request type PermissionRequestResult struct { - Kind string `json:"kind"` - Rules []any `json:"rules,omitempty"` + Kind PermissionRequestResultKind `json:"kind"` + Rules []any `json:"rules,omitempty"` } // PermissionHandlerFunc executes a permission request diff --git a/go/types_test.go b/go/types_test.go new file mode 100644 index 000000000..22b7966b1 --- /dev/null +++ b/go/types_test.go @@ -0,0 +1,91 @@ +package copilot + +import ( + "encoding/json" + "testing" +) + +func TestPermissionRequestResultKind_Constants(t *testing.T) { + tests := []struct { + name string + kind PermissionRequestResultKind + expected string + }{ + {"Approved", PermissionKindApproved, "approved"}, + {"DeniedByRules", PermissionKindDeniedByRules, "denied-by-rules"}, + {"DeniedCouldNotRequestFromUser", PermissionKindDeniedCouldNotRequestFromUser, "denied-no-approval-rule-and-could-not-request-from-user"}, + {"DeniedInteractivelyByUser", PermissionKindDeniedInteractivelyByUser, "denied-interactively-by-user"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if string(tt.kind) != tt.expected { + t.Errorf("expected %q, got %q", tt.expected, string(tt.kind)) + } + }) + } +} + +func TestPermissionRequestResultKind_CustomValue(t *testing.T) { + custom := PermissionRequestResultKind("custom-kind") + if string(custom) != "custom-kind" { + t.Errorf("expected %q, got %q", "custom-kind", string(custom)) + } +} + +func TestPermissionRequestResult_JSONRoundTrip(t *testing.T) { + tests := []struct { + name string + kind PermissionRequestResultKind + }{ + {"Approved", PermissionKindApproved}, + {"DeniedByRules", PermissionKindDeniedByRules}, + {"DeniedCouldNotRequestFromUser", PermissionKindDeniedCouldNotRequestFromUser}, + {"DeniedInteractivelyByUser", PermissionKindDeniedInteractivelyByUser}, + {"Custom", PermissionRequestResultKind("custom")}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + original := PermissionRequestResult{Kind: tt.kind} + data, err := json.Marshal(original) + if err != nil { + t.Fatalf("failed to marshal: %v", err) + } + + var decoded PermissionRequestResult + if err := json.Unmarshal(data, &decoded); err != nil { + t.Fatalf("failed to unmarshal: %v", err) + } + + if decoded.Kind != tt.kind { + t.Errorf("expected kind %q, got %q", tt.kind, decoded.Kind) + } + }) + } +} + +func TestPermissionRequestResult_JSONDeserialize(t *testing.T) { + jsonStr := `{"kind":"denied-by-rules"}` + var result PermissionRequestResult + if err := json.Unmarshal([]byte(jsonStr), &result); err != nil { + t.Fatalf("failed to unmarshal: %v", err) + } + + if result.Kind != PermissionKindDeniedByRules { + t.Errorf("expected %q, got %q", PermissionKindDeniedByRules, result.Kind) + } +} + +func TestPermissionRequestResult_JSONSerialize(t *testing.T) { + result := PermissionRequestResult{Kind: PermissionKindApproved} + data, err := json.Marshal(result) + if err != nil { + t.Fatalf("failed to marshal: %v", err) + } + + expected := `{"kind":"approved"}` + if string(data) != expected { + t.Errorf("expected %s, got %s", expected, string(data)) + } +} From c72f67382294dcb69d66d00ecb25eda59a3a52e4 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Mon, 2 Mar 2026 12:41:04 -0500 Subject: [PATCH 2/3] Address PR feedback and fix CI issues Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/guides/skills.md | 4 +-- dotnet/src/Types.cs | 27 +++++++++++++------ .../test/PermissionRequestResultKindTests.cs | 14 ++++++++-- go/client.go | 2 +- go/internal/e2e/permissions_test.go | 10 +++---- go/internal/e2e/tools_test.go | 4 +-- go/permissions.go | 2 +- go/session.go | 2 +- go/types.go | 16 +++++------ go/types_test.go | 22 +++++++-------- .../callbacks/hooks/csharp/Program.cs | 2 +- .../callbacks/permissions/csharp/Program.cs | 2 +- .../callbacks/user-input/csharp/Program.cs | 2 +- test/scenarios/tools/skills/csharp/Program.cs | 2 +- .../virtual-filesystem/csharp/Program.cs | 2 +- 15 files changed, 67 insertions(+), 46 deletions(-) diff --git a/docs/guides/skills.md b/docs/guides/skills.md index b2ea3ae7a..5e085e3b2 100644 --- a/docs/guides/skills.md +++ b/docs/guides/skills.md @@ -92,7 +92,7 @@ func main() { "./skills/documentation", }, OnPermissionRequest: func(req copilot.PermissionRequest, inv copilot.PermissionInvocation) (copilot.PermissionRequestResult, error) { - return copilot.PermissionRequestResult{Kind: "approved"}, nil + return copilot.PermissionRequestResult{Kind: copilot.PermissionRequestResultKindApproved}, nil }, }) if err != nil { @@ -127,7 +127,7 @@ await using var session = await client.CreateSessionAsync(new SessionConfig "./skills/documentation", }, OnPermissionRequest = (req, inv) => - Task.FromResult(new PermissionRequestResult { Kind = "approved" }), + Task.FromResult(new PermissionRequestResult { Kind = PermissionRequestResultKind.Approved }), }); // Copilot now has access to skills in those directories diff --git a/dotnet/src/Types.cs b/dotnet/src/Types.cs index e4bc6eadb..dcb0ad0f9 100644 --- a/dotnet/src/Types.cs +++ b/dotnet/src/Types.cs @@ -183,16 +183,14 @@ public class PermissionRequest public static PermissionRequestResultKind DeniedInteractivelyByUser { get; } = new("denied-interactively-by-user"); /// Gets the underlying string value of this . - public string Value { get; } + public string Value => _value ?? string.Empty; + + private readonly string? _value; /// Initializes a new instance of the struct. /// The string value for this kind. [JsonConstructor] - public PermissionRequestResultKind(string value) - { - ArgumentNullException.ThrowIfNull(value); - Value = value; - } + public PermissionRequestResultKind(string value) => _value = value; /// public static bool operator ==(PermissionRequestResultKind left, PermissionRequestResultKind right) => left.Equals(right); @@ -217,8 +215,21 @@ public PermissionRequestResultKind(string value) public sealed class Converter : JsonConverter { /// - public override PermissionRequestResultKind Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) => - new(reader.GetString()!); + public override PermissionRequestResultKind Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + if (reader.TokenType != JsonTokenType.String) + { + throw new JsonException("Expected string for PermissionRequestResultKind."); + } + + var value = reader.GetString(); + if (value is null) + { + throw new JsonException("PermissionRequestResultKind value cannot be null."); + } + + return new PermissionRequestResultKind(value); + } /// public override void Write(Utf8JsonWriter writer, PermissionRequestResultKind value, JsonSerializerOptions options) => diff --git a/dotnet/test/PermissionRequestResultKindTests.cs b/dotnet/test/PermissionRequestResultKindTests.cs index d87b8c7d0..d0cfed6f0 100644 --- a/dotnet/test/PermissionRequestResultKindTests.cs +++ b/dotnet/test/PermissionRequestResultKindTests.cs @@ -69,9 +69,19 @@ public void CustomValue_IsPreserved() } [Fact] - public void Constructor_NullValue_Throws() + public void Constructor_NullValue_TreatedAsEmpty() { - Assert.Throws(() => new PermissionRequestResultKind(null!)); + var kind = new PermissionRequestResultKind(null!); + Assert.Equal(string.Empty, kind.Value); + } + + [Fact] + public void Default_HasEmptyStringValue() + { + var defaultKind = default(PermissionRequestResultKind); + Assert.Equal(string.Empty, defaultKind.Value); + Assert.Equal(string.Empty, defaultKind.ToString()); + Assert.Equal(defaultKind.GetHashCode(), defaultKind.GetHashCode()); } [Fact] diff --git a/go/client.go b/go/client.go index 2ceba1f9f..f83b60f1d 100644 --- a/go/client.go +++ b/go/client.go @@ -1342,7 +1342,7 @@ func (c *Client) handlePermissionRequest(req permissionRequestRequest) (*permiss // Return denial on error return &permissionRequestResponse{ Result: PermissionRequestResult{ - Kind: PermissionKindDeniedCouldNotRequestFromUser, + Kind: PermissionRequestResultKindDeniedCouldNotRequestFromUser, }, }, nil } diff --git a/go/internal/e2e/permissions_test.go b/go/internal/e2e/permissions_test.go index b5def9178..328e7e788 100644 --- a/go/internal/e2e/permissions_test.go +++ b/go/internal/e2e/permissions_test.go @@ -31,7 +31,7 @@ func TestPermissions(t *testing.T) { t.Error("Expected non-empty session ID in invocation") } - return copilot.PermissionRequestResult{Kind: copilot.PermissionKindApproved}, nil + return copilot.PermissionRequestResult{Kind: copilot.PermissionRequestResultKindApproved}, nil } session, err := client.CreateSession(t.Context(), &copilot.SessionConfig{ @@ -82,7 +82,7 @@ func TestPermissions(t *testing.T) { permissionRequests = append(permissionRequests, request) mu.Unlock() - return copilot.PermissionRequestResult{Kind: copilot.PermissionKindApproved}, nil + return copilot.PermissionRequestResult{Kind: copilot.PermissionRequestResultKindApproved}, nil } session, err := client.CreateSession(t.Context(), &copilot.SessionConfig{ @@ -117,7 +117,7 @@ func TestPermissions(t *testing.T) { ctx.ConfigureForTest(t) onPermissionRequest := func(request copilot.PermissionRequest, invocation copilot.PermissionInvocation) (copilot.PermissionRequestResult, error) { - return copilot.PermissionRequestResult{Kind: copilot.PermissionKindDeniedInteractivelyByUser}, nil + return copilot.PermissionRequestResult{Kind: copilot.PermissionRequestResultKindDeniedInteractivelyByUser}, nil } session, err := client.CreateSession(t.Context(), &copilot.SessionConfig{ @@ -162,7 +162,7 @@ func TestPermissions(t *testing.T) { session, err := client.CreateSession(t.Context(), &copilot.SessionConfig{ OnPermissionRequest: func(request copilot.PermissionRequest, invocation copilot.PermissionInvocation) (copilot.PermissionRequestResult, error) { - return copilot.PermissionRequestResult{Kind: copilot.PermissionKindDeniedCouldNotRequestFromUser}, nil + return copilot.PermissionRequestResult{Kind: copilot.PermissionRequestResultKindDeniedCouldNotRequestFromUser}, nil }, }) if err != nil { @@ -212,7 +212,7 @@ func TestPermissions(t *testing.T) { session2, err := client.ResumeSession(t.Context(), sessionID, &copilot.ResumeSessionConfig{ OnPermissionRequest: func(request copilot.PermissionRequest, invocation copilot.PermissionInvocation) (copilot.PermissionRequestResult, error) { - return copilot.PermissionRequestResult{Kind: copilot.PermissionKindDeniedCouldNotRequestFromUser}, nil + return copilot.PermissionRequestResult{Kind: copilot.PermissionRequestResultKindDeniedCouldNotRequestFromUser}, nil }, }) if err != nil { diff --git a/go/internal/e2e/tools_test.go b/go/internal/e2e/tools_test.go index 6eb95d7c5..21ce7d8a0 100644 --- a/go/internal/e2e/tools_test.go +++ b/go/internal/e2e/tools_test.go @@ -285,7 +285,7 @@ func TestTools(t *testing.T) { mu.Lock() permissionRequests = append(permissionRequests, request) mu.Unlock() - return copilot.PermissionRequestResult{Kind: copilot.PermissionKindApproved}, nil + return copilot.PermissionRequestResult{Kind: copilot.PermissionRequestResultKindApproved}, nil }, }) if err != nil { @@ -341,7 +341,7 @@ func TestTools(t *testing.T) { }), }, OnPermissionRequest: func(request copilot.PermissionRequest, invocation copilot.PermissionInvocation) (copilot.PermissionRequestResult, error) { - return copilot.PermissionRequestResult{Kind: copilot.PermissionKindDeniedInteractivelyByUser}, nil + return copilot.PermissionRequestResult{Kind: copilot.PermissionRequestResultKindDeniedInteractivelyByUser}, nil }, }) if err != nil { diff --git a/go/permissions.go b/go/permissions.go index 127644de5..fb28851e3 100644 --- a/go/permissions.go +++ b/go/permissions.go @@ -6,6 +6,6 @@ var PermissionHandler = struct { ApproveAll PermissionHandlerFunc }{ ApproveAll: func(_ PermissionRequest, _ PermissionInvocation) (PermissionRequestResult, error) { - return PermissionRequestResult{Kind: PermissionKindApproved}, nil + return PermissionRequestResult{Kind: PermissionRequestResultKindApproved}, nil }, } diff --git a/go/session.go b/go/session.go index 9c3313715..d7f3b9b61 100644 --- a/go/session.go +++ b/go/session.go @@ -310,7 +310,7 @@ func (s *Session) handlePermissionRequest(request PermissionRequest) (Permission if handler == nil { return PermissionRequestResult{ - Kind: PermissionKindDeniedCouldNotRequestFromUser, + Kind: PermissionRequestResultKindDeniedCouldNotRequestFromUser, }, nil } diff --git a/go/types.go b/go/types.go index f48667f65..4717b90ed 100644 --- a/go/types.go +++ b/go/types.go @@ -136,18 +136,18 @@ func (p *PermissionRequest) UnmarshalJSON(data []byte) error { type PermissionRequestResultKind string const ( - // PermissionKindApproved indicates the permission was approved. - PermissionKindApproved PermissionRequestResultKind = "approved" + // PermissionRequestResultKindApproved indicates the permission was approved. + PermissionRequestResultKindApproved PermissionRequestResultKind = "approved" - // PermissionKindDeniedByRules indicates the permission was denied by rules. - PermissionKindDeniedByRules PermissionRequestResultKind = "denied-by-rules" + // PermissionRequestResultKindDeniedByRules indicates the permission was denied by rules. + PermissionRequestResultKindDeniedByRules PermissionRequestResultKind = "denied-by-rules" - // PermissionKindDeniedCouldNotRequestFromUser indicates the permission was denied because + // PermissionRequestResultKindDeniedCouldNotRequestFromUser indicates the permission was denied because // no approval rule was found and the user could not be prompted. - PermissionKindDeniedCouldNotRequestFromUser PermissionRequestResultKind = "denied-no-approval-rule-and-could-not-request-from-user" + PermissionRequestResultKindDeniedCouldNotRequestFromUser PermissionRequestResultKind = "denied-no-approval-rule-and-could-not-request-from-user" - // PermissionKindDeniedInteractivelyByUser indicates the permission was denied interactively by the user. - PermissionKindDeniedInteractivelyByUser PermissionRequestResultKind = "denied-interactively-by-user" + // PermissionRequestResultKindDeniedInteractivelyByUser indicates the permission was denied interactively by the user. + PermissionRequestResultKindDeniedInteractivelyByUser PermissionRequestResultKind = "denied-interactively-by-user" ) // PermissionRequestResult represents the result of a permission request diff --git a/go/types_test.go b/go/types_test.go index 22b7966b1..190cd913d 100644 --- a/go/types_test.go +++ b/go/types_test.go @@ -11,10 +11,10 @@ func TestPermissionRequestResultKind_Constants(t *testing.T) { kind PermissionRequestResultKind expected string }{ - {"Approved", PermissionKindApproved, "approved"}, - {"DeniedByRules", PermissionKindDeniedByRules, "denied-by-rules"}, - {"DeniedCouldNotRequestFromUser", PermissionKindDeniedCouldNotRequestFromUser, "denied-no-approval-rule-and-could-not-request-from-user"}, - {"DeniedInteractivelyByUser", PermissionKindDeniedInteractivelyByUser, "denied-interactively-by-user"}, + {"Approved", PermissionRequestResultKindApproved, "approved"}, + {"DeniedByRules", PermissionRequestResultKindDeniedByRules, "denied-by-rules"}, + {"DeniedCouldNotRequestFromUser", PermissionRequestResultKindDeniedCouldNotRequestFromUser, "denied-no-approval-rule-and-could-not-request-from-user"}, + {"DeniedInteractivelyByUser", PermissionRequestResultKindDeniedInteractivelyByUser, "denied-interactively-by-user"}, } for _, tt := range tests { @@ -38,10 +38,10 @@ func TestPermissionRequestResult_JSONRoundTrip(t *testing.T) { name string kind PermissionRequestResultKind }{ - {"Approved", PermissionKindApproved}, - {"DeniedByRules", PermissionKindDeniedByRules}, - {"DeniedCouldNotRequestFromUser", PermissionKindDeniedCouldNotRequestFromUser}, - {"DeniedInteractivelyByUser", PermissionKindDeniedInteractivelyByUser}, + {"Approved", PermissionRequestResultKindApproved}, + {"DeniedByRules", PermissionRequestResultKindDeniedByRules}, + {"DeniedCouldNotRequestFromUser", PermissionRequestResultKindDeniedCouldNotRequestFromUser}, + {"DeniedInteractivelyByUser", PermissionRequestResultKindDeniedInteractivelyByUser}, {"Custom", PermissionRequestResultKind("custom")}, } @@ -72,13 +72,13 @@ func TestPermissionRequestResult_JSONDeserialize(t *testing.T) { t.Fatalf("failed to unmarshal: %v", err) } - if result.Kind != PermissionKindDeniedByRules { - t.Errorf("expected %q, got %q", PermissionKindDeniedByRules, result.Kind) + if result.Kind != PermissionRequestResultKindDeniedByRules { + t.Errorf("expected %q, got %q", PermissionRequestResultKindDeniedByRules, result.Kind) } } func TestPermissionRequestResult_JSONSerialize(t *testing.T) { - result := PermissionRequestResult{Kind: PermissionKindApproved} + result := PermissionRequestResult{Kind: PermissionRequestResultKindApproved} data, err := json.Marshal(result) if err != nil { t.Fatalf("failed to marshal: %v", err) diff --git a/test/scenarios/callbacks/hooks/csharp/Program.cs b/test/scenarios/callbacks/hooks/csharp/Program.cs index 30b34e309..63c15128f 100644 --- a/test/scenarios/callbacks/hooks/csharp/Program.cs +++ b/test/scenarios/callbacks/hooks/csharp/Program.cs @@ -16,7 +16,7 @@ { Model = "claude-haiku-4.5", OnPermissionRequest = (request, invocation) => - Task.FromResult(new PermissionRequestResult { Kind = "approved" }), + Task.FromResult(new PermissionRequestResult { Kind = PermissionRequestResultKind.Approved }), Hooks = new SessionHooks { OnSessionStart = (input, invocation) => diff --git a/test/scenarios/callbacks/permissions/csharp/Program.cs b/test/scenarios/callbacks/permissions/csharp/Program.cs index 7803c3a75..0000ed575 100644 --- a/test/scenarios/callbacks/permissions/csharp/Program.cs +++ b/test/scenarios/callbacks/permissions/csharp/Program.cs @@ -21,7 +21,7 @@ ? value?.ToString() ?? "unknown" : "unknown"; permissionLog.Add($"approved:{toolName}"); - return Task.FromResult(new PermissionRequestResult { Kind = "approved" }); + return Task.FromResult(new PermissionRequestResult { Kind = PermissionRequestResultKind.Approved }); }, Hooks = new SessionHooks { diff --git a/test/scenarios/callbacks/user-input/csharp/Program.cs b/test/scenarios/callbacks/user-input/csharp/Program.cs index 4e1c15cab..6ad0454d7 100644 --- a/test/scenarios/callbacks/user-input/csharp/Program.cs +++ b/test/scenarios/callbacks/user-input/csharp/Program.cs @@ -16,7 +16,7 @@ { Model = "claude-haiku-4.5", OnPermissionRequest = (request, invocation) => - Task.FromResult(new PermissionRequestResult { Kind = "approved" }), + Task.FromResult(new PermissionRequestResult { Kind = PermissionRequestResultKind.Approved }), OnUserInputRequest = (request, invocation) => { inputLog.Add($"question: {request.Question}"); diff --git a/test/scenarios/tools/skills/csharp/Program.cs b/test/scenarios/tools/skills/csharp/Program.cs index 38df0dac1..d0394a396 100644 --- a/test/scenarios/tools/skills/csharp/Program.cs +++ b/test/scenarios/tools/skills/csharp/Program.cs @@ -17,7 +17,7 @@ Model = "claude-haiku-4.5", SkillDirectories = [skillsDir], OnPermissionRequest = (request, invocation) => - Task.FromResult(new PermissionRequestResult { Kind = "approved" }), + Task.FromResult(new PermissionRequestResult { Kind = PermissionRequestResultKind.Approved }), Hooks = new SessionHooks { OnPreToolUse = (input, invocation) => diff --git a/test/scenarios/tools/virtual-filesystem/csharp/Program.cs b/test/scenarios/tools/virtual-filesystem/csharp/Program.cs index 7dd5d710e..d67a3738c 100644 --- a/test/scenarios/tools/virtual-filesystem/csharp/Program.cs +++ b/test/scenarios/tools/virtual-filesystem/csharp/Program.cs @@ -49,7 +49,7 @@ "List all files in the virtual filesystem"), ], OnPermissionRequest = (request, invocation) => - Task.FromResult(new PermissionRequestResult { Kind = "approved" }), + Task.FromResult(new PermissionRequestResult { Kind = PermissionRequestResultKind.Approved }), Hooks = new SessionHooks { OnPreToolUse = (input, invocation) => From 5db34d2a4e931a0e43bb39b36d8f90ed543eff5f Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Mon, 2 Mar 2026 15:48:48 -0500 Subject: [PATCH 3/3] Mock session.resume response in client test to fix fork PR CI The 'forwards clientName in session.resume request' test calls through to the real CLI, which requires COPILOT_HMAC_KEY for authentication. This secret is unavailable to fork PRs, causing CI failures. Mock the sendRequest return value since the test only verifies parameter forwarding. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/test/client.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/nodejs/test/client.test.ts b/nodejs/test/client.test.ts index 6fa33e9ec..bddc66e93 100644 --- a/nodejs/test/client.test.ts +++ b/nodejs/test/client.test.ts @@ -68,7 +68,9 @@ describe("CopilotClient", () => { onTestFinished(() => client.forceStop()); const session = await client.createSession({ onPermissionRequest: approveAll }); - const spy = vi.spyOn((client as any).connection!, "sendRequest"); + const spy = vi + .spyOn((client as any).connection!, "sendRequest") + .mockResolvedValue({ sessionId: session.sessionId }); await client.resumeSession(session.sessionId, { clientName: "my-app", onPermissionRequest: approveAll,