Skip to content

Commit 13fea36

Browse files
Address PR review feedback
- Use 'supports versions' consistently in all error messages (all SDKs) - Move 'import inspect' to top of module (Python) - Return full PermissionRequestResult fields, not just kind (Python) - Change bare catch to catch (Exception) (C#) - Make ToolCallResponseV2.Result non-nullable (C#) - Throw for unknown session in OnPermissionRequestV2 (C#) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent b8836bf commit 13fea36

4 files changed

Lines changed: 19 additions & 18 deletions

File tree

dotnet/src/Client.cs

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -938,7 +938,7 @@ private async Task VerifyProtocolVersionAsync(Connection connection, Cancellatio
938938
if (!pingResponse.ProtocolVersion.HasValue)
939939
{
940940
throw new InvalidOperationException(
941-
$"SDK protocol version mismatch: SDK expects version {MinProtocolVersion}-{maxVersion}, " +
941+
$"SDK protocol version mismatch: SDK supports versions {MinProtocolVersion}-{maxVersion}, " +
942942
$"but server does not report a protocol version. " +
943943
$"Please update your server to ensure compatibility.");
944944
}
@@ -1346,22 +1346,15 @@ public async Task<ToolCallResponseV2> OnToolCallV2(string sessionId,
13461346

13471347
public async Task<PermissionRequestResponseV2> OnPermissionRequestV2(string sessionId, JsonElement permissionRequest)
13481348
{
1349-
var session = client.GetSession(sessionId);
1350-
if (session == null)
1351-
{
1352-
return new PermissionRequestResponseV2(new PermissionRequestResult
1353-
{
1354-
Kind = PermissionRequestResultKind.DeniedCouldNotRequestFromUser
1355-
});
1356-
}
1349+
var session = client.GetSession(sessionId)
1350+
?? throw new ArgumentException($"Unknown session {sessionId}");
13571351

13581352
try
13591353
{
13601354
var result = await session.HandlePermissionRequestAsync(permissionRequest);
13611355
return new PermissionRequestResponseV2(result);
13621356
}
1363-
catch
1364-
{
1357+
catch (Exception) {
13651358
return new PermissionRequestResponseV2(new PermissionRequestResult
13661359
{
13671360
Kind = PermissionRequestResultKind.DeniedCouldNotRequestFromUser
@@ -1489,7 +1482,7 @@ internal record HooksInvokeResponse(
14891482

14901483
// Protocol v2 backward-compatibility response types
14911484
internal record ToolCallResponseV2(
1492-
ToolResultObject? Result);
1485+
ToolResultObject Result);
14931486

14941487
internal record PermissionRequestResponseV2(
14951488
PermissionRequestResult Result);

go/client.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1082,7 +1082,7 @@ func (c *Client) verifyProtocolVersion(ctx context.Context) error {
10821082
}
10831083

10841084
if pingResult.ProtocolVersion == nil {
1085-
return fmt.Errorf("SDK protocol version mismatch: SDK expects version %d-%d, but server does not report a protocol version. Please update your server to ensure compatibility", minProtocolVersion, maxVersion)
1085+
return fmt.Errorf("SDK protocol version mismatch: SDK supports versions %d-%d, but server does not report a protocol version. Please update your server to ensure compatibility", minProtocolVersion, maxVersion)
10861086
}
10871087

10881088
serverVersion := *pingResult.ProtocolVersion

nodejs/src/client.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -806,7 +806,7 @@ export class CopilotClient {
806806

807807
if (serverVersion === undefined) {
808808
throw new Error(
809-
`SDK protocol version mismatch: SDK expects version ${MIN_PROTOCOL_VERSION}-${maxVersion}, but server does not report a protocol version. ` +
809+
`SDK protocol version mismatch: SDK supports versions ${MIN_PROTOCOL_VERSION}-${maxVersion}, but server does not report a protocol version. ` +
810810
`Please update your server to ensure compatibility.`
811811
);
812812
}

python/copilot/client.py

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
"""
1414

1515
import asyncio
16+
import inspect
1617
import os
1718
import re
1819
import subprocess
@@ -1155,7 +1156,7 @@ async def _verify_protocol_version(self) -> None:
11551156
if server_version is None:
11561157
raise RuntimeError(
11571158
"SDK protocol version mismatch: "
1158-
f"SDK expects version {MIN_PROTOCOL_VERSION}-{max_version}"
1159+
f"SDK supports versions {MIN_PROTOCOL_VERSION}-{max_version}"
11591160
", but server does not report a protocol version. "
11601161
"Please update your server to ensure compatibility."
11611162
)
@@ -1571,8 +1572,6 @@ async def _handle_tool_call_request_v2(self, params: dict) -> dict:
15711572
)
15721573

15731574
try:
1574-
import inspect
1575-
15761575
result = handler(invocation)
15771576
if inspect.isawaitable(result):
15781577
result = await result
@@ -1615,7 +1614,16 @@ async def _handle_permission_request_v2(self, params: dict) -> dict:
16151614
try:
16161615
perm_request = PermissionRequest.from_dict(permission_request)
16171616
result = await session._handle_permission_request(perm_request)
1618-
return {"result": {"kind": result.kind}}
1617+
result_payload: dict = {"kind": result.kind}
1618+
if result.rules is not None:
1619+
result_payload["rules"] = result.rules
1620+
if result.feedback is not None:
1621+
result_payload["feedback"] = result.feedback
1622+
if result.message is not None:
1623+
result_payload["message"] = result.message
1624+
if result.path is not None:
1625+
result_payload["path"] = result.path
1626+
return {"result": result_payload}
16191627
except Exception: # pylint: disable=broad-except
16201628
return {
16211629
"result": {

0 commit comments

Comments
 (0)