Send GitHub telemetry forwarding opt-in on the connect handshake by stephentoub · Pull Request #1902 · github/copilot-sdk

stephentoub · 2026-07-03T15:44:37Z

Why

The runtime moved the enableGitHubTelemetryForwarding opt-in from session.create to the connection-level connect handshake, so it can forward the first session's un-replayable session.start event. The SDKs still only sent the flag on session.create/session.resume, so against a runtime built from that change nothing opted the connection in and GitHub telemetry forwarding never fired. This surfaced as the telemetry-forwarding E2E test timing out in copilot-agent-runtime CI (which builds the runtime from HEAD), across the legs for all six SDKs.

This is not a runtime bug; it is an intentional API refinement. The fix belongs in the SDKs.

What

Dual-send the flag across all six SDKs (Node, C#, Go, Python, Rust, Java): send enableGitHubTelemetryForwarding: true on the connect handshake when a GitHub telemetry handler is registered, in addition to keeping the existing session.create/session.resume send.

This is backward and forward compatible:

New runtimes read the flag on connect and ignore the (now redundant) session.create copy.
Older CLIs (the currently published 1.0.69-0) still read it on session.create and ignore the unknown connect field.

Each SDK also gets unit tests asserting the connect request carries the flag when a handler is registered and omits it otherwise.

Notes for reviewers

The wire field must serialize as exactly enableGitHubTelemetryForwarding (capital "H" in "Hub"). Rust's rename_all = "camelCase" would mangle it to a lowercase h, so that field uses an explicit #[serde(rename = ...)].
Rust and Java build the connect params inline rather than editing generated code, per their respective "do not hand-edit generated types" boundaries. The other four hand-apply the field to the generated ConnectRequest type, since the post-change CLI schema is not published yet and regeneration would not add it.

Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com

The runtime moved the `enableGitHubTelemetryForwarding` opt-in from `session.create` to the connection-level `connect` handshake, so it can forward the first session's un-replayable `session.start` event. SDKs only sent the flag on session.create/resume, so against a post-move runtime nothing opted the connection in and GitHub telemetry forwarding timed out. Dual-send the flag across all six SDKs: send it on `connect` (when a GitHub telemetry handler is registered) in addition to the existing session.create/resume send. This is backward and forward compatible; unknown fields are ignored by both old and new runtimes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

This PR updates all six SDKs (Node, .NET, Go, Python, Rust, Java) to opt into GitHub telemetry forwarding at the connection-level connect handshake (in addition to the existing session.create/session.resume send), matching the runtime’s moved behavior so the first session’s session.start can be forwarded.

Changes:

Add enableGitHubTelemetryForwarding: true to the connect request when a GitHub telemetry handler/callback is registered (while keeping the session-level send for backward compatibility).
Update generated/hand-built connect request types/params as needed per language (including Rust serde(rename=...) to preserve exact casing).
Add/extend unit tests in each SDK to assert the connect handshake includes (or omits) the flag based on handler registration.

Show a summary per file

File	Description
rust/tests/session_test.rs	Adds tests asserting `connect` includes/omits `enableGitHubTelemetryForwarding`.
rust/src/lib.rs	Builds `connect` params inline to include the telemetry opt-in without editing generated types.
python/test_client.py	Adds connect-flag tests (but currently appears to merge in the prior telemetry-routing test body).
python/copilot/generated/rpc.py	Extends `_ConnectRequest` to serialize `enableGitHubTelemetryForwarding`.
python/copilot/client.py	Sends the connect-level opt-in when `on_github_telemetry` is set.
nodejs/test/client.test.ts	Adds tests for connect-level opt-in behavior.
nodejs/src/generated/rpc.ts	Extends `ConnectRequest` with `enableGitHubTelemetryForwarding?: boolean`.
nodejs/src/client.ts	Sends the connect-level opt-in when `onGitHubTelemetry` is set.
java/src/test/java/com/github/copilot/GitHubTelemetryTest.java	Captures and asserts connect params include/omit the flag.
java/src/main/java/com/github/copilot/CopilotClient.java	Sends the connect-level opt-in by building connect params inline.
go/rpc/zrpc.go	Extends `ConnectRequest` with `EnableGitHubTelemetryForwarding`.
go/client.go	Sends the connect-level opt-in when `OnGitHubTelemetry` is set.
go/client_test.go	Adds tests asserting connect carries/omits the flag.
dotnet/test/Unit/GitHubTelemetryTests.cs	Adds connect-level opt-in tests (one assertion can be tightened to require omission).
dotnet/src/Generated/Rpc.cs	Extends `ConnectRequest` with `EnableGitHubTelemetryForwarding`.
dotnet/src/Client.cs	Sends the connect-level opt-in when `OnGitHubTelemetry` is set.

Review details

Files not reviewed (1)

go/rpc/zrpc.go: Generated file

Files reviewed: 12/16 changed files
Comments generated: 2
Review effort level: Low

- python/test_client.py: restore test_event_routes_to_handler as its own test method; the connect-omit test had accidentally absorbed the telemetry dispatch body, so keep it limited to the connect assertion. - dotnet/test/Unit/GitHubTelemetryTests.cs: tighten Connect_Does_Not_Opt_In_Without_Handler to require the flag be absent or null, so it fails if `false` is ever sent (matches the other SDKs). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Move connect telemetry forwarding onto SDK-owned handshake payloads so generated RPC files stay aligned with the published schema while preserving the wire field name. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

github-actions · 2026-07-03T21:14:18Z

Cross-SDK Consistency Review ✅

All six SDK implementations (Node.js, Python, Go, .NET, Java, Rust) have been updated consistently in this PR. Here's a summary of the cross-language review:

What was checked

Aspect	Node.js	Python	Go	.NET	Java	Rust
Sends flag on `connect` when handler registered	✅	✅	✅	✅	✅	✅
Omits/skips flag on `connect` when no handler	✅	✅	✅	⚠️	✅	✅
Keeps flag on `session.create`/`session.resume`	✅	✅	✅	✅	✅	✅
Wire field name: `enableGitHubTelemetryForwarding`	✅	✅	✅	✅	✅	✅ (explicit `rename`)
Tests: with-handler + without-handler cases	✅	✅	✅	✅	✅	✅

Minor observation: .NET null vs. absent

All SDKs except .NET strictly omit enableGitHubTelemetryForwarding from the serialized payload when no handler is registered (Go uses *bool + omitempty; Python/Node/Java only insert the key when the handler is present; Rust uses skip_serializing_if = "Option::is_none"). The .NET ConnectHandshakeRequest record passes null for the field:

internal record ConnectHandshakeRequest(
    string? Token,
    [property: JsonPropertyName("enableGitHubTelemetryForwarding")] bool? EnableGitHubTelemetryForwarding = null);

Depending on the JsonSerializerOptions in use, System.Text.Json may serialize this as "enableGitHubTelemetryForwarding": null rather than omitting the key entirely. The corresponding test reflects this ambiguity by accepting either:

Assert.True(!present || flag.ValueKind == JsonValueKind.Null, ...);

This is functionally equivalent — the runtime should treat null and absent the same way (no opt-in). If the context does have DefaultIgnoreCondition = WhenWritingNull, the field is already omitted and this is a non-issue. If not, consider adding [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] to the property to bring .NET strictly in line with the other SDKs. Either way, this doesn't affect correctness.

Overall

This is a well-executed cross-SDK change. The implementation strategy is appropriately tailored per language (inline struct in Rust/Java to avoid editing generated code; direct field on a new record in .NET; inline dict/object in the others), and the dual-send approach correctly maintains backward compatibility with older CLIs.

Generated by SDK Consistency Review Agent for issue #1902 · sonnet46 1.3M · ◷

Copilot AI review requested due to automatic review settings July 3, 2026 15:44

stephentoub requested a review from a team as a code owner July 3, 2026 15:44

Copilot started reviewing on behalf of stephentoub July 3, 2026 15:45 View session

Copilot AI reviewed Jul 3, 2026

View reviewed changes

Comment thread python/test_client.py

Comment thread dotnet/test/Unit/GitHubTelemetryTests.cs

This comment has been minimized.

Sign in to view

Avoid hand-editing generated connect types

be62939

Move connect telemetry forwarding onto SDK-owned handshake payloads so generated RPC files stay aligned with the published schema while preserving the wire field name. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Send GitHub telemetry forwarding opt-in on the connect handshake#1902

Send GitHub telemetry forwarding opt-in on the connect handshake#1902
stephentoub wants to merge 3 commits into
mainfrom
stephentoub-fix-sdk-test-failures

stephentoub commented Jul 3, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

github-actions Bot commented Jul 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

stephentoub commented Jul 3, 2026

Why

What

Notes for reviewers

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Review details

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

github-actions Bot commented Jul 3, 2026

Cross-SDK Consistency Review ✅

What was checked

Minor observation: .NET null vs. absent

Overall

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants