From 43d1207360339713f61875ee0898652349d1a0a3 Mon Sep 17 00:00:00 2001 From: abhinavgautam01 Date: Sat, 13 Jun 2026 14:34:21 +0530 Subject: [PATCH 1/4] Fix Python codegen synthetic permission approval names --- python/test_codegen_type_names.py | 11 +++++++++++ scripts/codegen/python.ts | 32 +++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) create mode 100644 python/test_codegen_type_names.py diff --git a/python/test_codegen_type_names.py b/python/test_codegen_type_names.py new file mode 100644 index 0000000000..012ee581f7 --- /dev/null +++ b/python/test_codegen_type_names.py @@ -0,0 +1,11 @@ +from copilot.generated import rpc + + +def test_python_codegen_does_not_export_quicktype_synthetic_permission_approval_name(): + assert not hasattr(rpc, "PermissionDecisionApproveForIonApproval") + assert "PermissionDecisionApproveForIonApproval" not in rpc.__all__ + + assert hasattr(rpc, "PermissionDecisionApproveForSessionApproval") + assert hasattr(rpc, "PermissionDecisionApproveForLocationApproval") + assert "PermissionDecisionApproveForSessionApproval" in rpc.__all__ + assert "PermissionDecisionApproveForLocationApproval" in rpc.__all__ diff --git a/scripts/codegen/python.ts b/scripts/codegen/python.ts index 7415914282..7ab3d469f6 100644 --- a/scripts/codegen/python.ts +++ b/scripts/codegen/python.ts @@ -928,6 +928,34 @@ function collapsePlaceholderPythonDataclasses(code: string, knownDefinitionNames return code.replace(/\n{3,}/g, "\n\n"); } +function removeUnusedSyntheticPythonDataclasses(code: string, knownDefinitionNames: Set): string { + let changed = true; + + while (changed) { + changed = false; + const classBlockRe = + /((?:^# (?:Experimental|Deprecated|Internal):[^\n]*\r?\n)*@dataclass\r?\nclass\s+(\w+):[\s\S]*?)(?=^(?:# (?:Experimental|Deprecated|Internal):[^\n]*\r?\n)*@dataclass|^class\s+\w|^def\s+\w|^[A-Z]\w+\s*=|\Z)/gm; + const matches = [...code.matchAll(classBlockRe)]; + + for (const match of matches) { + const fullBlock = match[1]; + const className = match[2]; + if (knownDefinitionNames.has(className.toLowerCase())) continue; + + const before = code.slice(0, match.index); + const after = code.slice((match.index ?? 0) + fullBlock.length); + const referenceRe = new RegExp(`\\b${escapeRegExp(className)}\\b`); + if (referenceRe.test(before) || referenceRe.test(after)) continue; + + code = `${before}${after}`; + changed = true; + break; + } + } + + return code; +} + /** * Reorder Python class/enum definitions so forward references are resolved. * Quicktype may emit classes in an order where a class references another @@ -3261,6 +3289,10 @@ def _patch_model_capabilities(data: dict) -> dict: finalCode = applyUnionRewritesToPython(finalCode, refBasedUnions); finalCode = postProcessDiscriminatorDefaultsForPython(finalCode, refBasedUnions); finalCode = unwrapRedundantPythonLambdas(finalCode); + finalCode = removeUnusedSyntheticPythonDataclasses( + finalCode, + new Set(Object.keys(allDefinitions).map((name) => name.toLowerCase())) + ); // Apply `_`-prefix to type names of internal RPC types so the leading-underscore // Python convention signals "internal, no stability guarantees" to consumers. From b746c3038facec4410c1a74d82ed0b1cd4b858cd Mon Sep 17 00:00:00 2001 From: abhinavgautam01 Date: Sat, 13 Jun 2026 14:42:38 +0530 Subject: [PATCH 2/4] Address Python codegen cleanup review feedback --- scripts/codegen/python.ts | 105 ++++++++++++++++++++++++++++++-------- 1 file changed, 83 insertions(+), 22 deletions(-) diff --git a/scripts/codegen/python.ts b/scripts/codegen/python.ts index 7ab3d469f6..cf74efbbd1 100644 --- a/scripts/codegen/python.ts +++ b/scripts/codegen/python.ts @@ -929,31 +929,92 @@ function collapsePlaceholderPythonDataclasses(code: string, knownDefinitionNames } function removeUnusedSyntheticPythonDataclasses(code: string, knownDefinitionNames: Set): string { - let changed = true; - - while (changed) { - changed = false; - const classBlockRe = - /((?:^# (?:Experimental|Deprecated|Internal):[^\n]*\r?\n)*@dataclass\r?\nclass\s+(\w+):[\s\S]*?)(?=^(?:# (?:Experimental|Deprecated|Internal):[^\n]*\r?\n)*@dataclass|^class\s+\w|^def\s+\w|^[A-Z]\w+\s*=|\Z)/gm; - const matches = [...code.matchAll(classBlockRe)]; - - for (const match of matches) { - const fullBlock = match[1]; - const className = match[2]; - if (knownDefinitionNames.has(className.toLowerCase())) continue; - - const before = code.slice(0, match.index); - const after = code.slice((match.index ?? 0) + fullBlock.length); - const referenceRe = new RegExp(`\\b${escapeRegExp(className)}\\b`); - if (referenceRe.test(before) || referenceRe.test(after)) continue; - - code = `${before}${after}`; - changed = true; - break; + interface DataclassBlock { + name: string; + text: string; + start: number; + end: number; + synthetic: boolean; + } + + const classBlockRe = + /((?:^# (?:Experimental|Deprecated|Internal):[^\n]*\r?\n)*@dataclass(?:\([^\r\n]*\))?\r?\nclass\s+(\w+):[\s\S]*?)(?=^(?:# (?:Experimental|Deprecated|Internal):[^\n]*\r?\n)*@dataclass(?:\([^\r\n]*\))?\r?\nclass\s+\w|^class\s+\w|^def\s+\w|^[A-Z]\w+\s*=|\Z)/gm; + const blocks: DataclassBlock[] = [...code.matchAll(classBlockRe)].map((match) => ({ + name: match[2], + text: match[1], + start: match.index ?? 0, + end: (match.index ?? 0) + match[1].length, + synthetic: !knownDefinitionNames.has(match[2].toLowerCase()), + })); + const syntheticBlocks = blocks.filter((block) => block.synthetic); + if (syntheticBlocks.length === 0) return code; + + let outsideSyntheticBlocks = ""; + let cursor = 0; + for (const block of syntheticBlocks) { + outsideSyntheticBlocks += code.slice(cursor, block.start); + cursor = block.end; + } + outsideSyntheticBlocks += code.slice(cursor); + + const syntheticNames = new Set(syntheticBlocks.map((block) => block.name)); + const dependencies = new Map>(); + const live = new Set(); + + for (const block of syntheticBlocks) { + const referenceRe = new RegExp(`\\b${escapeRegExp(block.name)}\\b`); + if (referenceRe.test(outsideSyntheticBlocks)) { + live.add(block.name); } + + const blockDependencies = new Set(); + for (const dependency of syntheticNames) { + if (dependency === block.name) continue; + const dependencyRe = new RegExp(`\\b${escapeRegExp(dependency)}\\b`); + if (dependencyRe.test(block.text)) { + blockDependencies.add(dependency); + } + } + dependencies.set(block.name, blockDependencies); } - return code; + const worklist = [...live]; + while (worklist.length > 0) { + const name = worklist.pop()!; + for (const dependency of dependencies.get(name) ?? []) { + if (live.has(dependency)) continue; + live.add(dependency); + worklist.push(dependency); + } + } + + const blocksToRemove = new Set(syntheticBlocks.filter((block) => !live.has(block.name)).map((block) => block.name)); + if (blocksToRemove.size === 0) return code; + + const appendSegment = (parts: string[], segment: string): void => { + if (parts.length === 0 || segment.length === 0) { + parts.push(segment); + return; + } + const previous = parts[parts.length - 1]; + const trailingNewlines = previous.match(/\n+$/)?.[0].length ?? 0; + const leadingNewlines = segment.match(/^\n+/)?.[0].length ?? 0; + if (trailingNewlines + leadingNewlines > 2) { + segment = "\n".repeat(Math.max(0, 2 - trailingNewlines)) + segment.slice(leadingNewlines); + } + parts.push(segment); + }; + + const parts: string[] = []; + cursor = 0; + for (const block of blocks) { + if (!blocksToRemove.has(block.name)) continue; + appendSegment(parts, code.slice(cursor, block.start)); + cursor = block.end; + } + appendSegment(parts, code.slice(cursor)); + + return parts.join(""); } /** From 4aa1f836c4d1c205c57c63e35f7266cba2a19248 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Fri, 31 Jul 2026 14:57:04 +0000 Subject: [PATCH 3/4] Regenerate python rpc after rebase Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- python/copilot/generated/rpc.py | 171 -------------------------------- 1 file changed, 171 deletions(-) diff --git a/python/copilot/generated/rpc.py b/python/copilot/generated/rpc.py index 84f21db7e5..463e6d346b 100644 --- a/python/copilot/generated/rpc.py +++ b/python/copilot/generated/rpc.py @@ -17641,54 +17641,6 @@ def to_dict(self) -> dict: result["wireApi"] = from_union([lambda x: to_enum(ProviderWireAPI, x), from_none], self.wire_api) return result -@dataclass -class PushAttachmentGitHubSide: - """File location on the base side of the diff. Absent for additions. - - One side of a file diff (head or base) - - File location on the head side of the diff. Absent for deletions. - - Base side of the comparison - - One side of a tree comparison (head or base) - - Head side of the comparison - """ - repo: PushGitHubRepoRef - """Repository the file lives in - - Repository the revision belongs to - """ - path: str | None = None - """Repository-relative path to the file""" - - ref: str | None = None - """Git ref (branch, tag, or commit SHA) the file is read at""" - - revision: str | None = None - """Git revision (branch, tag, or commit SHA)""" - - @staticmethod - def from_dict(obj: Any) -> 'PushAttachmentGitHubSide': - assert isinstance(obj, dict) - repo = PushGitHubRepoRef.from_dict(obj.get("repo")) - path = from_union([from_str, from_none], obj.get("path")) - ref = from_union([from_str, from_none], obj.get("ref")) - revision = from_union([from_str, from_none], obj.get("revision")) - return PushAttachmentGitHubSide(repo, path, ref, revision) - - def to_dict(self) -> dict: - result: dict = {} - result["repo"] = to_class(PushGitHubRepoRef, self.repo) - if self.path is not None: - result["path"] = from_union([from_str, from_none], self.path) - if self.ref is not None: - result["ref"] = from_union([from_str, from_none], self.ref) - if self.revision is not None: - result["revision"] = from_union([from_str, from_none], self.revision) - return result - # Experimental: this type is part of an experimental API and may change or be removed. @dataclass class PushAttachmentGitHubFileDiffSide: @@ -21113,127 +21065,6 @@ def to_dict(self) -> dict: result["extensions"] = from_list(lambda x: to_class(Extension, x), self.extensions) return result -@dataclass -class PermissionDecisionApproveForIonApproval: - """Session-scoped approval to remember (tool prompts only; omitted for path/url prompts) - - Session-scoped approval details for specific command identifiers. - - Session-scoped approval details for read-only filesystem operations. - - Session-scoped approval details for filesystem write operations. - - Session-scoped approval details for an MCP server tool, or all tools on the server when - `toolName` is null. - - Session-scoped approval details for MCP sampling requests from a server. - - Session-scoped approval details for writes to long-term memory. - - Session-scoped approval details for a custom tool, keyed by tool name. - - Session-scoped approval details for extension-management operations, optionally narrowed - by operation. - - Session-scoped approval details for an extension's permission-gated capability access, - keyed by extension name. - - Approval to persist for this location - - Location-scoped approval details for specific command identifiers. - - Location-scoped approval details for read-only filesystem operations. - - Location-scoped approval details for filesystem write operations. - - Location-scoped approval details for an MCP server tool, or all tools on the server when - `toolName` is null. - - Location-scoped approval details for MCP sampling requests from a server. - - Location-scoped approval details for writes to long-term memory. - - Location-scoped approval details for a custom tool, keyed by tool name. - - Location-scoped approval details for extension-management operations, optionally narrowed - by operation. - - Location-scoped approval details for an extension's permission-gated capability access, - keyed by extension name. - - The approval to add as a session-scoped rule - - The approval to persist for this location - """ - command_identifiers: list[str] | None = None - """Command identifiers covered by this approval.""" - - kind: ApprovalKind | None = None - """Approval scoped to specific command identifiers. - - Approval covering read-only filesystem operations. - - Approval covering filesystem write operations. - - Approval covering an MCP tool. - - Approval covering MCP sampling requests for a server. - - Approval covering writes to long-term memory. - - Approval covering a custom tool. - - Approval covering extension lifecycle operations such as enable, disable, or reload. - - Approval covering an extension's request to access a permission-gated capability. - """ - server_name: str | None = None - """MCP server name.""" - - tool_name: str | None = None - """MCP tool name, or null to cover every tool on the server. - - Custom tool name. - """ - operation: str | None = None - """Optional operation identifier; when omitted, the approval covers all extension management - operations. - """ - extension_name: str | None = None - """Extension name.""" - - external_ref_marker_external_ref_user_tool_session_approval: str | None = None - - @staticmethod - def from_dict(obj: Any) -> 'PermissionDecisionApproveForIonApproval': - assert isinstance(obj, dict) - command_identifiers = from_union([lambda x: from_list(from_str, x), from_none], obj.get("commandIdentifiers")) - kind = from_union([ApprovalKind, from_none], obj.get("kind")) - server_name = from_union([from_str, from_none], obj.get("serverName")) - tool_name = from_union([from_none, from_str], obj.get("toolName")) - operation = from_union([from_str, from_none], obj.get("operation")) - extension_name = from_union([from_str, from_none], obj.get("extensionName")) - external_ref_marker_external_ref_user_tool_session_approval = from_union([from_str, from_none], obj.get("__externalRefMarker___ExternalRef_UserToolSessionApproval")) - return PermissionDecisionApproveForIonApproval(command_identifiers, kind, server_name, tool_name, operation, extension_name, external_ref_marker_external_ref_user_tool_session_approval) - - def to_dict(self) -> dict: - result: dict = {} - if self.command_identifiers is not None: - result["commandIdentifiers"] = from_union([lambda x: from_list(from_str, x), from_none], self.command_identifiers) - if self.kind is not None: - result["kind"] = from_union([lambda x: to_enum(ApprovalKind, x), from_none], self.kind) - if self.server_name is not None: - result["serverName"] = from_union([from_str, from_none], self.server_name) - if self.tool_name is not None: - result["toolName"] = from_union([from_none, from_str], self.tool_name) - if self.operation is not None: - result["operation"] = from_union([from_str, from_none], self.operation) - if self.extension_name is not None: - result["extensionName"] = from_union([from_str, from_none], self.extension_name) - if self.external_ref_marker_external_ref_user_tool_session_approval is not None: - result["__externalRefMarker___ExternalRef_UserToolSessionApproval"] = from_union([from_str, from_none], self.external_ref_marker_external_ref_user_tool_session_approval) - return result - # Experimental: this type is part of an experimental API and may change or be removed. @dataclass class PermissionDecisionApproveForLocationApprovalExtensionPermissionAccess: @@ -34297,7 +34128,6 @@ async def handle_git_hub_telemetry_event(params: dict) -> None: "PendingPermissionRequest", "PendingPermissionRequestList", "PermissionDecision", - "PermissionDecisionApproveForIonApproval", "PermissionDecisionApproveForLocation", "PermissionDecisionApproveForLocationApproval", "PermissionDecisionApproveForLocationApprovalCommands", @@ -34494,7 +34324,6 @@ async def handle_git_hub_telemetry_event(params: dict) -> None: "PushAttachmentGitHubReleaseType", "PushAttachmentGitHubRepository", "PushAttachmentGitHubRepositoryType", - "PushAttachmentGitHubSide", "PushAttachmentGitHubSnippet", "PushAttachmentGitHubSnippetType", "PushAttachmentGitHubTreeComparison", From 901cbc2fc767a14d1d301cb732cee4dd16160441 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Fri, 31 Jul 2026 15:00:36 +0000 Subject: [PATCH 4/4] Replace brittle python codegen symbol test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- python/test_codegen_type_names.py | 32 ++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/python/test_codegen_type_names.py b/python/test_codegen_type_names.py index 012ee581f7..5242f1c786 100644 --- a/python/test_codegen_type_names.py +++ b/python/test_codegen_type_names.py @@ -1,11 +1,29 @@ +import re +import types + from copilot.generated import rpc -def test_python_codegen_does_not_export_quicktype_synthetic_permission_approval_name(): - assert not hasattr(rpc, "PermissionDecisionApproveForIonApproval") - assert "PermissionDecisionApproveForIonApproval" not in rpc.__all__ +def test_permission_approval_exports_are_union_aliases(): + approval_exports = [ + name + for name in rpc.__all__ + if re.fullmatch(r"PermissionDecisionApproveFor.*Approval", name) + ] + assert approval_exports + + for name in approval_exports: + exported = getattr(rpc, name) + assert isinstance(exported, types.UnionType), ( + f"{name} must be a union alias, not a synthetic dataclass" + ) + + +def test_permission_approval_union_loaders_deserialize_expected_variants(): + session = rpc._load_PermissionDecisionApproveForSessionApproval( + {"kind": "commands", "commandIdentifiers": ["git status"]} + ) + location = rpc._load_PermissionDecisionApproveForLocationApproval({"kind": "read"}) - assert hasattr(rpc, "PermissionDecisionApproveForSessionApproval") - assert hasattr(rpc, "PermissionDecisionApproveForLocationApproval") - assert "PermissionDecisionApproveForSessionApproval" in rpc.__all__ - assert "PermissionDecisionApproveForLocationApproval" in rpc.__all__ + assert isinstance(session, rpc.PermissionDecisionApproveForSessionApprovalCommands) + assert isinstance(location, rpc.PermissionDecisionApproveForLocationApprovalRead)