From e94157e78552e3a5f309d20ecda8aaa9bfecaaf5 Mon Sep 17 00:00:00 2001 From: abhinavgautam01 Date: Sat, 13 Jun 2026 14:34:21 +0530 Subject: [PATCH 1/2] Fix Python codegen synthetic permission approval names --- python/copilot/generated/rpc.py | 118 ------------------------------ python/test_codegen_type_names.py | 11 +++ scripts/codegen/python.ts | 32 ++++++++ 3 files changed, 43 insertions(+), 118 deletions(-) create mode 100644 python/test_codegen_type_names.py diff --git a/python/copilot/generated/rpc.py b/python/copilot/generated/rpc.py index 69bf16007..3cee207d2 100644 --- a/python/copilot/generated/rpc.py +++ b/python/copilot/generated/rpc.py @@ -17025,123 +17025,6 @@ def to_dict(self) -> dict: result["copilotUser"] = from_union([lambda x: to_class(CopilotUserResponse, x), from_none], self.copilot_user) return result -@dataclass -class PermissionDecisionApproveForIonApproval: - """Session-scoped approval to remember (tool prompts only; omitted for path/url prompts) - - Schema for the `PermissionDecisionApproveForSessionApprovalCommands` type. - - Schema for the `PermissionDecisionApproveForSessionApprovalRead` type. - - Schema for the `PermissionDecisionApproveForSessionApprovalWrite` type. - - Schema for the `PermissionDecisionApproveForSessionApprovalMcp` type. - - Schema for the `PermissionDecisionApproveForSessionApprovalMcpSampling` type. - - Schema for the `PermissionDecisionApproveForSessionApprovalMemory` type. - - Schema for the `PermissionDecisionApproveForSessionApprovalCustomTool` type. - - Schema for the `PermissionDecisionApproveForSessionApprovalExtensionManagement` type. - - Schema for the `PermissionDecisionApproveForSessionApprovalExtensionPermissionAccess` - type. - - Approval to persist for this location - - Schema for the `PermissionDecisionApproveForLocationApprovalCommands` type. - - Schema for the `PermissionDecisionApproveForLocationApprovalRead` type. - - Schema for the `PermissionDecisionApproveForLocationApprovalWrite` type. - - Schema for the `PermissionDecisionApproveForLocationApprovalMcp` type. - - Schema for the `PermissionDecisionApproveForLocationApprovalMcpSampling` type. - - Schema for the `PermissionDecisionApproveForLocationApprovalMemory` type. - - Schema for the `PermissionDecisionApproveForLocationApprovalCustomTool` type. - - Schema for the `PermissionDecisionApproveForLocationApprovalExtensionManagement` type. - - Schema for the `PermissionDecisionApproveForLocationApprovalExtensionPermissionAccess` - type. - - 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 HandlePendingToolCallRequest: @@ -23811,7 +23694,6 @@ async def handle_canvas_action_invoke(params: dict) -> dict | None: "PendingPermissionRequest", "PendingPermissionRequestList", "PermissionDecision", - "PermissionDecisionApproveForIonApproval", "PermissionDecisionApproveForLocation", "PermissionDecisionApproveForLocationApproval", "PermissionDecisionApproveForLocationApprovalCommands", diff --git a/python/test_codegen_type_names.py b/python/test_codegen_type_names.py new file mode 100644 index 000000000..012ee581f --- /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 0ba72db50..6da3cbc59 100644 --- a/scripts/codegen/python.ts +++ b/scripts/codegen/python.ts @@ -895,6 +895,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 @@ -3208,6 +3236,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 8c0d07a36a9029b356367810780c1ee8fbaa578c Mon Sep 17 00:00:00 2001 From: abhinavgautam01 Date: Sat, 13 Jun 2026 14:42:38 +0530 Subject: [PATCH 2/2] 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 6da3cbc59..602df9a77 100644 --- a/scripts/codegen/python.ts +++ b/scripts/codegen/python.ts @@ -896,31 +896,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(""); } /**