Skip to content

Commit e3a5cab

Browse files
examonSteveSandersonMSCopilot
authored
python: decode boolean-discriminated unions (#2123)
* python: decode boolean-discriminated unions The Python generator captured a union discriminator's JSON Schema `const` with `String()`, so a boolean const became the string "true"/"false" and the emitted dispatcher matched `case "true":`. A JSON boolean decodes to Python `True`, which never equals `"true"`, so every boolean-discriminated union fell through to `raise ValueError`. Two unions are affected. `sessions.list()` raised `ValueError: Unknown SessionListEntry isRemote: False` for any non-empty session list, and `QueuedCommandHandled.to_dict()` put the string `"true"` on the wire where the schema declares `{"type": "boolean", "const": true}`. Keep the const's JSON type through codegen and render it as a Python literal (`True`/`False`), annotating the discriminator `ClassVar` as `bool`. This mirrors how `go.ts` already models discriminator values. Regenerating changes six lines of `python/copilot/generated/rpc.py`; no other language changes. * python: strengthen sessions.list e2e discriminator coverage Ensure the rpc sessions.list e2e test persists at least one session entry and asserts the matching session decodes to LocalSessionMetadataValue with is_remote=False, exercising the boolean discriminator path end-to-end. Use an authed client token from GITHUB_TOKEN (default fakevalue) and enqueue a user turn before save/list so the entry is present without depending on full model completion. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * python: document intentional ExceptionGroup swallow in sessions.list e2e CodeQL flagged the new `except ExceptionGroup: pass` in the sessions.list teardown as an empty except clause with no explanation. Add the same explanatory comment the four other identical teardowns in this file already carry. Also wrap the enqueue `session.send(...)` call, which was 104 characters and failed the repo's 100-column ruff lint. * python: wait for the saved session instead of a fixed sleep in sessions.list e2e The sessions.list e2e test enqueued a turn, slept 200ms, then saved and listed once. On the Windows runners the enqueued turn was not recorded yet when save ran, so sessions.list came back empty and `assert len(listed.sessions) >= 1` failed with `assert 0 >= 1`. Linux and macOS happened to win the race. Replace the fixed sleep with the existing `wait_for_condition` harness helper, re-saving on each attempt until the session actually appears in sessions.list. All discriminator assertions are unchanged, so the boolean-discriminator path this PR fixes is still exercised end-to-end. `asyncio` was imported only for the removed sleep, so drop the import. --------- Co-authored-by: examon <examon@users.noreply.github.com> Co-authored-by: Steve Sanderson <SteveSandersonMS@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 3186a84 commit e3a5cab

4 files changed

Lines changed: 197 additions & 40 deletions

File tree

python/copilot/generated/rpc.py

Lines changed: 6 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

python/e2e/test_rpc_server_e2e.py

Lines changed: 64 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@
5555
)
5656
from copilot.session import PermissionHandler
5757

58-
from .testharness import E2ETestContext
58+
from .testharness import E2ETestContext, wait_for_condition
5959

6060
pytestmark = pytest.mark.asyncio(loop_scope="module")
6161

@@ -282,63 +282,104 @@ async def test_should_add_secret_filter_values(self, ctx: E2ETestContext):
282282
# error from anyio. We don't want it to fail the test.
283283
pass
284284

285-
async def test_should_list_find_and_inspect_persisted_session_state(self, ctx: E2ETestContext):
285+
async def test_should_list_find_and_inspect_persisted_session_state(
286+
self, authed_ctx: E2ETestContext
287+
):
288+
token = os.environ.get("GITHUB_TOKEN", "fakevalue")
289+
await _configure_user(authed_ctx, token)
290+
client = _make_authed_client(authed_ctx, token)
291+
286292
session_id = str(uuid.uuid4())
287-
working_directory = Path(ctx.work_dir) / f"server-rpc-list-{uuid.uuid4().hex}"
293+
working_directory = Path(authed_ctx.work_dir) / f"server-rpc-list-{uuid.uuid4().hex}"
288294
working_directory.mkdir(parents=True, exist_ok=True)
289295
missing_task_id = f"missing-task-{uuid.uuid4().hex}"
290296
missing_session_id = str(uuid.uuid4())
291-
292-
session = await ctx.client.create_session(
293-
session_id=session_id,
294-
working_directory=str(working_directory),
295-
on_permission_request=PermissionHandler.approve_all,
296-
)
297+
session = None
297298
try:
298-
await session.log("SERVER_RPC_LIST_READY")
299-
save = await ctx.client.rpc.sessions.save(SessionsSaveRequest(session_id=session_id))
300-
assert save is not None
301-
302-
listed = await ctx.client.rpc.sessions.list(
303-
SessionsListRequest(
304-
filter=SessionListFilter(cwd=str(working_directory)),
305-
metadata_limit=0,
299+
await client.start()
300+
session = await client.create_session(
301+
session_id=session_id,
302+
working_directory=str(working_directory),
303+
on_permission_request=PermissionHandler.approve_all,
304+
)
305+
306+
await session.send(
307+
"Record a turn for sessions.list discriminator coverage", mode="enqueue"
308+
)
309+
310+
listed = None
311+
312+
async def session_is_listed() -> bool:
313+
nonlocal listed
314+
# Re-save on every attempt: on slower runners the enqueued turn is not
315+
# necessarily recorded yet when the first save runs, so a single save
316+
# followed by a fixed sleep races the CLI's own persistence.
317+
save = await client.rpc.sessions.save(SessionsSaveRequest(session_id=session_id))
318+
assert save is not None
319+
listed = await client.rpc.sessions.list(
320+
SessionsListRequest(
321+
filter=SessionListFilter(cwd=str(working_directory)),
322+
metadata_limit=0,
323+
)
306324
)
325+
return any(item.session_id == session_id for item in listed.sessions or [])
326+
327+
await wait_for_condition(
328+
session_is_listed,
329+
timeout=60.0,
330+
timeout_message=(
331+
"Timed out waiting for the saved session to be returned by sessions.list."
332+
),
307333
)
334+
335+
assert listed is not None
308336
assert listed.sessions is not None
337+
assert len(listed.sessions) >= 1
338+
matching = [item for item in listed.sessions if item.session_id == session_id]
339+
assert len(matching) == 1
340+
assert isinstance(matching[0], LocalSessionMetadataValue)
341+
assert matching[0].is_remote is False
309342
assert all(
310343
item.context is None
311344
or os.path.normcase(os.path.abspath(item.context.cwd))
312345
== os.path.normcase(os.path.abspath(str(working_directory)))
313346
for item in listed.sessions
314347
)
315348

316-
by_prefix = await ctx.client.rpc.sessions.find_by_prefix(
349+
by_prefix = await client.rpc.sessions.find_by_prefix(
317350
SessionsFindByPrefixRequest(prefix=session_id[:8])
318351
)
319352
assert by_prefix.session_id in (None, session_id)
320353

321-
by_task = await ctx.client.rpc.sessions.find_by_task_id(
354+
by_task = await client.rpc.sessions.find_by_task_id(
322355
SessionsFindByTaskIDRequest(task_id=missing_task_id)
323356
)
324357
assert by_task.session_id is None
325358

326-
last_for_context = await ctx.client.rpc.sessions.get_last_for_context(
359+
last_for_context = await client.rpc.sessions.get_last_for_context(
327360
SessionsGetLastForContextRequest(context=SessionContext(cwd=str(working_directory)))
328361
)
329362
assert last_for_context.session_id in (None, session_id)
330363

331-
sizes = await ctx.client.rpc.sessions.get_sizes()
364+
sizes = await client.rpc.sessions.get_sizes()
332365
assert sizes.sizes is not None
333366
if session_id in sizes.sizes:
334367
assert sizes.sizes[session_id] >= 0
335368

336-
in_use = await ctx.client.rpc.sessions.check_in_use(
369+
in_use = await client.rpc.sessions.check_in_use(
337370
SessionsCheckInUseRequest(session_ids=[session_id, missing_session_id])
338371
)
339372
assert missing_session_id not in in_use.in_use
340373
finally:
341-
await session.disconnect()
374+
if session is not None:
375+
await session.disconnect()
376+
try:
377+
await client.stop()
378+
except ExceptionGroup:
379+
# Intentional: shutting down the per-test client can race the
380+
# CLI's own teardown and surface as an aggregated cancellation
381+
# error from anyio. We don't want it to fail the test.
382+
pass
342383

343384
async def test_should_enrich_basic_session_metadata(self, ctx: E2ETestContext):
344385
session_id = str(uuid.uuid4())

python/test_rpc_generated.py

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,21 @@
11
"""Tests for generated RPC method behavior."""
22

3+
import json
34
from unittest.mock import AsyncMock
45

56
import pytest
67

78
from copilot.rpc import (
89
CommandsApi,
910
CommandsInvokeRequest,
11+
CommandsRespondToQueuedCommandRequest,
12+
LocalSessionMetadataValue,
13+
QueuedCommandHandled,
14+
QueuedCommandNotHandled,
15+
RemoteControlStatusOff,
16+
RemoteControlStatusResult,
17+
RemoteSessionMetadataValue,
18+
SessionList,
1019
SlashCommandTextResult,
1120
)
1221

@@ -22,3 +31,77 @@ async def test_commands_invoke_deserializes_slash_command_result():
2231
assert isinstance(result, SlashCommandTextResult)
2332
assert result.text == "hello"
2433
assert result.markdown is True
34+
35+
36+
def test_remote_control_status_deserializes_string_discriminated_union():
37+
result = RemoteControlStatusResult.from_dict({"status": {"state": "off"}})
38+
39+
assert isinstance(result.status, RemoteControlStatusOff)
40+
assert result.status.state == "off"
41+
assert result.status.to_dict() == {"state": "off"}
42+
43+
44+
def test_session_list_deserializes_boolean_discriminated_entries():
45+
payload = {
46+
"sessions": [
47+
{
48+
"sessionId": "example-local",
49+
"startTime": "2026-07-26T10:00:00.000Z",
50+
"modifiedTime": "2026-07-26T10:05:00.000Z",
51+
"isRemote": False,
52+
},
53+
{
54+
"sessionId": "example-remote",
55+
"startTime": "2026-07-26T11:00:00.000Z",
56+
"modifiedTime": "2026-07-26T11:05:00.000Z",
57+
"isRemote": True,
58+
"remoteSessionIds": ["example-remote"],
59+
"repository": {"owner": "github", "name": "copilot-sdk", "branch": "main"},
60+
},
61+
]
62+
}
63+
64+
result = SessionList.from_dict(payload)
65+
66+
local, remote = result.sessions
67+
assert isinstance(local, LocalSessionMetadataValue)
68+
assert local.session_id == "example-local"
69+
assert local.is_remote is False
70+
assert isinstance(remote, RemoteSessionMetadataValue)
71+
assert remote.session_id == "example-remote"
72+
assert remote.is_remote is True
73+
assert remote.repository.owner == "github"
74+
75+
76+
@pytest.mark.parametrize(
77+
("handled", "expected_type"),
78+
[(True, QueuedCommandHandled), (False, QueuedCommandNotHandled)],
79+
)
80+
def test_queued_command_result_deserializes_boolean_discriminator(handled, expected_type):
81+
request = CommandsRespondToQueuedCommandRequest.from_dict(
82+
{"requestId": "example-request", "result": {"handled": handled}}
83+
)
84+
85+
assert isinstance(request.result, expected_type)
86+
87+
88+
@pytest.mark.parametrize(
89+
("variant", "expected_handled", "expected_json"),
90+
[
91+
(QueuedCommandHandled(), True, '{"handled": true}'),
92+
(QueuedCommandNotHandled(), False, '{"handled": false}'),
93+
],
94+
)
95+
def test_queued_command_result_serializes_boolean_discriminator(
96+
variant, expected_handled, expected_json
97+
):
98+
encoded = variant.to_dict()
99+
100+
assert encoded["handled"] is expected_handled
101+
assert json.dumps(encoded) == expected_json
102+
103+
request = CommandsRespondToQueuedCommandRequest(request_id="example-request", result=variant)
104+
round_tripped = CommandsRespondToQueuedCommandRequest.from_dict(request.to_dict())
105+
106+
assert request.to_dict()["result"]["handled"] is expected_handled
107+
assert isinstance(round_tripped.result, type(variant))

0 commit comments

Comments
 (0)