From c2bd5bf44b9678d7e4e6f47b46bfe07c05fad264 Mon Sep 17 00:00:00 2001 From: examon Date: Wed, 29 Jul 2026 12:49:50 +0000 Subject: [PATCH 1/4] 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/copilot/generated/rpc.py | 12 ++--- python/test_rpc_generated.py | 83 +++++++++++++++++++++++++++++++++ scripts/codegen/python.ts | 55 +++++++++++++++++----- 3 files changed, 133 insertions(+), 17 deletions(-) diff --git a/python/copilot/generated/rpc.py b/python/copilot/generated/rpc.py index 713e78cca0..15deb019a2 100644 --- a/python/copilot/generated/rpc.py +++ b/python/copilot/generated/rpc.py @@ -6528,7 +6528,7 @@ class QueuedCommandHandled: """Queued-command response indicating the host executed the command, with an optional flag to stop queue processing. """ - handled: ClassVar[str] = "true" + handled: ClassVar[bool] = True """The host actually executed the queued command.""" stop_processing_queue: bool | None = None @@ -6555,7 +6555,7 @@ class QueuedCommandNotHandled: """Queued-command response indicating the host did not execute the command and the queue may continue. """ - handled: ClassVar[str] = "false" + handled: ClassVar[bool] = False """The host did not execute the queued command. Unblocks the queue without claiming the command was processed (e.g. when the handler threw before completing). """ @@ -27742,8 +27742,8 @@ def _load_QueuedCommandResult(obj: Any) -> "QueuedCommandResult": assert isinstance(obj, dict) kind = obj.get("handled") match kind: - case "true": return QueuedCommandHandled.from_dict(obj) - case "false": return QueuedCommandNotHandled.from_dict(obj) + case True: return QueuedCommandHandled.from_dict(obj) + case False: return QueuedCommandNotHandled.from_dict(obj) case _: raise ValueError(f"Unknown QueuedCommandResult handled: {kind!r}") # State of the runtime-managed remote-control singleton. @@ -27766,8 +27766,8 @@ def _load_SessionListEntry(obj: Any) -> "SessionListEntry": assert isinstance(obj, dict) kind = obj.get("isRemote") match kind: - case "false": return LocalSessionMetadataValue.from_dict(obj) - case "true": return RemoteSessionMetadataValue.from_dict(obj) + case False: return LocalSessionMetadataValue.from_dict(obj) + case True: return RemoteSessionMetadataValue.from_dict(obj) case _: raise ValueError(f"Unknown SessionListEntry isRemote: {kind!r}") # Open a session by creating, resuming, attaching, connecting to a remote, or handing off. diff --git a/python/test_rpc_generated.py b/python/test_rpc_generated.py index 8b9423c137..5556a77c38 100644 --- a/python/test_rpc_generated.py +++ b/python/test_rpc_generated.py @@ -1,5 +1,6 @@ """Tests for generated RPC method behavior.""" +import json from unittest.mock import AsyncMock import pytest @@ -7,6 +8,14 @@ from copilot.rpc import ( CommandsApi, CommandsInvokeRequest, + CommandsRespondToQueuedCommandRequest, + LocalSessionMetadataValue, + QueuedCommandHandled, + QueuedCommandNotHandled, + RemoteControlStatusOff, + RemoteControlStatusResult, + RemoteSessionMetadataValue, + SessionList, SlashCommandTextResult, ) @@ -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)) diff --git a/scripts/codegen/python.ts b/scripts/codegen/python.ts index 5b343122b6..7415914282 100644 --- a/scripts/codegen/python.ts +++ b/scripts/codegen/python.ts @@ -272,6 +272,39 @@ function postProcessExternalUnionAliasesForPython(code: string, aliases: Map; + dispatch: Array<{ value: PyDiscriminatorValue; typeName: string }>; } function postProcessRefBasedDiscriminatedUnionsForPython( code: string, @@ -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[] = []; @@ -334,7 +367,7 @@ function postProcessRefBasedDiscriminatedUnionsForPython( discriminator.property ]; return { - value: String(discProp.const), + value: pyDiscriminatorValue(discProp.const), typeName: toPascalCase(variantRefNames[i]), }; }); @@ -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]); @@ -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}")` @@ -500,7 +533,7 @@ function postProcessDiscriminatorDefaultsForPython( unions: ResolvedRefBasedUnion[] ): string { // Build variant lookup: variant class name → { prop, value }. - const variantInfo = new Map(); + const variantInfo = new Map(); for (const union of unions) { for (const d of union.dispatch) { // First-wins; multiple unions referencing the same variant share a @@ -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 @@ -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); @@ -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)) { @@ -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( From 59ce28d8d5cd32aea6de00524619079c6f27e62c Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Thu, 30 Jul 2026 15:28:38 +0000 Subject: [PATCH 2/4] 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/e2e/test_rpc_server_e2e.py | 54 +++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 17 deletions(-) diff --git a/python/e2e/test_rpc_server_e2e.py b/python/e2e/test_rpc_server_e2e.py index bcef26754b..4fd8d34b90 100644 --- a/python/e2e/test_rpc_server_e2e.py +++ b/python/e2e/test_rpc_server_e2e.py @@ -6,6 +6,7 @@ from __future__ import annotations +import asyncio import os import uuid from datetime import UTC, datetime @@ -282,30 +283,44 @@ 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)) @@ -313,32 +328,37 @@ async def test_should_list_find_and_inspect_persisted_session_state(self, ctx: E 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: + pass async def test_should_enrich_basic_session_metadata(self, ctx: E2ETestContext): session_id = str(uuid.uuid4()) From 4c6c817d1846bc8183e928318d83560673160cdf Mon Sep 17 00:00:00 2001 From: examon Date: Thu, 30 Jul 2026 15:40:49 +0000 Subject: [PATCH 3/4] 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/e2e/test_rpc_server_e2e.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/python/e2e/test_rpc_server_e2e.py b/python/e2e/test_rpc_server_e2e.py index 4fd8d34b90..5d71c764b3 100644 --- a/python/e2e/test_rpc_server_e2e.py +++ b/python/e2e/test_rpc_server_e2e.py @@ -304,7 +304,9 @@ async def test_should_list_find_and_inspect_persisted_session_state( on_permission_request=PermissionHandler.approve_all, ) - await session.send("Record a turn for sessions.list discriminator coverage", mode="enqueue") + 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 @@ -358,6 +360,9 @@ async def test_should_list_find_and_inspect_persisted_session_state( try: await client.stop() except ExceptionGroup: + # 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): From 9d60739a4abd6e262bd2ecfd0296bfc45a865457 Mon Sep 17 00:00:00 2001 From: examon Date: Thu, 30 Jul 2026 16:16:43 +0000 Subject: [PATCH 4/4] 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. --- python/e2e/test_rpc_server_e2e.py | 36 ++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/python/e2e/test_rpc_server_e2e.py b/python/e2e/test_rpc_server_e2e.py index 5d71c764b3..e7c4a446ce 100644 --- a/python/e2e/test_rpc_server_e2e.py +++ b/python/e2e/test_rpc_server_e2e.py @@ -6,7 +6,6 @@ from __future__ import annotations -import asyncio import os import uuid from datetime import UTC, datetime @@ -56,7 +55,7 @@ ) from copilot.session import PermissionHandler -from .testharness import E2ETestContext +from .testharness import E2ETestContext, wait_for_condition pytestmark = pytest.mark.asyncio(loop_scope="module") @@ -307,16 +306,33 @@ async def test_should_list_find_and_inspect_persisted_session_state( 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 client.rpc.sessions.list( - SessionsListRequest( - filter=SessionListFilter(cwd=str(working_directory)), - metadata_limit=0, + + listed = None + + async def session_is_listed() -> bool: + nonlocal listed + # Re-save on every attempt: on slower runners the enqueued turn is not + # necessarily recorded yet when the first save runs, so a single save + # followed by a fixed sleep races the CLI's own persistence. + save = await client.rpc.sessions.save(SessionsSaveRequest(session_id=session_id)) + assert save is not None + listed = await client.rpc.sessions.list( + SessionsListRequest( + filter=SessionListFilter(cwd=str(working_directory)), + metadata_limit=0, + ) ) + return any(item.session_id == session_id for item in listed.sessions or []) + + await wait_for_condition( + session_is_listed, + timeout=60.0, + timeout_message=( + "Timed out waiting for the saved session to be returned by sessions.list." + ), ) + + assert listed is not None assert listed.sessions is not None assert len(listed.sessions) >= 1 matching = [item for item in listed.sessions if item.session_id == session_id]