Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions python/copilot/generated/rpc.py

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

59 changes: 42 additions & 17 deletions python/e2e/test_rpc_server_e2e.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

from __future__ import annotations

import asyncio
import os
import uuid
from datetime import UTC, datetime
Expand Down Expand Up @@ -282,63 +283,87 @@ async def test_should_add_secret_filter_values(self, ctx: E2ETestContext):
# error from anyio. We don't want it to fail the test.
pass

async def test_should_list_find_and_inspect_persisted_session_state(self, ctx: E2ETestContext):
async def test_should_list_find_and_inspect_persisted_session_state(
self, authed_ctx: E2ETestContext
):
token = os.environ.get("GITHUB_TOKEN", "fakevalue")
await _configure_user(authed_ctx, token)
client = _make_authed_client(authed_ctx, token)

session_id = str(uuid.uuid4())
working_directory = Path(ctx.work_dir) / f"server-rpc-list-{uuid.uuid4().hex}"
working_directory = Path(authed_ctx.work_dir) / f"server-rpc-list-{uuid.uuid4().hex}"
working_directory.mkdir(parents=True, exist_ok=True)
missing_task_id = f"missing-task-{uuid.uuid4().hex}"
missing_session_id = str(uuid.uuid4())

session = await ctx.client.create_session(
session_id=session_id,
working_directory=str(working_directory),
on_permission_request=PermissionHandler.approve_all,
)
session = None
try:
await session.log("SERVER_RPC_LIST_READY")
save = await ctx.client.rpc.sessions.save(SessionsSaveRequest(session_id=session_id))
await client.start()
session = await client.create_session(
session_id=session_id,
working_directory=str(working_directory),
on_permission_request=PermissionHandler.approve_all,
)

await session.send(
"Record a turn for sessions.list discriminator coverage", mode="enqueue"
)
await asyncio.sleep(0.2)
save = await client.rpc.sessions.save(SessionsSaveRequest(session_id=session_id))
assert save is not None

listed = await ctx.client.rpc.sessions.list(
listed = await client.rpc.sessions.list(
SessionsListRequest(
filter=SessionListFilter(cwd=str(working_directory)),
metadata_limit=0,
)
)
assert listed.sessions is not None
assert len(listed.sessions) >= 1
matching = [item for item in listed.sessions if item.session_id == session_id]
assert len(matching) == 1
assert isinstance(matching[0], LocalSessionMetadataValue)
assert matching[0].is_remote is False
assert all(
item.context is None
or os.path.normcase(os.path.abspath(item.context.cwd))
== os.path.normcase(os.path.abspath(str(working_directory)))
for item in listed.sessions
)

by_prefix = await ctx.client.rpc.sessions.find_by_prefix(
by_prefix = await client.rpc.sessions.find_by_prefix(
SessionsFindByPrefixRequest(prefix=session_id[:8])
)
assert by_prefix.session_id in (None, session_id)

by_task = await ctx.client.rpc.sessions.find_by_task_id(
by_task = await client.rpc.sessions.find_by_task_id(
SessionsFindByTaskIDRequest(task_id=missing_task_id)
)
assert by_task.session_id is None

last_for_context = await ctx.client.rpc.sessions.get_last_for_context(
last_for_context = await client.rpc.sessions.get_last_for_context(
SessionsGetLastForContextRequest(context=SessionContext(cwd=str(working_directory)))
)
assert last_for_context.session_id in (None, session_id)

sizes = await ctx.client.rpc.sessions.get_sizes()
sizes = await client.rpc.sessions.get_sizes()
assert sizes.sizes is not None
if session_id in sizes.sizes:
assert sizes.sizes[session_id] >= 0

in_use = await ctx.client.rpc.sessions.check_in_use(
in_use = await client.rpc.sessions.check_in_use(
SessionsCheckInUseRequest(session_ids=[session_id, missing_session_id])
)
assert missing_session_id not in in_use.in_use
finally:
await session.disconnect()
if session is not None:
await session.disconnect()
try:
await client.stop()
except ExceptionGroup:
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
# Intentional: shutting down the per-test client can race the
# CLI's own teardown and surface as an aggregated cancellation
# error from anyio. We don't want it to fail the test.
pass

async def test_should_enrich_basic_session_metadata(self, ctx: E2ETestContext):
session_id = str(uuid.uuid4())
Expand Down
83 changes: 83 additions & 0 deletions python/test_rpc_generated.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,21 @@
"""Tests for generated RPC method behavior."""

import json
from unittest.mock import AsyncMock

import pytest

from copilot.rpc import (
CommandsApi,
CommandsInvokeRequest,
CommandsRespondToQueuedCommandRequest,
LocalSessionMetadataValue,
QueuedCommandHandled,
QueuedCommandNotHandled,
RemoteControlStatusOff,
RemoteControlStatusResult,
RemoteSessionMetadataValue,
SessionList,
SlashCommandTextResult,
)

Expand All @@ -22,3 +31,77 @@ async def test_commands_invoke_deserializes_slash_command_result():
assert isinstance(result, SlashCommandTextResult)
assert result.text == "hello"
assert result.markdown is True


def test_remote_control_status_deserializes_string_discriminated_union():
result = RemoteControlStatusResult.from_dict({"status": {"state": "off"}})

assert isinstance(result.status, RemoteControlStatusOff)
assert result.status.state == "off"
assert result.status.to_dict() == {"state": "off"}


def test_session_list_deserializes_boolean_discriminated_entries():
payload = {
"sessions": [
{
"sessionId": "example-local",
"startTime": "2026-07-26T10:00:00.000Z",
"modifiedTime": "2026-07-26T10:05:00.000Z",
"isRemote": False,
},
{
"sessionId": "example-remote",
"startTime": "2026-07-26T11:00:00.000Z",
"modifiedTime": "2026-07-26T11:05:00.000Z",
"isRemote": True,
"remoteSessionIds": ["example-remote"],
"repository": {"owner": "github", "name": "copilot-sdk", "branch": "main"},
},
]
}

result = SessionList.from_dict(payload)

local, remote = result.sessions
assert isinstance(local, LocalSessionMetadataValue)
assert local.session_id == "example-local"
assert local.is_remote is False
assert isinstance(remote, RemoteSessionMetadataValue)
assert remote.session_id == "example-remote"
assert remote.is_remote is True
assert remote.repository.owner == "github"


@pytest.mark.parametrize(
("handled", "expected_type"),
[(True, QueuedCommandHandled), (False, QueuedCommandNotHandled)],
)
def test_queued_command_result_deserializes_boolean_discriminator(handled, expected_type):
request = CommandsRespondToQueuedCommandRequest.from_dict(
{"requestId": "example-request", "result": {"handled": handled}}
)

assert isinstance(request.result, expected_type)


@pytest.mark.parametrize(
("variant", "expected_handled", "expected_json"),
[
(QueuedCommandHandled(), True, '{"handled": true}'),
(QueuedCommandNotHandled(), False, '{"handled": false}'),
],
)
def test_queued_command_result_serializes_boolean_discriminator(
variant, expected_handled, expected_json
):
encoded = variant.to_dict()

assert encoded["handled"] is expected_handled
assert json.dumps(encoded) == expected_json

request = CommandsRespondToQueuedCommandRequest(request_id="example-request", result=variant)
round_tripped = CommandsRespondToQueuedCommandRequest.from_dict(request.to_dict())

assert request.to_dict()["result"]["handled"] is expected_handled
assert isinstance(round_tripped.result, type(variant))
55 changes: 44 additions & 11 deletions scripts/codegen/python.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,39 @@ function postProcessExternalUnionAliasesForPython(code: string, aliases: Map<str
return code.replace(/\n{3,}/g, "\n\n");
}

/**
* Literal value of a union discriminator. The API schema discriminates on
* string `const`s (`kind: "text"`) and on boolean ones
* (`SessionListEntry.isRemote`, `QueuedCommandResult.handled`), so the JSON
* type has to survive codegen: coercing `true` to `"true"` emits a dispatcher
* arm that the decoded Python `True` can never match. Mirrors
* `GoDiscriminatorValue` in `go.ts`.
*/
type PyDiscriminatorValue = string | boolean;

/**
* Capture a schema `const` as a discriminator value, keeping booleans as
* booleans and stringifying everything else.
*/
function pyDiscriminatorValue(constValue: unknown): PyDiscriminatorValue {
return typeof constValue === "boolean" ? constValue : String(constValue);
}

/**
* Render a discriminator value as a Python literal. Booleans need Python's
* `True` / `False` spelling, since the JSON `true` would parse as a capture
* pattern in a `match` arm rather than as a literal.
*/
function pyDiscriminatorValueExpr(value: PyDiscriminatorValue): string {
if (typeof value === "boolean") return value ? "True" : "False";
return JSON.stringify(value);
}

/** Python type of a discriminator constant, for its `ClassVar` annotation. */
function pyDiscriminatorValueType(value: PyDiscriminatorValue): string {
return typeof value === "boolean" ? "bool" : "str";
}

/**
* Replace flat-merged dataclasses emitted by quicktype for $ref-based
* discriminated unions with proper Python unions: a `Name = VariantA | ...`
Expand All @@ -293,7 +326,7 @@ function postProcessExternalUnionAliasesForPython(code: string, aliases: Map<str
interface ResolvedRefBasedUnion {
aliasName: string;
discriminatorProp: string;
dispatch: Array<{ value: string; typeName: string }>;
dispatch: Array<{ value: PyDiscriminatorValue; typeName: string }>;
}
function postProcessRefBasedDiscriminatedUnionsForPython(
code: string,
Expand All @@ -304,7 +337,7 @@ function postProcessRefBasedDiscriminatedUnionsForPython(
aliasName: string;
variantNames: string[];
discriminatorProp: string;
dispatch: Array<{ value: string; typeName: string }>;
dispatch: Array<{ value: PyDiscriminatorValue; typeName: string }>;
description: string | undefined;
}
const unions: UnionInfo[] = [];
Expand Down Expand Up @@ -334,7 +367,7 @@ function postProcessRefBasedDiscriminatedUnionsForPython(
discriminator.property
];
return {
value: String(discProp.const),
value: pyDiscriminatorValue(discProp.const),
typeName: toPascalCase(variantRefNames[i]),
};
});
Expand Down Expand Up @@ -387,7 +420,7 @@ function postProcessRefBasedDiscriminatedUnionsForPython(
for (const union of unions) {
const actualAliasName = resolveActualName(union.aliasName);
const actualVariantNames: string[] = [];
const actualDispatch: Array<{ value: string; typeName: string }> = [];
const actualDispatch: Array<{ value: PyDiscriminatorValue; typeName: string }> = [];
let allResolved = true;
for (let i = 0; i < union.variantNames.length; i++) {
const actual = resolveActualName(union.variantNames[i]);
Expand Down Expand Up @@ -450,7 +483,7 @@ function postProcessRefBasedDiscriminatedUnionsForPython(
dispatcherLines.push(` kind = obj.get(${JSON.stringify(union.discriminatorProp)})`);
dispatcherLines.push(` match kind:`);
for (const m of actualDispatch) {
dispatcherLines.push(` case ${JSON.stringify(m.value)}: return ${m.typeName}.from_dict(obj)`);
dispatcherLines.push(` case ${pyDiscriminatorValueExpr(m.value)}: return ${m.typeName}.from_dict(obj)`);
}
dispatcherLines.push(
` case _: raise ValueError(f"Unknown ${actualAliasName} ${union.discriminatorProp}: {kind!r}")`
Expand Down Expand Up @@ -500,7 +533,7 @@ function postProcessDiscriminatorDefaultsForPython(
unions: ResolvedRefBasedUnion[]
): string {
// Build variant lookup: variant class name → { prop, value }.
const variantInfo = new Map<string, { prop: string; value: string }>();
const variantInfo = new Map<string, { prop: string; value: PyDiscriminatorValue }>();
for (const union of unions) {
for (const d of union.dispatch) {
// First-wins; multiple unions referencing the same variant share a
Expand Down Expand Up @@ -571,9 +604,9 @@ function postProcessDiscriminatorDefaultsForPython(
continue;
}
const fieldIndent = (block[fieldIdx].match(/^(\s+)/) ?? ["", ""])[1];
const literal = JSON.stringify(info.value);
const literal = pyDiscriminatorValueExpr(info.value);
// Replace the field with a class-level constant.
block[fieldIdx] = `${fieldIndent}${info.prop}: ClassVar[str] = ${literal}`;
block[fieldIdx] = `${fieldIndent}${info.prop}: ClassVar[${pyDiscriminatorValueType(info.value)}] = ${literal}`;
usedClassVar = true;

// Drop any field-trailing docstring lines that immediately followed the
Expand Down Expand Up @@ -1590,7 +1623,7 @@ function tryEmitPyRefBasedDiscriminatedUnion(
if (!discriminator) return undefined;

const variantTypeNames: string[] = [];
const dispatch: Array<{ value: string; typeName: string }> = [];
const dispatch: Array<{ value: PyDiscriminatorValue; typeName: string }> = [];
for (let i = 0; i < variants.length; i++) {
const variantTypeName = toPascalCase(variantRefNames[i]);
const variantSchema = resolveObjectSchema(variants[i], ctx.definitions);
Expand All @@ -1599,7 +1632,7 @@ function tryEmitPyRefBasedDiscriminatedUnion(
}
variantTypeNames.push(variantTypeName);
const discProp = resolvedVariants[i].properties?.[discriminator.property] as JSONSchema7;
dispatch.push({ value: String(discProp.const), typeName: variantTypeName });
dispatch.push({ value: pyDiscriminatorValue(discProp.const), typeName: variantTypeName });
}

if (!ctx.aliasesByName.has(aliasName)) {
Expand Down Expand Up @@ -1627,7 +1660,7 @@ function tryEmitPyRefBasedDiscriminatedUnion(
lines.push(` match kind:`);
for (const m of dispatch) {
lines.push(
` case ${JSON.stringify(m.value)}: return ${m.typeName}.from_dict(obj)`
` case ${pyDiscriminatorValueExpr(m.value)}: return ${m.typeName}.from_dict(obj)`
);
}
lines.push(
Expand Down
Loading