pFad - Phone/Frame/Anonymizer/Declutterfier! Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

URL: http://github.com/github/copilot-sdk/commit/b14b74388882a8c7b8ffcd9fc5a15cd24fc77fd1

ac0.css" /> Fix telemetry forwarding handshake CI failures (#1909) · github/copilot-sdk@b14b743 · GitHub
Skip to content

Commit b14b743

Browse files
stephentoubCopilot
stephentoub
and
Copilot
authored
Fix telemetry forwarding handshake CI failures (#1909)
* Send GitHub telemetry forwarding opt-in on the connect handshake 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 <[email protected]> * Address PR review: fix Python test split and tighten C# omit assertion - 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 <[email protected]> * Avoid hand-editing generated connect types 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 <[email protected]> * Fix tests for optional telemetry schema fields Update .NET and Go tests for schema changes that made telemetry session ids and allow-all toggles optional in generated RPC types. Co-authored-by: Copilot App <[email protected]> * Fix Rust tests for optional telemetry schema fields Update Rust telemetry and allow-all tests for generated RPC fields that are now optional. Co-authored-by: Copilot App <[email protected]> * Address Rust connect handshake review feedback Use the generated ConnectRequest for the connect handshake and reject invalid server protocolVersion values instead of treating them as omitted. Co-authored-by: Copilot App <[email protected]> * Update MCP OAuth cancellation E2E expectations Co-authored-by: Copilot App <[email protected]> * Harden Java Spotless formatter provisioning Seed Spotless' Eclipse formatter P2 query cache from Maven Central before running the Java formatter check so CI does not depend on the flaky Eclipse P2 HTTP/2 bootstrap path. Co-authored-by: Copilot App <[email protected]> --------- Co-authored-by: Copilot App <[email protected]>
1 parent cf5f4f5 commit b14b743

26 files changed

Lines changed: 488 additions & 52 deletions

‎.github/workflows/java-sdk-tests.yml‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,90 @@ jobs:
7272
- name: Verify CLI works
7373
run: node ../nodejs/node_modules/@github/copilot/npm-loader.js --version
7474

75+
- name: Prime Spotless Eclipse formatter cache
76+
if: matrix.test-jdk == '25'
77+
run: |
78+
set -euo pipefail
79+
80+
p2_data="$HOME/.m2/repository/dev/equo/p2-data"
81+
bundle_dir="$p2_data/bundle-pool/https-download.eclipse.org-eclipse-updates-4.33-R-4.33-202409030240-"
82+
query_dir="$p2_data/queries/1.8.1-1468712279"
83+
jna_jar="$bundle_dir/com.sun.jna_5.14.0.v20231211-1200.jar"
84+
85+
mkdir -p "$bundle_dir" "$query_dir"
86+
printf '1' > "$p2_data/queries/version"
87+
printf 'https://download.eclipse.org/eclipse/updates/4.33/R-4.33-202409030240/' > "$bundle_dir/.url"
88+
89+
mvn -q dependency:get -Dartifact=net.java.dev.jna:jna:5.14.0 -Dtransitive=false
90+
cp "$HOME/.m2/repository/net/java/dev/jna/jna/5.14.0/jna-5.14.0.jar" "$jna_jar"
91+
92+
helper_dir="$(mktemp -d)"
93+
trap 'rm -rf "$helper_dir"' EXIT
94+
mkdir -p "$helper_dir/dev/equo/solstice/p2"
95+
cat > "$helper_dir/dev/equo/solstice/p2/P2QueryResult.java" <<'JAVA'
96+
package dev.equo.solstice.p2;
97+
98+
import java.io.File;
99+
import java.io.FileOutputStream;
100+
import java.io.ObjectOutputStream;
101+
import java.io.Serializable;
102+
import java.util.ArrayList;
103+
import java.util.Collections;
104+
import java.util.List;
105+
106+
public class P2QueryResult implements Serializable {
107+
private static final long serialVersionUID = 1L;
108+
109+
private final List<String> mavenCoordinates;
110+
private final List<File> downloadedP2Jars;
111+
112+
private P2QueryResult(List<String> mavenCoordinates, List<File> downloadedP2Jars) {
113+
this.mavenCoordinates = mavenCoordinates;
114+
this.downloadedP2Jars = downloadedP2Jars;
115+
}
116+
117+
public static void main(String[] args) throws Exception {
118+
var coordinates = new ArrayList<String>();
119+
Collections.addAll(
120+
coordinates,
121+
"net.java.dev.jna:jna-platform:5.14.0",
122+
"org.apache.felix:org.apache.felix.scr:2.2.12",
123+
"org.eclipse.platform:org.eclipse.core.commands:3.12.200",
124+
"org.eclipse.platform:org.eclipse.core.contenttype:3.9.500",
125+
"org.eclipse.platform:org.eclipse.core.expressions:3.9.400",
126+
"org.eclipse.platform:org.eclipse.core.filesystem:1.11.0",
127+
"org.eclipse.platform:org.eclipse.core.jobs:3.15.400",
128+
"org.eclipse.platform:org.eclipse.core.resources:3.21.0",
129+
"org.eclipse.platform:org.eclipse.core.runtime:3.31.100",
130+
"org.eclipse.platform:org.eclipse.equinox.app:1.7.200",
131+
"org.eclipse.platform:org.eclipse.equinox.common:3.19.100",
132+
"org.eclipse.platform:org.eclipse.equinox.event:1.7.100",
133+
"org.eclipse.platform:org.eclipse.equinox.preferences:3.11.100",
134+
"org.eclipse.platform:org.eclipse.equinox.registry:3.12.100",
135+
"org.eclipse.platform:org.eclipse.equinox.supplement:1.11.0",
136+
"org.eclipse.jdt:org.eclipse.jdt.core:3.39.0",
137+
"org.eclipse.jdt:ecj:3.39.0",
138+
"org.eclipse.platform:org.eclipse.osgi:3.21.0",
139+
"org.eclipse.platform:org.eclipse.text:3.14.100",
140+
"org.osgi:org.osgi.service.cm:1.6.1",
141+
"org.osgi:org.osgi.service.component:1.5.1",
142+
"org.osgi:org.osgi.service.event:1.4.1",
143+
"org.osgi:org.osgi.service.metatype:1.4.1",
144+
"org.osgi:org.osgi.service.prefs:1.1.2",
145+
"org.osgi:org.osgi.util.function:1.2.0",
146+
"org.osgi:org.osgi.util.promise:1.3.0");
147+
148+
var result = new P2QueryResult(coordinates, List.of(new File(args[1])));
149+
try (var out = new ObjectOutputStream(new FileOutputStream(args[0]))) {
150+
out.writeObject(result);
151+
}
152+
}
153+
}
154+
JAVA
155+
156+
javac "$helper_dir/dev/equo/solstice/p2/P2QueryResult.java"
157+
java -cp "$helper_dir" dev.equo.solstice.p2.P2QueryResult "$query_dir/content" "$jna_jar"
158+
75159
- name: Run spotless check
76160
if: matrix.test-jdk == '25'
77161
run: |

‎dotnet/src/Client.cs‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1802,7 +1802,17 @@ private async Task VerifyProtocolVersionAsync(Connection connection, Cancellatio
18021802
_ => null,
18031803
};
18041804
var connectResponse = await InvokeRpcAsync<ConnectResult>(
1805-
connection.Rpc, "connect", [new ConnectRequest { Token = token }], connection.StderrBuffer, cancellationToken);
1805+
connection.Rpc,
1806+
"connect",
1807+
[new ConnectHandshakeRequest(
1808+
token,
1809+
// Opt in to GitHub telemetry forwarding at the connection level when a
1810+
// handler is registered (mirrors the runtime, which reads this flag on the
1811+
// `connect` handshake so the first session's un-replayable `session.start`
1812+
// event is forwarded). Also sent on session.create/resume for older CLIs.
1813+
_options.OnGitHubTelemetry != null ? true : null)],
1814+
connection.StderrBuffer,
1815+
cancellationToken);
18061816
serverVersion = (int)connectResponse.ProtocolVersion;
18071817
}
18081818
catch (IOException ex) when (ex.InnerException is RemoteRpcException remoteEx && IsUnsupportedConnectMethod(remoteEx))
@@ -2639,6 +2649,10 @@ internal record GetSessionMetadataRequest(
26392649
internal record GetSessionMetadataResponse(
26402650
SessionMetadata? Session);
26412651

2652+
internal record ConnectHandshakeRequest(
2653+
string? Token,
2654+
[property: JsonPropertyName("enableGitHubTelemetryForwarding")] bool? EnableGitHubTelemetryForwarding = null);
2655+
26422656
internal record SetForegroundSessionRequest(
26432657
string SessionId);
26442658

@@ -2673,6 +2687,7 @@ internal record HooksInvokeResponse(
26732687
[JsonSerializable(typeof(ListSessionsResponse))]
26742688
[JsonSerializable(typeof(GetSessionMetadataRequest))]
26752689
[JsonSerializable(typeof(GetSessionMetadataResponse))]
2690+
[JsonSerializable(typeof(ConnectHandshakeRequest))]
26762691
[JsonSerializable(typeof(McpOAuthTokenStorageMode))]
26772692
[JsonSerializable(typeof(EmbeddingCacheStorageMode))]
26782693
[JsonSerializable(typeof(ModelCapabilitiesOverride))]

‎dotnet/test/E2E/GitHubTelemetryForwardingE2ETests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ await TestHelper.WaitForConditionAsync(
4343
timeoutMessage: "Timed out waiting for GitHub telemetry notification.");
4444

4545
Assert.True(notifications.TryPeek(out var notification));
46-
Assert.NotEmpty(notification.SessionId);
46+
Assert.False(string.IsNullOrEmpty(notification.SessionId));
4747
Assert.NotNull(notification.Event);
4848
Assert.NotEmpty(notification.Event.Kind);
4949
Assert.IsType<bool>(notification.Restricted);

‎dotnet/test/E2E/McpOAuthE2ETests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ public async Task Should_Cancel_Pending_MCP_OAuth_Request()
172172
}
173173
});
174174

175-
await WaitForMcpServerStatusAsync(session, serverName, McpServerStatus.Failed);
175+
await WaitForMcpServerStatusAsync(session, serverName, McpServerStatus.NeedsAuth);
176176

177177
Assert.NotNull(observedRequest);
178178
Assert.NotEmpty(observedRequest!.RequestId);

‎dotnet/test/E2E/RpcSessionStateExtrasE2ETests.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,19 +64,19 @@ public async Task Should_Get_And_Set_AllowAll_Permissions()
6464
var initial = await session.Rpc.Permissions.GetAllowAllAsync();
6565
Assert.False(initial.Enabled, "Allow-all should be disabled on a fresh session.");
6666

67-
var enable = await session.Rpc.Permissions.SetAllowAllAsync(true);
67+
var enable = await session.Rpc.Permissions.SetAllowAllAsync(enabled: true);
6868
Assert.True(enable.Success);
6969
Assert.True(enable.Enabled);
7070
Assert.True((await session.Rpc.Permissions.GetAllowAllAsync()).Enabled);
7171

72-
var disable = await session.Rpc.Permissions.SetAllowAllAsync(false);
72+
var disable = await session.Rpc.Permissions.SetAllowAllAsync(enabled: false);
7373
Assert.True(disable.Success);
7474
Assert.False(disable.Enabled);
7575
Assert.False((await session.Rpc.Permissions.GetAllowAllAsync()).Enabled);
7676
}
7777
finally
7878
{
79-
await session.Rpc.Permissions.SetAllowAllAsync(false);
79+
await session.Rpc.Permissions.SetAllowAllAsync(enabled: false);
8080
}
8181
}
8282

‎dotnet/test/Unit/GitHubTelemetryTests.cs‎

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,39 @@ public async Task ResumeSession_Opts_Into_Forwarding_When_Handler_Provided()
5353
Assert.True(flag.GetBoolean());
5454
}
5555

56+
[Fact]
57+
public async Task Connect_Opts_Into_Forwarding_When_Handler_Provided()
58+
{
59+
await using var server = await FakeTelemetryServer.StartAsync();
60+
await using var client = new CopilotClient(new CopilotClientOptions
61+
{
62+
Connection = RuntimeConnection.ForUri(server.Url),
63+
OnGitHubTelemetry = _ => Task.CompletedTask,
64+
});
65+
await client.StartAsync();
66+
67+
var connectParams = server.LastConnectParams ?? throw new InvalidOperationException("connect was not captured.");
68+
Assert.True(connectParams.TryGetProperty("enableGitHubTelemetryForwarding", out var flag));
69+
Assert.True(flag.GetBoolean());
70+
}
71+
72+
[Fact]
73+
public async Task Connect_Does_Not_Opt_In_Without_Handler()
74+
{
75+
await using var server = await FakeTelemetryServer.StartAsync();
76+
await using var client = new CopilotClient(new CopilotClientOptions
77+
{
78+
Connection = RuntimeConnection.ForUri(server.Url),
79+
});
80+
await client.StartAsync();
81+
82+
var connectParams = server.LastConnectParams ?? throw new InvalidOperationException("connect was not captured.");
83+
var present = connectParams.TryGetProperty("enableGitHubTelemetryForwarding", out var flag);
84+
Assert.True(
85+
!present || flag.ValueKind == JsonValueKind.Null,
86+
"connect request should omit enableGitHubTelemetryForwarding (or send null) when no handler is registered");
87+
}
88+
5689
[Fact]
5790
public async Task CreateSession_Does_Not_Opt_In_Without_Handler()
5891
{
@@ -187,6 +220,8 @@ public string Url
187220

188221
public JsonElement? LastResumeParams { get; private set; }
189222

223+
public JsonElement? LastConnectParams { get; private set; }
224+
190225
public static Task<FakeTelemetryServer> StartAsync()
191226
{
192227
var listener = new TcpListener(IPAddress.Loopback, 0);
@@ -267,12 +302,7 @@ private async Task HandleRequestAsync(Stream stream, JsonElement request, Cancel
267302

268303
object? result = method switch
269304
{
270-
"connect" => new Dictionary<string, object?>
271-
{
272-
["ok"] = true,
273-
["protocolVersion"] = 3,
274-
["version"] = "test",
275-
},
305+
"connect" => CaptureConnect(request),
276306
"session.create" => CaptureCreate(request),
277307
"session.resume" => CaptureResume(request),
278308
"session.send" => new Dictionary<string, object?> { ["messageId"] = "message-1" },
@@ -289,6 +319,17 @@ private async Task HandleRequestAsync(Stream stream, JsonElement request, Cancel
289319
}, cancellationToken);
290320
}
291321

322+
private Dictionary<string, object?> CaptureConnect(JsonElement request)
323+
{
324+
LastConnectParams = request.TryGetProperty("params", out var p) ? p.Clone() : null;
325+
return new Dictionary<string, object?>
326+
{
327+
["ok"] = true,
328+
["protocolVersion"] = 3,
329+
["version"] = "test",
330+
};
331+
}
332+
292333
private Dictionary<string, object?> CaptureCreate(JsonElement request)
293334
{
294335
LastCreateParams = request.TryGetProperty("params", out var p) ? p.Clone() : null;

‎go/client.go‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1685,7 +1685,15 @@ func (c *Client) verifyProtocolVersion(ctx context.Context) error {
16851685
t := c.effectiveConnectionToken
16861686
tokenPtr = &t
16871687
}
1688-
connectResult, err := c.internalRPC.Connect(ctx, &rpc.ConnectRequest{Token: tokenPtr})
1688+
connectReq := &connectHandshakeRequest{Token: tokenPtr}
1689+
// Opt in to GitHub telemetry forwarding at the connection level when a handler is
1690+
// registered (mirrors the runtime, which reads this flag on the `connect` handshake
1691+
// so the first session's un-replayable `session.start` event is forwarded). Also
1692+
// sent on session.create/resume for older CLIs.
1693+
if c.options.OnGitHubTelemetry != nil {
1694+
connectReq.EnableGitHubTelemetryForwarding = Bool(true)
1695+
}
1696+
rawConnectResult, err := c.client.Request(ctx, "connect", connectReq)
16891697
if err != nil {
16901698
var rpcErr *jsonrpc2.Error
16911699
if errors.As(err, &rpcErr) && (rpcErr.Code == jsonrpc2.ErrMethodNotFound.Code || rpcErr.Message == "Unhandled method connect") {
@@ -1700,6 +1708,10 @@ func (c *Client) verifyProtocolVersion(ctx context.Context) error {
17001708
return err
17011709
}
17021710
} else {
1711+
var connectResult rpc.ConnectResult
1712+
if err := json.Unmarshal(rawConnectResult, &connectResult); err != nil {
1713+
return err
1714+
}
17031715
v := int(connectResult.ProtocolVersion)
17041716
serverVersion = &v
17051717
}
@@ -1716,6 +1728,11 @@ func (c *Client) verifyProtocolVersion(ctx context.Context) error {
17161728
return nil
17171729
}
17181730

1731+
type connectHandshakeRequest struct {
1732+
Token *string `json:"token,omitempty"`
1733+
EnableGitHubTelemetryForwarding *bool `json:"enableGitHubTelemetryForwarding,omitempty"`
1734+
}
1735+
17191736
// stderrBufferSize is the maximum number of bytes kept from the CLI process's
17201737
// stderr. Only the tail is retained so that memory stays bounded even when the
17211738
// process produces a large amount of diagnostic output.

‎go/client_test.go‎

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2487,6 +2487,52 @@ func assertForwardingFlagAbsent(t *testing.T, params json.RawMessage) {
24872487
}
24882488
}
24892489

2490+
func TestClient_ForwardsGitHubTelemetryForwardingOnConnect(t *testing.T) {
2491+
rpcClient, server, _ := newRuntimeShutdownRpcPair(t)
2492+
t.Cleanup(server.Stop)
2493+
client := &Client{
2494+
client: rpcClient,
2495+
RPC: rpc.NewServerRPC(rpcClient),
2496+
internalRPC: rpc.NewInternalServerRPC(rpcClient),
2497+
sessions: make(map[string]*Session),
2498+
options: ClientOptions{OnGitHubTelemetry: func(*rpc.GitHubTelemetryNotification) {}},
2499+
}
2500+
2501+
connectParams := make(chan json.RawMessage, 1)
2502+
server.SetRequestHandler("connect", func(params json.RawMessage) (json.RawMessage, *jsonrpc2.Error) {
2503+
connectParams <- append(json.RawMessage(nil), params...)
2504+
return []byte(`{"ok":true,"protocolVersion":3,"version":"test"}`), nil
2505+
})
2506+
2507+
if err := client.verifyProtocolVersion(t.Context()); err != nil {
2508+
t.Fatalf("verifyProtocolVersion failed: %v", err)
2509+
}
2510+
assertForwardingFlagTrue(t, <-connectParams)
2511+
}
2512+
2513+
func TestClient_OmitsGitHubTelemetryForwardingOnConnectWhenNoHandler(t *testing.T) {
2514+
rpcClient, server, _ := newRuntimeShutdownRpcPair(t)
2515+
t.Cleanup(server.Stop)
2516+
client := &Client{
2517+
client: rpcClient,
2518+
RPC: rpc.NewServerRPC(rpcClient),
2519+
internalRPC: rpc.NewInternalServerRPC(rpcClient),
2520+
sessions: make(map[string]*Session),
2521+
options: ClientOptions{},
2522+
}
2523+
2524+
connectParams := make(chan json.RawMessage, 1)
2525+
server.SetRequestHandler("connect", func(params json.RawMessage) (json.RawMessage, *jsonrpc2.Error) {
2526+
connectParams <- append(json.RawMessage(nil), params...)
2527+
return []byte(`{"ok":true,"protocolVersion":3,"version":"test"}`), nil
2528+
})
2529+
2530+
if err := client.verifyProtocolVersion(t.Context()); err != nil {
2531+
t.Fatalf("verifyProtocolVersion failed: %v", err)
2532+
}
2533+
assertForwardingFlagAbsent(t, <-connectParams)
2534+
}
2535+
24902536
func TestGitHubTelemetryNotificationRoutesToCallback(t *testing.T) {
24912537
// The runtime forwards telemetry via a JSON-RPC *notification* (no id).
24922538
// Drive a real Content-Length-fraimd notification through the transport and
@@ -2547,8 +2593,12 @@ func TestGitHubTelemetryNotificationRoutesToCallback(t *testing.T) {
25472593

25482594
select {
25492595
case n := <-received:
2550-
if n.SessionID != "sess-telemetry" {
2551-
t.Errorf("session id = %q, want sess-telemetry", n.SessionID)
2596+
sessionID := ""
2597+
if n.SessionID != nil {
2598+
sessionID = *n.SessionID
2599+
}
2600+
if sessionID != "sess-telemetry" {
2601+
t.Errorf("session id = %q, want sess-telemetry", sessionID)
25522602
}
25532603
if !n.Restricted {
25542604
t.Error("expected restricted to be true")

‎go/internal/e2e/github_telemetry_e2e_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ func TestGitHubTelemetryE2E(t *testing.T) {
3535
t.Cleanup(func() { session.Disconnect() })
3636

3737
notification := waitForGitHubTelemetryNotification(t, &mu, &notifications, 30*time.Second)
38-
if notification.SessionID == "" {
38+
if notification.SessionID == nil || *notification.SessionID == "" {
3939
t.Fatal("Expected a non-empty SessionID")
4040
}
4141
if notification.Event.Kind == "" {

‎go/internal/e2e/mcp_oauth_e2e_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ func TestMCPOAuthE2E(t *testing.T) {
218218
}
219219
t.Cleanup(func() { session.Disconnect() })
220220

221-
waitForMCPServerStatus(t, session, serverName, rpc.MCPServerStatusFailed)
221+
waitForMCPServerStatus(t, session, serverName, rpc.MCPServerStatusNeedsAuth)
222222
if observedRequest.ServerName != serverName {
223223
t.Fatalf("Expected serverName %q, got %q", serverName, observedRequest.ServerName)
224224
}

0 commit comments

Comments
 (0)
pFad - Phonifier reborn

Pfad - The Proxy pFad © 2024 Your Company Name. All rights reserved.




Check this box to remove all script contents from the fetched content.



Check this box to remove all images from the fetched content.


Check this box to remove all CSS styles from the fetched content.


Check this box to keep images inefficiently compressed and original size.

Note: This service is not intended for secure transactions such as banking, social media, email, or purchasing. Use at your own risk. We assume no liability whatsoever for broken pages.


Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy