Skip to content

Commit aa4e707

Browse files
stephentoubCopilot
andauthored
Version-independent SDK test and codegen fixes split from the 1.0.76-0 bump (#2110)
* Bound Python e2e tests with a per-test timeout pytest-timeout has been a declared dev dependency for a while but was never configured, and the Python job sets no timeout-minutes either. When CLI 1.0.76-0 introduced a session.destroy deadlock, five of the six Python jobs sat in_progress for hours instead of failing, producing no diagnostic signal at all and holding runners until GitHub's 6-hour job limit. Setting timeout = 300 makes a deadlocked test fail in five minutes with a full stack dump pointing at the blocked call, which is how the other SDKs already behave (Node uses 30s/60s/180s per test, Go bounds the package at 20m). The value is deliberately generous: the whole Python suite completes in about 10 minutes, so no individual test comes close to 300s. Verified locally that pytest honours the setting from pyproject.toml and that it interrupts a hanging test with a traceback. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e615d062-bcb7-431e-aa9c-d3e47405723a (cherry picked from commit c33d8e2) * Close Go/.NET shell-test teardown gap The "should kill shell process" e2e test exists in all five SDKs, but only Node, Python and Rust destroy the session at the end of it. Go tears the whole client down with client.ForceStop() and .NET simply ended at the last assert, so neither ever exercised session.destroy after a session.shell.exec. That gap is why Go and .NET stayed green on CLI 1.0.76-0 while Node, Python and Rust hang: the hang is in session.destroy after a shell exec, and the two green SDKs never made the call. Adding the teardown makes all five cover the same sequence. Verified locally against the same CLI binary (1.0.76-0, sha256 b8bfad2c...): with the added Disconnect(), the Go test hangs past a 3-minute timeout where it previously passed in seconds. The same test passes on CLI 1.0.73. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e615d062-bcb7-431e-aa9c-d3e47405723a (cherry picked from commit 2e54b47) * Remove Rust e2e test for session.mcp.registerExternalClient The runtime team confirmed this method is marked `visibility: internal` in the shared API contract: its `client` and `transport` fields are live in-process MCP SDK instances that cannot be serialized over JSON-RPC. No SDK exposes it as a typed method, and the Rust test drove it through a raw `call_session_rpc` with placeholder JSON objects that could never satisfy that contract. It passed on CLI 1.0.73 only because the old TypeScript dispatch happened to route internal methods generically. 1.0.76-0 routes the method through the Rust native registry, where the host-effect switch has no case for it, so it now returns -32603 "Unsupported native session host effect". That is correct behaviour, not a regression, so the test is removed along with its now-orphaned snapshot and a note recording why the gap is deliberate. Both `call_session_rpc` and `is_mcp_server_running` remain in use by other tests in the file, so no helper became dead. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e615d062-bcb7-431e-aa9c-d3e47405723a (cherry picked from commit f0c846c) * codegen: emit Rust type aliases for scalar RPC result schemas The Rust codegen only emitted a named type for result schemas that were an enum, array, map, or object. A method whose result is an inline primitive (e.g. {"type":"integer"}) produced a reference to a type that was never defined, so the generated crate failed to compile with E0425. No RPC method had a primitive result until session.cancelAllBackgroundAgents was added, so this latent gap only surfaces on a schema update. Add rustScalarType()/emitRustScalarAlias() so scalar results emit a plain type alias alongside the existing shapes. This is a no-op against the current pinned schema and prevents the next dependency bump from hitting the same build break. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e615d062-bcb7-431e-aa9c-d3e47405723a * Ensure shell test teardown on failure Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b565403c-57ed-4368-84f1-f04bfe39e46e --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e615d062-bcb7-431e-aa9c-d3e47405723a Copilot-Session: b565403c-57ed-4368-84f1-f04bfe39e46e
1 parent 523b0c7 commit aa4e707

6 files changed

Lines changed: 54 additions & 56 deletions

File tree

dotnet/test/E2E/RpcShellAndFleetE2ETests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ public async Task Should_Execute_Shell_Command()
2828
[Fact]
2929
public async Task Should_Kill_Shell_Process()
3030
{
31-
var session = await CreateSessionAsync();
31+
await using var session = await CreateSessionAsync();
3232
var command = OperatingSystem.IsWindows()
3333
? "powershell -NoLogo -NoProfile -Command \"Start-Sleep -Seconds 30\""
3434
: "sleep 30";

go/internal/e2e/rpc_shell_and_fleet_e2e_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ func TestRPCShellAndFleetE2E(t *testing.T) {
5555
if err != nil {
5656
t.Fatalf("Failed to create session: %v", err)
5757
}
58+
t.Cleanup(func() { _ = session.Disconnect() })
5859

5960
var command string
6061
if runtime.GOOS == "windows" {

python/pyproject.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,3 +92,7 @@ python_files = "test_*.py"
9292
python_classes = "Test*"
9393
python_functions = "test_*"
9494
asyncio_mode = "auto"
95+
# Bound every test so a deadlock fails fast with a stack dump instead of occupying the
96+
# whole CI leg until GitHub's 6-hour job limit. The full suite runs in ~10 minutes, so no
97+
# individual test legitimately approaches this.
98+
timeout = 300

rust/tests/e2e/rpc_mcp_lifecycle.rs

Lines changed: 5 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -194,58 +194,11 @@ async fn should_start_and_restart_mcp_server() {
194194
.await;
195195
}
196196

197-
#[tokio::test]
198-
async fn should_register_and_unregister_external_mcp_client() {
199-
with_e2e_context(
200-
"rpc_mcp_lifecycle",
201-
"should_register_and_unregister_external_mcp_client",
202-
|ctx| {
203-
Box::pin(async move {
204-
ctx.set_default_copilot_user();
205-
let host_server = "rpc-lifecycle-extclient-host";
206-
let client = ctx.start_client().await;
207-
let session =
208-
client
209-
.create_session(ctx.approve_all_session_config().with_mcp_servers(
210-
create_test_mcp_servers(ctx.repo_root(), host_server),
211-
))
212-
.await
213-
.expect("create session");
214-
wait_for_mcp_server_status(&session, host_server, McpServerStatus::Connected).await;
215-
216-
let external_name = "rpc-lifecycle-external-client";
217-
assert!(!is_mcp_server_running(&session, external_name).await);
218-
219-
call_session_rpc(
220-
&session,
221-
"session.mcp.registerExternalClient",
222-
json!({
223-
"serverName": external_name,
224-
"client": { "id": external_name },
225-
"transport": { "kind": "in-process" },
226-
"config": { "command": "noop" }
227-
}),
228-
)
229-
.await
230-
.expect("register external MCP client");
231-
assert!(is_mcp_server_running(&session, external_name).await);
232-
233-
call_session_rpc(
234-
&session,
235-
"session.mcp.unregisterExternalClient",
236-
json!({ "serverName": external_name }),
237-
)
238-
.await
239-
.expect("unregister external MCP client");
240-
assert!(!is_mcp_server_running(&session, external_name).await);
241-
242-
session.disconnect().await.expect("disconnect session");
243-
client.stop().await.expect("stop client");
244-
})
245-
},
246-
)
247-
.await;
248-
}
197+
// There is deliberately no e2e test for `session.mcp.registerExternalClient`. That method is
198+
// marked `visibility: internal` in the shared API contract: its `client` and `transport` fields
199+
// are live in-process MCP SDK instances, so it cannot be driven over JSON-RPC, and no SDK
200+
// exposes it as a typed method. A raw-RPC test used to pass only because older CLIs routed
201+
// internal methods generically; it never exercised a supported wire API.
249202

250203
#[tokio::test]
251204
async fn should_reload_mcp_servers_with_config() {

scripts/codegen/rust.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -575,6 +575,45 @@ function emitRustMapAlias(
575575
);
576576
}
577577

578+
/**
579+
* Map a primitive JSON Schema type to its Rust equivalent, or `undefined` when
580+
* the schema is not a plain scalar. Mirrors the primitive branches of
581+
* {@link resolveRustType}.
582+
*/
583+
function rustScalarType(schema: JSONSchema7): string | undefined {
584+
if (schema.enum || schema.const !== undefined) return undefined;
585+
switch (schema.type) {
586+
case "string":
587+
return "String";
588+
case "number":
589+
return "f64";
590+
case "integer":
591+
return isIntegerSchemaBoundedToInt32(schema) ? "i32" : "i64";
592+
case "boolean":
593+
return "bool";
594+
default:
595+
return undefined;
596+
}
597+
}
598+
599+
/**
600+
* Emit a type alias for a named schema that resolves to a primitive scalar
601+
* (e.g. an RPC result declared as `{ "type": "integer" }`). Without this the
602+
* generated RPC surface would reference a `*Result` type that was never
603+
* defined.
604+
*/
605+
function emitRustScalarAlias(
606+
typeName: string,
607+
schema: JSONSchema7,
608+
ctx: RustCodegenCtx,
609+
description?: string,
610+
): void {
611+
if (ctx.generatedNames.has(typeName)) return;
612+
const scalarType = rustScalarType(schema);
613+
if (!scalarType) return;
614+
emitRustTypeAlias(typeName, schema, scalarType, ctx, description);
615+
}
616+
578617
function rustRpcResultDescription(
579618
method: RpcMethod,
580619
resultSchema: JSONSchema7 | undefined,
@@ -1527,6 +1566,8 @@ function generateApiTypesCode(
15271566
} else {
15281567
tryEmitRustUnion(schema, name, "", ctx);
15291568
}
1569+
} else {
1570+
emitRustScalarAlias(name, schema, ctx, schema.description);
15301571
}
15311572
}
15321573

@@ -1576,6 +1617,8 @@ function generateApiTypesCode(
15761617
emitRustMapAlias(resultName, resolved, ctx, resolved.description);
15771618
} else if (isObjectSchema(resolved)) {
15781619
emitRustStruct(resultName, resolved, ctx, resolved.description);
1620+
} else {
1621+
emitRustScalarAlias(resultName, resolved, ctx, resolved.description);
15791622
}
15801623
}
15811624
}

test/snapshots/rpc_mcp_lifecycle/should_register_and_unregister_external_mcp_client.yaml

Lines changed: 0 additions & 3 deletions
This file was deleted.

0 commit comments

Comments
 (0)