Skip to content

Commit c72f673

Browse files
stephentoubCopilot
stephentoub
and
Copilot
committed
Address PR feedback and fix CI issues
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 37f5ad2 commit c72f673

File tree

15 files changed

+67
-46
lines changed

15 files changed

+67
-46
lines changed

‎docs/guides/skills.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ func main() {
9292
"./skills/documentation",
9393
},
9494
OnPermissionRequest: func(req copilot.PermissionRequest, inv copilot.PermissionInvocation) (copilot.PermissionRequestResult, error) {
95-
return copilot.PermissionRequestResult{Kind: "approved"}, nil
95+
return copilot.PermissionRequestResult{Kind: copilot.PermissionRequestResultKindApproved}, nil
9696
},
9797
})
9898
if err != nil {
@@ -127,7 +127,7 @@ await using var session = await client.CreateSessionAsync(new SessionConfig
127127
"./skills/documentation",
128128
},
129129
OnPermissionRequest = (req, inv) =>
130-
Task.FromResult(new PermissionRequestResult { Kind = "approved" }),
130+
Task.FromResult(new PermissionRequestResult { Kind = PermissionRequestResultKind.Approved }),
131131
});
132132

133133
// Copilot now has access to skills in those directories

‎dotnet/src/Types.cs‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -183,16 +183,14 @@ public class PermissionRequest
183183
public static PermissionRequestResultKind DeniedInteractivelyByUser { get; } = new("denied-interactively-by-user");
184184

185185
/// <summary>Gets the underlying string value of this <see cref="PermissionRequestResultKind"/>.</summary>
186-
public string Value { get; }
186+
public string Value => _value ?? string.Empty;
187+
188+
private readonly string? _value;
187189

188190
/// <summary>Initializes a new instance of the <see cref="PermissionRequestResultKind"/> struct.</summary>
189191
/// <param name="value">The string value for this kind.</param>
190192
[JsonConstructor]
191-
public PermissionRequestResultKind(string value)
192-
{
193-
ArgumentNullException.ThrowIfNull(value);
194-
Value = value;
195-
}
193+
public PermissionRequestResultKind(string value) => _value = value;
196194

197195
/// <inheritdoc/>
198196
public static bool operator ==(PermissionRequestResultKind left, PermissionRequestResultKind right) => left.Equals(right);
@@ -217,8 +215,21 @@ public PermissionRequestResultKind(string value)
217215
public sealed class Converter : JsonConverter<PermissionRequestResultKind>
218216
{
219217
/// <inheritdoc/>
220-
public override PermissionRequestResultKind Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) =>
221-
new(reader.GetString()!);
218+
public override PermissionRequestResultKind Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
219+
{
220+
if (reader.TokenType != JsonTokenType.String)
221+
{
222+
throw new JsonException("Expected string for PermissionRequestResultKind.");
223+
}
224+
225+
var value = reader.GetString();
226+
if (value is null)
227+
{
228+
throw new JsonException("PermissionRequestResultKind value cannot be null.");
229+
}
230+
231+
return new PermissionRequestResultKind(value);
232+
}
222233

223234
/// <inheritdoc/>
224235
public override void Write(Utf8JsonWriter writer, PermissionRequestResultKind value, JsonSerializerOptions options) =>

‎dotnet/test/PermissionRequestResultKindTests.cs‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,19 @@ public void CustomValue_IsPreserved()
6969
}
7070

7171
[Fact]
72-
public void Constructor_NullValue_Throws()
72+
public void Constructor_NullValue_TreatedAsEmpty()
7373
{
74-
Assert.Throws<ArgumentNullException>(() => new PermissionRequestResultKind(null!));
74+
var kind = new PermissionRequestResultKind(null!);
75+
Assert.Equal(string.Empty, kind.Value);
76+
}
77+
78+
[Fact]
79+
public void Default_HasEmptyStringValue()
80+
{
81+
var defaultKind = default(PermissionRequestResultKind);
82+
Assert.Equal(string.Empty, defaultKind.Value);
83+
Assert.Equal(string.Empty, defaultKind.ToString());
84+
Assert.Equal(defaultKind.GetHashCode(), defaultKind.GetHashCode());
7585
}
7686

7787
[Fact]

‎go/client.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1342,7 +1342,7 @@ func (c *Client) handlePermissionRequest(req permissionRequestRequest) (*permiss
13421342
// Return denial on error
13431343
return &permissionRequestResponse{
13441344
Result: PermissionRequestResult{
1345-
Kind: PermissionKindDeniedCouldNotRequestFromUser,
1345+
Kind: PermissionRequestResultKindDeniedCouldNotRequestFromUser,
13461346
},
13471347
}, nil
13481348
}

‎go/internal/e2e/permissions_test.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ func TestPermissions(t *testing.T) {
3131
t.Error("Expected non-empty session ID in invocation")
3232
}
3333

34-
return copilot.PermissionRequestResult{Kind: copilot.PermissionKindApproved}, nil
34+
return copilot.PermissionRequestResult{Kind: copilot.PermissionRequestResultKindApproved}, nil
3535
}
3636

3737
session, err := client.CreateSession(t.Context(), &copilot.SessionConfig{
@@ -82,7 +82,7 @@ func TestPermissions(t *testing.T) {
8282
permissionRequests = append(permissionRequests, request)
8383
mu.Unlock()
8484

85-
return copilot.PermissionRequestResult{Kind: copilot.PermissionKindApproved}, nil
85+
return copilot.PermissionRequestResult{Kind: copilot.PermissionRequestResultKindApproved}, nil
8686
}
8787

8888
session, err := client.CreateSession(t.Context(), &copilot.SessionConfig{
@@ -117,7 +117,7 @@ func TestPermissions(t *testing.T) {
117117
ctx.ConfigureForTest(t)
118118

119119
onPermissionRequest := func(request copilot.PermissionRequest, invocation copilot.PermissionInvocation) (copilot.PermissionRequestResult, error) {
120-
return copilot.PermissionRequestResult{Kind: copilot.PermissionKindDeniedInteractivelyByUser}, nil
120+
return copilot.PermissionRequestResult{Kind: copilot.PermissionRequestResultKindDeniedInteractivelyByUser}, nil
121121
}
122122

123123
session, err := client.CreateSession(t.Context(), &copilot.SessionConfig{
@@ -162,7 +162,7 @@ func TestPermissions(t *testing.T) {
162162

163163
session, err := client.CreateSession(t.Context(), &copilot.SessionConfig{
164164
OnPermissionRequest: func(request copilot.PermissionRequest, invocation copilot.PermissionInvocation) (copilot.PermissionRequestResult, error) {
165-
return copilot.PermissionRequestResult{Kind: copilot.PermissionKindDeniedCouldNotRequestFromUser}, nil
165+
return copilot.PermissionRequestResult{Kind: copilot.PermissionRequestResultKindDeniedCouldNotRequestFromUser}, nil
166166
},
167167
})
168168
if err != nil {
@@ -212,7 +212,7 @@ func TestPermissions(t *testing.T) {
212212

213213
session2, err := client.ResumeSession(t.Context(), sessionID, &copilot.ResumeSessionConfig{
214214
OnPermissionRequest: func(request copilot.PermissionRequest, invocation copilot.PermissionInvocation) (copilot.PermissionRequestResult, error) {
215-
return copilot.PermissionRequestResult{Kind: copilot.PermissionKindDeniedCouldNotRequestFromUser}, nil
215+
return copilot.PermissionRequestResult{Kind: copilot.PermissionRequestResultKindDeniedCouldNotRequestFromUser}, nil
216216
},
217217
})
218218
if err != nil {

‎go/internal/e2e/tools_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ func TestTools(t *testing.T) {
285285
mu.Lock()
286286
permissionRequests = append(permissionRequests, request)
287287
mu.Unlock()
288-
return copilot.PermissionRequestResult{Kind: copilot.PermissionKindApproved}, nil
288+
return copilot.PermissionRequestResult{Kind: copilot.PermissionRequestResultKindApproved}, nil
289289
},
290290
})
291291
if err != nil {
@@ -341,7 +341,7 @@ func TestTools(t *testing.T) {
341341
}),
342342
},
343343
OnPermissionRequest: func(request copilot.PermissionRequest, invocation copilot.PermissionInvocation) (copilot.PermissionRequestResult, error) {
344-
return copilot.PermissionRequestResult{Kind: copilot.PermissionKindDeniedInteractivelyByUser}, nil
344+
return copilot.PermissionRequestResult{Kind: copilot.PermissionRequestResultKindDeniedInteractivelyByUser}, nil
345345
},
346346
})
347347
if err != nil {

‎go/permissions.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,6 @@ var PermissionHandler = struct {
66
ApproveAll PermissionHandlerFunc
77
}{
88
ApproveAll: func(_ PermissionRequest, _ PermissionInvocation) (PermissionRequestResult, error) {
9-
return PermissionRequestResult{Kind: PermissionKindApproved}, nil
9+
return PermissionRequestResult{Kind: PermissionRequestResultKindApproved}, nil
1010
},
1111
}

‎go/session.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,7 @@ func (s *Session) handlePermissionRequest(request PermissionRequest) (Permission
310310

311311
if handler == nil {
312312
return PermissionRequestResult{
313-
Kind: PermissionKindDeniedCouldNotRequestFromUser,
313+
Kind: PermissionRequestResultKindDeniedCouldNotRequestFromUser,
314314
}, nil
315315
}
316316

‎go/types.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -136,18 +136,18 @@ func (p *PermissionRequest) UnmarshalJSON(data []byte) error {
136136
type PermissionRequestResultKind string
137137

138138
const (
139-
// PermissionKindApproved indicates the permission was approved.
140-
PermissionKindApproved PermissionRequestResultKind = "approved"
139+
// PermissionRequestResultKindApproved indicates the permission was approved.
140+
PermissionRequestResultKindApproved PermissionRequestResultKind = "approved"
141141

142-
// PermissionKindDeniedByRules indicates the permission was denied by rules.
143-
PermissionKindDeniedByRules PermissionRequestResultKind = "denied-by-rules"
142+
// PermissionRequestResultKindDeniedByRules indicates the permission was denied by rules.
143+
PermissionRequestResultKindDeniedByRules PermissionRequestResultKind = "denied-by-rules"
144144

145-
// PermissionKindDeniedCouldNotRequestFromUser indicates the permission was denied because
145+
// PermissionRequestResultKindDeniedCouldNotRequestFromUser indicates the permission was denied because
146146
// no approval rule was found and the user could not be prompted.
147-
PermissionKindDeniedCouldNotRequestFromUser PermissionRequestResultKind = "denied-no-approval-rule-and-could-not-request-from-user"
147+
PermissionRequestResultKindDeniedCouldNotRequestFromUser PermissionRequestResultKind = "denied-no-approval-rule-and-could-not-request-from-user"
148148

149-
// PermissionKindDeniedInteractivelyByUser indicates the permission was denied interactively by the user.
150-
PermissionKindDeniedInteractivelyByUser PermissionRequestResultKind = "denied-interactively-by-user"
149+
// PermissionRequestResultKindDeniedInteractivelyByUser indicates the permission was denied interactively by the user.
150+
PermissionRequestResultKindDeniedInteractivelyByUser PermissionRequestResultKind = "denied-interactively-by-user"
151151
)
152152

153153
// PermissionRequestResult represents the result of a permission request

‎go/types_test.go‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,10 @@ func TestPermissionRequestResultKind_Constants(t *testing.T) {
1111
kind PermissionRequestResultKind
1212
expected string
1313
}{
14-
{"Approved", PermissionKindApproved, "approved"},
15-
{"DeniedByRules", PermissionKindDeniedByRules, "denied-by-rules"},
16-
{"DeniedCouldNotRequestFromUser", PermissionKindDeniedCouldNotRequestFromUser, "denied-no-approval-rule-and-could-not-request-from-user"},
17-
{"DeniedInteractivelyByUser", PermissionKindDeniedInteractivelyByUser, "denied-interactively-by-user"},
14+
{"Approved", PermissionRequestResultKindApproved, "approved"},
15+
{"DeniedByRules", PermissionRequestResultKindDeniedByRules, "denied-by-rules"},
16+
{"DeniedCouldNotRequestFromUser", PermissionRequestResultKindDeniedCouldNotRequestFromUser, "denied-no-approval-rule-and-could-not-request-from-user"},
17+
{"DeniedInteractivelyByUser", PermissionRequestResultKindDeniedInteractivelyByUser, "denied-interactively-by-user"},
1818
}
1919

2020
for _, tt := range tests {
@@ -38,10 +38,10 @@ func TestPermissionRequestResult_JSONRoundTrip(t *testing.T) {
3838
name string
3939
kind PermissionRequestResultKind
4040
}{
41-
{"Approved", PermissionKindApproved},
42-
{"DeniedByRules", PermissionKindDeniedByRules},
43-
{"DeniedCouldNotRequestFromUser", PermissionKindDeniedCouldNotRequestFromUser},
44-
{"DeniedInteractivelyByUser", PermissionKindDeniedInteractivelyByUser},
41+
{"Approved", PermissionRequestResultKindApproved},
42+
{"DeniedByRules", PermissionRequestResultKindDeniedByRules},
43+
{"DeniedCouldNotRequestFromUser", PermissionRequestResultKindDeniedCouldNotRequestFromUser},
44+
{"DeniedInteractivelyByUser", PermissionRequestResultKindDeniedInteractivelyByUser},
4545
{"Custom", PermissionRequestResultKind("custom")},
4646
}
4747

@@ -72,13 +72,13 @@ func TestPermissionRequestResult_JSONDeserialize(t *testing.T) {
7272
t.Fatalf("failed to unmarshal: %v", err)
7373
}
7474

75-
if result.Kind != PermissionKindDeniedByRules {
76-
t.Errorf("expected %q, got %q", PermissionKindDeniedByRules, result.Kind)
75+
if result.Kind != PermissionRequestResultKindDeniedByRules {
76+
t.Errorf("expected %q, got %q", PermissionRequestResultKindDeniedByRules, result.Kind)
7777
}
7878
}
7979

8080
func TestPermissionRequestResult_JSONSerialize(t *testing.T) {
81-
result := PermissionRequestResult{Kind: PermissionKindApproved}
81+
result := PermissionRequestResult{Kind: PermissionRequestResultKindApproved}
8282
data, err := json.Marshal(result)
8383
if err != nil {
8484
t.Fatalf("failed to marshal: %v", err)

0 commit comments

Comments
 (0)