Skip to content

feat: protocol v3 negotiation with v2 backwards compat#702

Closed
patniko wants to merge 9 commits intomainfrom
patniko/protocol-v3-negotiation
Closed

feat: protocol v3 negotiation with v2 backwards compat#702
patniko wants to merge 9 commits intomainfrom
patniko/protocol-v3-negotiation

Conversation

@patniko
Copy link
Contributor

@patniko patniko commented Mar 7, 2026

Summary

Fixes #701 — SDK v0.1.30 fails to connect to CLI ≥ 0.0.423 / 1.0.x with SDK protocol version mismatch: SDK expects version 2, but server reports version 3.

Root Cause

Runtime PR github/copilot-agent-runtime#4243 replaced the tool.call / permission.request JSON-RPC callback model with a broadcast event model, bumping the protocol version from 2 → 3. The SDK was never updated.

Changes

Protocol negotiation (all four SDKs)

  • sdk-protocol-version.json now specifies a version range: {"version": 3, "minVersion": 2}
  • Codegen updated to emit both SDK_PROTOCOL_VERSION and MIN_SDK_PROTOCOL_VERSION
  • verifyProtocolVersion accepts a range [min, max] instead of strict equality
  • Negotiated version stored on the client instance

v2 backwards compatibility

  • Old tool.call / permission.request RPC handlers kept — they serve v2 servers and are harmless no-ops on v3

v3 broadcast event support

  • New event interception for external_tool.requested and permission.requested events, guarded by negotiatedProtocolVersion >= 3
  • v3 callbacks via session.tools.handlePendingToolCall and session.permissions.handlePendingPermissionRequest RPC methods
  • Guard prevents double-handling on v2 (where both RPC callbacks and broadcast events fire)

Scenario test fixes

Testing

  • Unit tests pass: Node (27/27), Go (18 suites), .NET builds clean
  • Zero protocol version mismatch errors across all 33 scenario tests
  • E2E/scenario failures are pre-existing infrastructure issues (CLI startup timeouts, test harness deps)

Files changed

  • sdk-protocol-version.json — version range
  • nodejs/scripts/update-protocol-version.ts — codegen for min/max
  • nodejs/src/client.ts, python/copilot/client.py, go/client.go, dotnet/src/Client.cs — negotiation + v3 handlers
  • Generated protocol version files (4 languages)
  • ~99 scenario test files — permission handler fix

patniko and others added 2 commits March 6, 2026 21:00
Add version negotiation so the SDK accepts servers reporting protocol
version 2 or 3. This fixes the incompatibility with CLI >= 0.0.423 / 1.0.x
that was introduced by the broadcast model migration (runtime PR #4243).

Changes across all four SDKs (Node, Python, Go, .NET):
- sdk-protocol-version.json now specifies version range (min: 2, max: 3)
- Codegen emits both SDK_PROTOCOL_VERSION and MIN_SDK_PROTOCOL_VERSION
- verifyProtocolVersion accepts a range instead of strict equality
- Negotiated version stored on client instance
- Old tool.call / permission.request RPC handlers kept for v2 compat
- New v3 broadcast event interception (external_tool.requested,
  permission.requested) guarded by negotiatedProtocolVersion >= 3
- v3 callbacks via session.tools.handlePendingToolCall and
  session.permissions.handlePendingPermissionRequest RPC methods

Fixes #701

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Scenarios were created before PR #554 required permission handlers on
session creation. Add approve_all / ApproveAll / PermissionHandler to
all ~100 scenario test files across TypeScript, Python, Go, and C#.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@patniko patniko requested a review from a team as a code owner March 7, 2026 05:21
Copilot AI review requested due to automatic review settings March 7, 2026 05:21
{
if (client._negotiatedProtocolVersion >= 3)
{
_ = Task.Run(() => client.HandleExternalToolRequestedAsync(sessionId, @event.Value));
// v3: permission.requested - handle via RPC callback in addition to event dispatch
if (client._negotiatedProtocolVersion >= 3 && eventType == "permission.requested")
{
_ = Task.Run(() => client.HandlePermissionRequestedEventAsync(sessionId, @event.Value));
Comment on lines +1268 to +1280
catch (Exception)
{
try
{
if (_jsonRpc is { } rpc && requestId != null)
{
await InvokeRpcAsync<JsonElement>(rpc, "session.tools.handlePendingToolCall",
[new HandlePendingToolCallRequest(sessionId, requestId, Error: "Internal error handling tool call")],
CancellationToken.None);
}
}
catch { /* Connection may be closed */ }
}
CancellationToken.None);
}
}
catch { /* Connection may be closed */ }
Comment on lines +1251 to +1259
catch (Exception ex)
{
resultObj = new ToolResultObject
{
TextResultForLlm = "Invoking this tool produced an error. Detailed information is not available.",
ResultType = "failure",
Error = ex.Message
};
}
Comment on lines +1306 to +1321
catch (Exception)
{
try
{
if (_jsonRpc is { } rpc && requestId != null)
{
var deniedResult = new PermissionRequestResult
{
Kind = PermissionRequestResultKind.DeniedCouldNotRequestFromUser
};
await InvokeRpcAsync<JsonElement>(rpc, "session.permissions.handlePendingPermissionRequest",
[new HandlePendingPermissionRequestRequest(sessionId, requestId, deniedResult)], CancellationToken.None);
}
}
catch { /* Connection may be closed */ }
}
[new HandlePendingPermissionRequestRequest(sessionId, requestId, deniedResult)], CancellationToken.None);
}
}
catch { /* Connection may be closed */ }
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the multi-language Copilot SDKs to negotiate SDK protocol v3 with backwards compatibility for v2, and adds v3 broadcast-event handling for tool and permission requests. This unblocks SDK clients from connecting to newer Copilot CLI versions while maintaining compatibility with older servers.

Changes:

  • Switch protocol validation from strict equality to a supported range (min/max) and store the negotiated protocol version on the client.
  • Add v3 broadcast event handling for external_tool.requested and permission.requested with version gating.
  • Update scenario fixtures across languages to always provide an onPermissionRequest/equivalent handler.

Reviewed changes

Copilot reviewed 111 out of 113 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
sdk-protocol-version.json Defines protocol max/min range (v3 max, v2 min).
nodejs/scripts/update-protocol-version.ts Generates min/max protocol constants for all SDKs.
nodejs/src/sdkProtocolVersion.ts Exposes MIN/MAX protocol constants and getters.
nodejs/src/client.ts Implements range negotiation and v3 broadcast tool/permission handling.
nodejs/package.json Bumps bundled CLI dependency to @github/copilot@^1.0.1.
nodejs/package-lock.json Updates lockfile to the resolved @github/copilot 1.0.x artifacts.
python/copilot/sdk_protocol_version.py Adds MIN protocol constant and getters.
python/copilot/client.py Implements range negotiation and v3 broadcast tool/permission handling.
go/sdk_protocol_version.go Adds MIN protocol constant and getters.
go/client.go Implements range negotiation and v3 broadcast tool/permission handling.
dotnet/src/SdkProtocolVersion.cs Adds MIN protocol constant and getters.
dotnet/src/Client.cs Implements range negotiation and v3 broadcast tool/permission handling.
test/scenarios/transport/tcp/typescript/src/index.ts Adds approveAll permission handler to session creation.
test/scenarios/transport/tcp/python/main.py Adds PermissionHandler.approve_all to session creation.
test/scenarios/transport/tcp/go/main.go Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/transport/tcp/csharp/Program.cs Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/transport/stdio/typescript/src/index.ts Adds approveAll permission handler to session creation.
test/scenarios/transport/stdio/python/main.py Adds PermissionHandler.approve_all to session creation.
test/scenarios/transport/stdio/go/main.go Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/transport/stdio/csharp/Program.cs Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/transport/reconnect/typescript/src/index.ts Adds approveAll permission handler to session creation(s).
test/scenarios/transport/reconnect/python/main.py Adds PermissionHandler.approve_all to session creation(s).
test/scenarios/transport/reconnect/go/main.go Adds PermissionHandler.ApproveAll to session creation(s).
test/scenarios/transport/reconnect/csharp/Program.cs Adds PermissionHandler.ApproveAll to session creation(s).
test/scenarios/tools/tool-filtering/typescript/src/index.ts Adds approveAll permission handler to session creation.
test/scenarios/tools/tool-filtering/python/main.py Adds PermissionHandler.approve_all to session creation.
test/scenarios/tools/tool-filtering/go/main.go Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/tools/tool-filtering/csharp/Program.cs Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/tools/no-tools/typescript/src/index.ts Adds approveAll permission handler to session creation.
test/scenarios/tools/no-tools/python/main.py Adds PermissionHandler.approve_all to session creation.
test/scenarios/tools/no-tools/go/main.go Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/tools/no-tools/csharp/Program.cs Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/tools/mcp-servers/typescript/src/index.ts Adds approveAll permission handler to session creation.
test/scenarios/tools/mcp-servers/python/main.py Adds PermissionHandler.approve_all to session config.
test/scenarios/tools/mcp-servers/go/main.go Adds PermissionHandler.ApproveAll to session config.
test/scenarios/tools/mcp-servers/csharp/Program.cs Adds PermissionHandler.ApproveAll to session config.
test/scenarios/tools/custom-agents/typescript/src/index.ts Adds approveAll permission handler to session creation.
test/scenarios/tools/custom-agents/python/main.py Adds PermissionHandler.approve_all to session creation.
test/scenarios/tools/custom-agents/go/main.go Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/tools/custom-agents/csharp/Program.cs Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/sessions/streaming/typescript/src/index.ts Adds approveAll permission handler to session creation.
test/scenarios/sessions/streaming/python/main.py Adds PermissionHandler.approve_all to session creation.
test/scenarios/sessions/streaming/go/main.go Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/sessions/streaming/csharp/Program.cs Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/sessions/session-resume/typescript/src/index.ts Adds approveAll permission handler to session creation.
test/scenarios/sessions/session-resume/python/main.py Adds PermissionHandler.approve_all to session creation.
test/scenarios/sessions/infinite-sessions/typescript/src/index.ts Adds approveAll permission handler to session creation.
test/scenarios/sessions/infinite-sessions/python/main.py Adds PermissionHandler.approve_all to session creation.
test/scenarios/sessions/infinite-sessions/go/main.go Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/sessions/infinite-sessions/csharp/Program.cs Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/sessions/concurrent-sessions/typescript/src/index.ts Adds approveAll permission handler to concurrent session creation.
test/scenarios/sessions/concurrent-sessions/python/main.py Adds PermissionHandler.approve_all to concurrent session creation.
test/scenarios/sessions/concurrent-sessions/go/main.go Adds PermissionHandler.ApproveAll to concurrent session creation.
test/scenarios/sessions/concurrent-sessions/csharp/Program.cs Adds PermissionHandler.ApproveAll to concurrent session creation.
test/scenarios/prompts/system-message/typescript/src/index.ts Adds approveAll permission handler to session creation.
test/scenarios/prompts/system-message/python/main.py Adds PermissionHandler.approve_all to session creation.
test/scenarios/prompts/system-message/go/main.go Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/prompts/system-message/csharp/Program.cs Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/prompts/reasoning-effort/typescript/src/index.ts Adds approveAll permission handler to session creation.
test/scenarios/prompts/reasoning-effort/python/main.py Adds PermissionHandler.approve_all to session creation.
test/scenarios/prompts/reasoning-effort/go/main.go Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/prompts/reasoning-effort/csharp/Program.cs Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/prompts/attachments/typescript/src/index.ts Adds approveAll permission handler to session creation.
test/scenarios/prompts/attachments/python/main.py Adds PermissionHandler.approve_all to session creation.
test/scenarios/prompts/attachments/go/main.go Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/prompts/attachments/csharp/Program.cs Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/modes/minimal/typescript/src/index.ts Adds approveAll permission handler to session creation.
test/scenarios/modes/minimal/python/main.py Adds PermissionHandler.approve_all to session creation.
test/scenarios/modes/minimal/go/main.go Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/modes/minimal/csharp/Program.cs Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/modes/default/typescript/src/index.ts Adds approveAll permission handler to session creation.
test/scenarios/modes/default/python/main.py Adds PermissionHandler.approve_all to session creation.
test/scenarios/modes/default/go/main.go Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/modes/default/csharp/Program.cs Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/bundling/fully-bundled/typescript/src/index.ts Adds approveAll permission handler to session creation.
test/scenarios/bundling/fully-bundled/python/main.py Adds PermissionHandler.approve_all to session creation.
test/scenarios/bundling/fully-bundled/go/main.go Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/bundling/fully-bundled/csharp/Program.cs Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/bundling/container-proxy/typescript/src/index.ts Adds approveAll permission handler to session creation.
test/scenarios/bundling/container-proxy/python/main.py Adds PermissionHandler.approve_all to session creation.
test/scenarios/bundling/container-proxy/go/main.go Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/bundling/container-proxy/csharp/Program.cs Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/bundling/app-direct-server/typescript/src/index.ts Adds approveAll permission handler to session creation.
test/scenarios/bundling/app-direct-server/python/main.py Adds PermissionHandler.approve_all to session creation.
test/scenarios/bundling/app-direct-server/go/main.go Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/bundling/app-direct-server/csharp/Program.cs Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/bundling/app-backend-to-server/typescript/src/index.ts Adds approveAll permission handler to session creation.
test/scenarios/bundling/app-backend-to-server/python/main.py Adds PermissionHandler.approve_all to session creation.
test/scenarios/bundling/app-backend-to-server/go/main.go Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/bundling/app-backend-to-server/csharp/Program.cs Adds PermissionHandler.ApproveAll to session creation (formatting needs cleanup).
test/scenarios/auth/gh-app/typescript/src/index.ts Adds approveAll permission handler to session creation.
test/scenarios/auth/gh-app/python/main.py Adds PermissionHandler.approve_all to session creation.
test/scenarios/auth/gh-app/go/main.go Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/auth/gh-app/csharp/Program.cs Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/auth/byok-openai/typescript/src/index.ts Adds approveAll permission handler to session creation.
test/scenarios/auth/byok-openai/python/main.py Adds PermissionHandler.approve_all to session creation.
test/scenarios/auth/byok-openai/go/main.go Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/auth/byok-openai/csharp/Program.cs Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/auth/byok-ollama/typescript/src/index.ts Adds approveAll permission handler to session creation.
test/scenarios/auth/byok-ollama/python/main.py Adds PermissionHandler.approve_all to session creation.
test/scenarios/auth/byok-ollama/go/main.go Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/auth/byok-ollama/csharp/Program.cs Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/auth/byok-azure/typescript/src/index.ts Adds approveAll permission handler to session creation.
test/scenarios/auth/byok-azure/python/main.py Adds PermissionHandler.approve_all to session creation.
test/scenarios/auth/byok-azure/go/main.go Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/auth/byok-azure/csharp/Program.cs Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/auth/byok-anthropic/typescript/src/index.ts Adds approveAll permission handler to session creation.
test/scenarios/auth/byok-anthropic/python/main.py Adds PermissionHandler.approve_all to session creation.
test/scenarios/auth/byok-anthropic/go/main.go Adds PermissionHandler.ApproveAll to session creation.
test/scenarios/auth/byok-anthropic/csharp/Program.cs Adds PermissionHandler.ApproveAll to session creation.
Files not reviewed (1)
  • nodejs/package-lock.json: Language not supported
Comments suppressed due to low confidence (2)

go/client.go:1576

  • Same issue as the external tool handler: this goroutine can run while Stop()/ForceStop() is clearing c.client, leading to a nil-pointer dereference when calling c.client.Request(...). Capture c.client safely or guard for nil before issuing the RPC.

	_, rpcErr := c.client.Request("session.permissions.handlePendingPermissionRequest", handlePendingPermissionParams{
		SessionID: sessionID,
		RequestID: requestID,
		Result:    result,
	})
	if rpcErr != nil {

python/copilot/client.py:1591

  • Similar to the tool broadcast handler: this code assumes _client is always available when sending session.permissions.handlePendingPermissionRequest. If stop() / force_stop() runs while this task is pending, _client may be None and this will raise, leaving the request unhandled. Guard _client before making the RPC call (and in the exception fallback).
            result = await session._handle_permission_request(permission_request)
            await self._client.request(
                "session.permissions.handlePendingPermissionRequest",
                {
                    "sessionId": session_id,
                    "requestId": request_id,
                    "result": result,
                },
            )

Comment on lines +1516 to +1522
}
_, err := c.client.Request("session.tools.handlePendingToolCall", handlePendingToolCallParams{
SessionID: sessionID,
RequestID: requestID,
Result: result,
})
if err != nil {
Copy link

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These v3 broadcast handlers run in goroutines and call c.client.Request(...) without ensuring c.client is non-nil. Stop() / ForceStop() can set c.client = nil concurrently, which can cause a nil-pointer panic here. Consider capturing the client pointer under a lock before starting the goroutine, or check for nil at call time and return early.

This issue also appears on line 1570 of the same file.

Copilot uses AI. Check for mistakes.
Comment on lines 29 to 33
await using var session = await client.CreateSessionAsync(new SessionConfig
{
OnPermissionRequest = PermissionHandler.ApproveAll,
Model = "claude-haiku-4.5",
});
Copy link

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The OnPermissionRequest initializer line is mis-indented relative to the surrounding object initializer. This makes the file look unformatted and may trip formatting/lint checks; please run the formatter (or align this property with the others).

Copilot uses AI. Check for mistakes.
Comment on lines +1384 to +1405
).data;
if (!data || !data.requestId || !data.toolName) {
return;
}

const session = this.sessions.get(sessionId);
if (!session) {
return;
}

const handler = session.getToolHandler(data.toolName);
try {
let result: ToolResult;
if (!handler) {
result = this.buildUnsupportedToolResult(data.toolName);
} else {
const response = await this.executeToolCall(handler, {
sessionId,
toolCallId: data.toolCallId,
toolName: data.toolName,
arguments: data.arguments,
});
Copy link

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the v3 broadcast handler, toolCallId is treated as a required string (passed into executeToolCall and likely required by the server RPC), but the guard only checks requestId and toolName. If the event omits toolCallId, this will forward undefined and can break tool execution / the pending-tool response. Validate toolCallId explicitly (or default it safely) before calling executeToolCall / handlePendingToolCall.

Copilot uses AI. Check for mistakes.
Comment on lines +1443 to +1453
if self._negotiated_protocol_version >= 3:
event_type = event_dict.get("type")
if event_type == "external_tool.requested":
asyncio.ensure_future(
Copy link

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The TCP handle_notification path reads _sessions without taking _sessions_lock (unlike the stdio path). Since sessions are mutated under _sessions_lock in other methods (create/stop/etc.), this can introduce races/inconsistent reads. Use with self._sessions_lock: when fetching the session here as well.

Copilot uses AI. Check for mistakes.
Comment on lines +1539 to +1546
await self._client.request(
"session.tools.handlePendingToolCall",
{
"sessionId": session_id,
"requestId": request_id,
"result": result,
},
)
Copy link

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These v3 broadcast handlers call self._client.request(...) without checking that _client is still set. Since stop() / force_stop() can set _client to None while scheduled tasks are still pending, this can raise and also prevent sending a best-effort response to the server. Add an early guard (e.g., if _client is None return) before attempting to send the handlePendingToolCall RPC (and similarly in the error fallback).

This issue also appears on line 1583 of the same file.

Copilot uses AI. Check for mistakes.
go/client.go Outdated
sessions map[string]*Session
sessionsMux sync.Mutex
isExternalServer bool
isExternalServer bool
Copy link

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new fields in the Client struct have inconsistent spacing/alignment (isExternalServer bool), which indicates gofmt hasn't been applied to this file. Please run gofmt so formatting is consistent and to avoid CI lint failures in Go formatting checks.

Suggested change
isExternalServer bool
isExternalServer bool

Copilot uses AI. Check for mistakes.
@github-actions
Copy link
Contributor

github-actions bot commented Mar 7, 2026

✅ Cross-SDK Consistency Review: PASSED

This PR demonstrates excellent consistency across all four SDK implementations (Node.js/TypeScript, Python, Go, and .NET). The protocol v3 negotiation and backwards compatibility features are implemented uniformly.

Consistency Verification

✅ Protocol Version Management

All four SDKs correctly implement:

  • Range-based negotiation: [minVersion=2, maxVersion=3] from sdk-protocol-version.json
  • Consistent constants: SDK_PROTOCOL_VERSION = 3, MIN_SDK_PROTOCOL_VERSION = 2
  • Identical validation logic: serverVersion < minVersion || serverVersion > maxVersion
  • Negotiated version storage: negotiatedProtocolVersion / _negotiated_protocol_version / negotiatedProtocolVersion / _negotiatedProtocolVersion

Files verified:

  • nodejs/src/sdkProtocolVersion.ts
  • python/copilot/sdk_protocol_version.py
  • go/sdk_protocol_version.go
  • dotnet/src/SdkProtocolVersion.cs

✅ v3 Event Handling

All SDKs implement the same broadcast event model:

  • Guard condition: if (negotiatedProtocolVersion >= 3) before handling broadcast events
  • Event types: external_tool.requested and permission.requested
  • RPC callbacks: session.tools.handlePendingToolCall and session.permissions.handlePendingPermissionRequest

Implementation locations:

  • Node: client.ts:1356-1367 (handleBroadcastEvent)
  • Python: client.py:1357-1366 (v3 protocol interception)
  • Go: client.go:1320-1322 (handleBroadcastEvent)
  • .NET: Client.cs:1366-1386 (OnSessionEvent v3 handling)

✅ v2 Backwards Compatibility

All SDKs retain the v2 RPC request handlers:

  • tool.call RPC handler (Node: L1292, Python: L1373, Go: L1300, .NET: L1140)
  • permission.request RPC handler (Node: L1298, Python: L1374, Go: L1301, .NET: L1141)

This ensures zero breaking changes for users on CLI v2 servers.

✅ API Naming Consistency

Method/property names follow language conventions appropriately:

  • Node/TypeScript: negotiatedProtocolVersion (camelCase)
  • Python: _negotiated_protocol_version (snake_case, private)
  • Go: negotiatedProtocolVersion (camelCase, unexported)
  • .NET: _negotiatedProtocolVersion (camelCase, private field)

✅ Codegen Consistency

The update-protocol-version.ts script generates all four language files from a single source (sdk-protocol-version.json), ensuring version numbers never drift.

Summary

This PR maintains 100% feature parity across all SDKs. The protocol negotiation, v3 broadcast handling, and v2 backwards compatibility are implemented identically (accounting for language idioms). No additional changes needed.

Great work on maintaining consistency! 🎉

Generated by SDK Consistency Review Agent for issue #702 ·

patniko and others added 6 commits March 6, 2026 21:35
gofmt fixes struct field alignment in Client type, prettier
reflows handlePermissionRequested signature.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The main verify.sh now auto-discovers COPILOT_CLI_PATH from the Node
SDK's bundled @github/copilot package using ESM import.meta.resolve.
This fixes TS, Python, and C# scenarios that were failing with
ERR_MODULE_NOT_FOUND / 'CLI not found' because the bundled CLI binary
wasn't resolvable from the scenario directory's node_modules.

Also fix 6 individual scenario verify.sh scripts that used the broken
require.resolve('@github/copilot') CJS pattern (fails due to package
exports restrictions) with the working ESM resolution.

Go scenarios were unaffected — they embed the CLI binary at compile time.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The ty type checker correctly identifies that self._client can be None.
Add explicit guards before the .request() calls in the v3 broadcast
event handlers.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The v3 server broadcasts new event types like permission.completed that
the generated session_events.py can't deserialize yet. Wrap
session_event_from_dict in try/except so unknown events are silently
skipped instead of crashing the notification handler. This affects both
the stdio and TCP connection handlers.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Regenerate session-events and RPC types from the @github/copilot@1.0.2
schemas. This adds the new v3 event types (external_tool.requested,
permission.completed, external_tool.completed) and new session-scoped
RPC methods (tools.handlePendingToolCall, permissions.handlePendingPermissionRequest).

Fix codegen naming collision: both server and session APIs have a
'tools' group. Prefix session API group classes with 'Session' to avoid
duplicate ToolsApi/ToolsRpcApi types (Go and C# codegen).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Contributor

github-actions bot commented Mar 7, 2026

✅ Cross-SDK Consistency Review

I've completed a comprehensive review of this PR across all four SDK implementations (Node.js/TypeScript, Python, Go, and .NET). The changes maintain excellent consistency across the multi-language SDK implementations.

Verified Consistent Implementation

All four SDKs implement the protocol v3 negotiation and backwards compatibility features in a parallel manner:

✅ Protocol Version Negotiation

  • Shared source of truth: sdk-protocol-version.json defines {version: 3, minVersion: 2}
  • Consistent constants: All SDKs generate/define both:
    • SDK_PROTOCOL_VERSION / SdkProtocolVersion / etc. = 3 (max)
    • MIN_SDK_PROTOCOL_VERSION / MinSdkProtocolVersion / etc. = 2 (min)
  • Consistent validation: All SDKs check serverVersion >= min && serverVersion <= max
  • Consistent error messages: All report "SDK supports versions {min}-{max}, but server reports version {serverVersion}"
  • State tracking: All SDKs store negotiatedProtocolVersion / _negotiated_protocol_version / etc.

✅ V2 Backwards Compatibility

All SDKs retain the original v2 RPC handlers (verified in code):

  • Node.js: connection.onRequest("tool.call", ...) and connection.onRequest("permission.request", ...)
  • Python: set_request_handler("tool.call", ...) and set_request_handler("permission.request", ...)
  • Go: SetRequestHandler("tool.call", ...) and SetRequestHandler("permission.request", ...)
  • .NET: AddLocalRpcMethod("tool.call", ...) and AddLocalRpcMethod("permission.request", ...)

✅ V3 Broadcast Event Support

All SDKs implement v3 event interception guarded by protocol version:

Guard condition (all SDKs):

if (negotiatedProtocolVersion >= 3) { ... }

V3 Event Handlers (parallel implementations):

  • Node.js: handleExternalToolRequested() and handlePermissionRequested()
  • Python: _handle_external_tool_requested() and _handle_permission_requested_event()
  • Go: handleExternalToolRequestedEvent() and handlePermissionRequestedEvent()
  • .NET: HandleExternalToolRequestedAsync() and HandlePermissionRequestedEventAsync()

RPC callback methods (identical across all SDKs):

  • session.tools.handlePendingToolCall
  • session.permissions.handlePendingPermissionRequest

API Design Consistency

The naming follows proper language conventions:

  • Node.js/TypeScript: camelCase (negotiatedProtocolVersion, handleExternalToolRequested)
  • Python: snake_case (_negotiated_protocol_version, _handle_external_tool_requested)
  • Go: PascalCase for exports, camelCase for private (negotiatedProtocolVersion, handleExternalToolRequestedEvent)
  • .NET: PascalCase (_negotiatedProtocolVersion, HandleExternalToolRequestedAsync)

Summary

This PR is a model example of cross-SDK consistency:

  • ✅ All four SDKs updated in parallel
  • ✅ Feature parity maintained across all languages
  • ✅ Backwards compatibility preserved uniformly
  • ✅ API naming follows language-specific conventions appropriately
  • ✅ Error messages and behavior are consistent

No consistency issues found. The implementation correctly balances language-specific idioms with consistent cross-SDK behavior. Great work! 🎉

Generated by SDK Consistency Review Agent for issue #702 ·

@patniko patniko closed this Mar 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Latest Go client incompatible with latest CLI

2 participants