Skip to content

Commit 9885b92

Browse files
Phase B fixup: address CI e2e failures from runtime port + Kind/Type enum
Six classes of failures surfaced by the Python e2e CI: 1. `_make_subprocess_client(ctx, use_stdio=False)` in test_pending_work_resume_e2e.py and test_suspend_e2e.py was still creating an stdio `RuntimeConnection` regardless of the `use_stdio` flag (leftover from the bulk migration). The downstream `f"localhost:{server.runtime_port}"` then formatted `None` because stdio doesn't listen on a port, triggering `ValueError: Invalid port in cli_url: localhost:None`. Now branches on `use_stdio` and constructs `RuntimeConnection.tcp(...)` when False. 2. `.kind.value` / `.type.value` usages in e2e tests assumed the discriminator field was an `Enum`. After the Phase L codegen change it's a `ClassVar[str]`, so direct equality (`r.kind == "write"`, `attachment.type == "file"`) works and no `.value` is needed. Updated test_multi_client_e2e.py, test_permissions_e2e.py, test_tools_e2e.py, test_session_e2e.py. 3. `test_rpc_tasks_and_handlers_e2e.py` was constructing the now-deleted `PermissionDecision`/`PermissionDecisionApproveForIonApproval` merged blob types. Rewrote each call to use the proper per-variant constructors (`PermissionDecisionReject(feedback=...)`, `PermissionDecisionApprovePermanently(domain=...)`, `PermissionDecisionApproveForSession(approval=PermissionDecisionApproveForSessionApprovalCustomTool(...))`, `PermissionDecisionApproveForLocation(location_key=..., approval=PermissionDecisionApproveForLocationApprovalCustomTool(...))`), and `found_task.type == TaskInfoType.AGENT` to `isinstance(found_task, TaskAgentInfo)`. 4. Codegen: quicktype merges structurally-identical types and synthesizes a fuzzy class name (`PermissionDecisionApproveForIonApproval` is the merge of `PermissionDecisionApproveFor{Session,Location}Approval`). Extended `acronymCandidates` to try the `Session→Ion` / `Location→Ion` substitutions so the post-pass can resolve the fuzzy name, and added an explicit cleanup step that deletes the now-orphan blob after the union rewrites complete. Updated generated rpc.py reflects this. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 30f2d6b commit 9885b92

9 files changed

Lines changed: 84 additions & 149 deletions

python/copilot/generated/rpc.py

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

python/e2e/test_multi_client_e2e.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ async def test_one_client_approves_permission_and_both_see_the_result(
279279
assert len(c1_perm_completed) > 0
280280
assert len(c2_perm_completed) > 0
281281
for event in c1_perm_completed + c2_perm_completed:
282-
assert event.data.result.kind.value == "approved"
282+
assert event.data.result.kind == "approved"
283283

284284
await session2.disconnect()
285285

@@ -328,7 +328,7 @@ async def test_one_client_rejects_permission_and_both_see_the_result(
328328
assert len(c1_perm_completed) > 0
329329
assert len(c2_perm_completed) > 0
330330
for event in c1_perm_completed + c2_perm_completed:
331-
assert event.data.result.kind.value == "denied-interactively-by-user"
331+
assert event.data.result.kind == "denied-interactively-by-user"
332332

333333
await session2.disconnect()
334334

python/e2e/test_pending_work_resume_e2e.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,15 @@ def _make_subprocess_client(ctx: E2ETestContext, *, use_stdio: bool = True) -> C
3636
github_token = (
3737
"fake-token-for-e2e-tests" if os.environ.get("GITHUB_ACTIONS") == "true" else None
3838
)
39+
if use_stdio:
40+
connection = RuntimeConnection.stdio(path=ctx.cli_path)
41+
else:
42+
connection = RuntimeConnection.tcp(
43+
path=ctx.cli_path, connection_token="py-tcp-shared-test-token"
44+
)
3945
return CopilotClient(
4046
CopilotClientOptions(
41-
connection=RuntimeConnection.stdio(path=ctx.cli_path),
47+
connection=connection,
4248
working_directory=ctx.work_dir,
4349
env=ctx.get_env(),
4450
github_token=github_token,

python/e2e/test_permissions_e2e.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ def on_permission_request(
4646
assert len(permission_requests) > 0
4747

4848
# Should include write permission request
49-
write_requests = [req for req in permission_requests if req.kind.value == "write"]
49+
write_requests = [req for req in permission_requests if req.kind == "write"]
5050
assert len(write_requests) > 0
5151

5252
await session.disconnect()

python/e2e/test_rpc_tasks_and_handlers_e2e.py

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,15 @@
1212
import pytest
1313

1414
from copilot.generated.rpc import (
15-
ApprovalKind,
1615
CommandsHandlePendingCommandRequest,
1716
HandlePendingToolCallRequest,
18-
PermissionDecision,
19-
PermissionDecisionApproveForIonApproval,
20-
PermissionDecisionKind,
17+
PermissionDecisionApproveForLocation,
18+
PermissionDecisionApproveForLocationApprovalCustomTool,
19+
PermissionDecisionApproveForSession,
20+
PermissionDecisionApproveForSessionApprovalCustomTool,
21+
PermissionDecisionApprovePermanently,
22+
PermissionDecisionReject,
2123
PermissionDecisionRequest,
22-
TaskInfoType,
2324
TasksCancelRequest,
2425
TasksPromoteToBackgroundRequest,
2526
TasksRemoveRequest,
@@ -137,32 +138,24 @@ async def test_should_return_expected_results_for_missing_pending_handler_reques
137138
permission = await session.rpc.permissions.handle_pending_permission_request(
138139
PermissionDecisionRequest(
139140
request_id="missing-permission-request",
140-
result=PermissionDecision(
141-
kind=PermissionDecisionKind.REJECT,
142-
feedback="not approved",
143-
),
141+
result=PermissionDecisionReject(feedback="not approved"),
144142
)
145143
)
146144
assert permission.success is False
147145

148146
permanent = await session.rpc.permissions.handle_pending_permission_request(
149147
PermissionDecisionRequest(
150148
request_id="missing-permanent-permission-request",
151-
result=PermissionDecision(
152-
kind=PermissionDecisionKind.APPROVE_PERMANENTLY,
153-
domain="example.com",
154-
),
149+
result=PermissionDecisionApprovePermanently(domain="example.com"),
155150
)
156151
)
157152
assert permanent.success is False
158153

159154
session_approval = await session.rpc.permissions.handle_pending_permission_request(
160155
PermissionDecisionRequest(
161156
request_id="missing-session-approval-request",
162-
result=PermissionDecision(
163-
kind=PermissionDecisionKind.APPROVE_FOR_SESSION,
164-
approval=PermissionDecisionApproveForIonApproval(
165-
kind=ApprovalKind.CUSTOM_TOOL,
157+
result=PermissionDecisionApproveForSession(
158+
approval=PermissionDecisionApproveForSessionApprovalCustomTool(
166159
tool_name="missing-tool",
167160
),
168161
),
@@ -173,11 +166,9 @@ async def test_should_return_expected_results_for_missing_pending_handler_reques
173166
location_approval = await session.rpc.permissions.handle_pending_permission_request(
174167
PermissionDecisionRequest(
175168
request_id="missing-location-approval-request",
176-
result=PermissionDecision(
177-
kind=PermissionDecisionKind.APPROVE_FOR_LOCATION,
169+
result=PermissionDecisionApproveForLocation(
178170
location_key="missing-location",
179-
approval=PermissionDecisionApproveForIonApproval(
180-
kind=ApprovalKind.CUSTOM_TOOL,
171+
approval=PermissionDecisionApproveForLocationApprovalCustomTool(
181172
tool_name="missing-tool",
182173
),
183174
),
@@ -215,7 +206,7 @@ async def test_should_report_implemented_error_for_invalid_task_agent_model(
215206

216207
async def test_should_start_background_agent_and_report_task_details(self, ctx: E2ETestContext):
217208
"""Start a background agent task and verify task details then remove it."""
218-
from copilot.generated.rpc import TaskInfoExecutionMode, TaskInfoStatus
209+
from copilot.generated.rpc import TaskAgentInfo, TaskInfoExecutionMode, TaskInfoStatus
219210

220211
session = await ctx.client.create_session(
221212
on_permission_request=PermissionHandler.approve_all,
@@ -248,7 +239,7 @@ async def test_should_start_background_agent_and_report_task_details(self, ctx:
248239
)
249240
assert found_task.id == task_id
250241
assert found_task.description == "SDK background agent coverage"
251-
assert found_task.type == TaskInfoType.AGENT
242+
assert isinstance(found_task, TaskAgentInfo)
252243
assert found_task.agent_type == "general-purpose"
253244
assert found_task.execution_mode == TaskInfoExecutionMode.BACKGROUND
254245
assert found_task.prompt == "Reply with TASK_AGENT_DONE exactly."

python/e2e/test_session_e2e.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -701,7 +701,7 @@ async def test_should_send_with_file_attachment(self, ctx: E2ETestContext):
701701
attachments = user_messages[-1].data.attachments
702702
assert attachments is not None and len(attachments) == 1
703703
attachment = attachments[0]
704-
assert attachment.type.value == "file"
704+
assert attachment.type == "file"
705705
assert attachment.display_name == "attached-file.txt"
706706
assert attachment.path == file_path
707707
assert attachment.line_range is not None
@@ -739,7 +739,7 @@ async def test_should_send_with_directory_attachment(self, ctx: E2ETestContext):
739739
attachments = user_messages[-1].data.attachments
740740
assert attachments is not None and len(attachments) == 1
741741
attachment = attachments[0]
742-
assert attachment.type.value == "directory"
742+
assert attachment.type == "directory"
743743
assert attachment.display_name == "attached-directory"
744744
assert attachment.path == directory_path
745745

@@ -778,7 +778,7 @@ async def test_should_send_with_selection_attachment(self, ctx: E2ETestContext):
778778
attachments = user_messages[-1].data.attachments
779779
assert attachments is not None and len(attachments) == 1
780780
attachment = attachments[0]
781-
assert attachment.type.value == "selection"
781+
assert attachment.type == "selection"
782782
assert attachment.display_name == "selected-file.cs"
783783
assert attachment.file_path == file_path
784784
assert attachment.text == 'string Value = "SELECTION_SENTINEL";'

python/e2e/test_suspend_e2e.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,15 @@ def _make_subprocess_client(ctx: E2ETestContext, *, use_stdio: bool = True) -> C
3030
github_token = (
3131
"fake-token-for-e2e-tests" if os.environ.get("GITHUB_ACTIONS") == "true" else None
3232
)
33+
if use_stdio:
34+
connection = RuntimeConnection.stdio(path=ctx.cli_path)
35+
else:
36+
connection = RuntimeConnection.tcp(
37+
path=ctx.cli_path, connection_token="py-tcp-shared-test-token"
38+
)
3339
return CopilotClient(
3440
CopilotClientOptions(
35-
connection=RuntimeConnection.stdio(path=ctx.cli_path),
41+
connection=connection,
3642
working_directory=ctx.work_dir,
3743
env=ctx.get_env(),
3844
github_token=github_token,

python/e2e/test_tools_e2e.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ def on_permission_request(request, invocation):
203203
assert "HELLO" in assistant_message.data.content
204204

205205
# Should have received a custom-tool permission request
206-
custom_tool_requests = [r for r in permission_requests if r.kind.value == "custom-tool"]
206+
custom_tool_requests = [r for r in permission_requests if r.kind == "custom-tool"]
207207
assert len(custom_tool_requests) > 0
208208
assert custom_tool_requests[0].tool_name == "encrypt_string"
209209

scripts/codegen/python.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,14 @@ function postProcessRefBasedDiscriminatedUnionsForPython(
359359
[/Id\b/g, "ID"],
360360
[/Llm/g, "LLM"],
361361
[/Cli/g, "CLI"],
362+
// quicktype merges structurally-identical unions and synthesizes a
363+
// single class name using a common substring. For our schema this
364+
// surfaces as e.g. `PermissionDecisionApproveForIonApproval` (merge
365+
// of `PermissionDecisionApproveFor{Session,Location}Approval` —
366+
// common substring is `Ion`). Try that substitution so we find
367+
// and delete the merged blob during union post-processing.
368+
[/Session/g, "Ion"],
369+
[/Location/g, "Ion"],
362370
];
363371
const results = new Set<string>([name]);
364372
for (const [pattern, replacement] of substitutions) {
@@ -451,6 +459,47 @@ function postProcessRefBasedDiscriminatedUnionsForPython(
451459
}
452460

453461
code = applyUnionRewritesToPython(code, resolved);
462+
463+
// Clean up quicktype-merged blobs that get left behind when quicktype merges
464+
// multiple structurally-identical types into one with a synthesized fuzzy
465+
// name (e.g. ``PermissionDecisionApproveForIonApproval`` for the merge of
466+
// ``PermissionDecisionApproveFor{Session,Location}Approval``). These
467+
// blobs are unused after we emit the proper per-variant + union shape.
468+
const orphanFuzzyMergedNames = ["PermissionDecisionApproveForIonApproval"];
469+
for (const orphanName of orphanFuzzyMergedNames) {
470+
const lines = code.split("\n");
471+
let classStart = -1;
472+
for (let i = 0; i < lines.length; i++) {
473+
if (lines[i] === `class ${orphanName}:` || lines[i].startsWith(`class ${orphanName}(`)) {
474+
classStart = i;
475+
break;
476+
}
477+
}
478+
if (classStart < 0) continue;
479+
let blockStart = classStart;
480+
while (
481+
blockStart > 0 &&
482+
(lines[blockStart - 1] === "@dataclass" || /^# /.test(lines[blockStart - 1]))
483+
) {
484+
blockStart--;
485+
}
486+
let blockEnd = classStart + 1;
487+
while (blockEnd < lines.length) {
488+
const ln = lines[blockEnd];
489+
if (
490+
/^class \w/.test(ln) ||
491+
/^def \w/.test(ln) ||
492+
ln === "@dataclass" ||
493+
/^# (?:Experimental|Deprecated|Internal):/.test(ln)
494+
) {
495+
break;
496+
}
497+
blockEnd++;
498+
}
499+
lines.splice(blockStart, blockEnd - blockStart);
500+
code = lines.join("\n");
501+
}
502+
454503
return { code, unions: resolved };
455504
}
456505

0 commit comments

Comments
 (0)