Fix codegen identifier sanitization (#1285)

* Fix generated identifier sanitization

Sanitize generated string enum identifiers across C#, Go, and Rust so schema values containing path or URL separators produce valid language identifiers while preserving wire values. Also emit top-level Rust discriminated unions needed by newer schemas and refresh the Rust generated output from the currently imported 1.0.46 schema.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Harden enum collision handling

Fail code generation when sanitized enum values collide with existing public member names instead of stabilizing arbitrary numeric suffixes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Complete denied-permission replay snapshots

Add the optional post-denial model turns so denied edit tests can replay deterministically if the CLI asks the model to summarize the denied tool result.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Complete shared denied-permission snapshot

Add the optional post-denial model turn for the shared multi-client replay snapshot so SDKs that wait for the final assistant message can replay deterministically.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Share denied-permission multi-client snapshot

Point the Rust denied-permission multi-client E2E test at the shared multi_client snapshot and remove the duplicate Rust-only copy.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Share approved-permission multi-client snapshot

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: stephentoub

rust/README.md

@@ -166,8 +166,8 @@ let forked = client
     .rpc()
     .sessions()
     .fork(github_copilot_sdk::generated::api_types::SessionsForkRequest {
-        session_id: "session-id".to_string(),
-        from_message_id: None,
+        session_id: "session-id".into(),
+        to_event_id: None,
     })
     .await?;
 ```

rust/src/generated/api_types.rs

@@ -105,7 +105,7 @@ async fn both_clients_see_tool_request_and_completion_events() {
 #[tokio::test]
 async fn one_client_approves_permission_and_both_see_the_result() {
     with_e2e_context(
-        "rust_multi_client",
+        "multi_client",
         "one_client_approves_permission_and_both_see_the_result",
         |ctx| {
             Box::pin(async move {
@@ -193,7 +193,7 @@ async fn one_client_approves_permission_and_both_see_the_result() {
 #[tokio::test]
 async fn one_client_rejects_permission_and_both_see_the_result() {
     with_e2e_context(
-        "rust_multi_client",
+        "multi_client",
         "one_client_rejects_permission_and_both_see_the_result",
         |ctx| {
             Box::pin(async move {

rust/tests/e2e/multi_client.rs

@@ -118,18 +118,42 @@ function xmlDocEnumComment(description: string | undefined, indent: string): str
 }
 
 function toPascalCase(name: string): string {
-    if (name.includes("_") || name.includes("-")) {
-        return name.split(/[-_]/).map((p) => p.charAt(0).toUpperCase() + p.slice(1)).join("");
-    }
+    const parts = splitCSharpIdentifierParts(name);
+    if (parts.length > 1) return parts.map(toPascalCasePart).join("");
     return name.charAt(0).toUpperCase() + name.slice(1);
 }
 
 function typeToClassName(typeName: string): string {
-    return typeName.split(/[._]/).map((p) => p.charAt(0).toUpperCase() + p.slice(1)).join("");
+    return splitCSharpIdentifierParts(typeName).map(toPascalCasePart).join("");
+}
+
+function splitCSharpIdentifierParts(value: string): string[] {
+    return value.split(/[^A-Za-z0-9]+/).filter(Boolean);
+}
+
+function toPascalCasePart(value: string): string {
+    return value.charAt(0).toUpperCase() + value.slice(1);
 }
 
-function toPascalCaseEnumMember(value: string): string {
-    return value.split(/[-_.]/).map((p) => p.charAt(0).toUpperCase() + p.slice(1)).join("");
+function toCSharpIdentifier(value: string, fallback: string): string {
+    let identifier = splitCSharpIdentifierParts(value).map(toPascalCasePart).join("");
+    if (!identifier) {
+        identifier = fallback;
+    } else if (!/^[A-Za-z_]/.test(identifier)) {
+        identifier = `${fallback}${identifier}`;
+    }
+    return identifier;
+}
+
+function uniqueCSharpIdentifier(value: string, used: Set<string>, fallback: string): string {
+    const identifier = toCSharpIdentifier(value, fallback);
+    if (used.has(identifier)) {
+        throw new Error(
+            `Generated C# string enum member identifier "${identifier}" is not unique for value "${value}". Add an explicit naming rule instead of stabilizing an arbitrary public member name.`
+        );
+    }
+    used.add(identifier);
+    return identifier;
 }
 
 async function formatCSharpFile(filePath: string): Promise<void> {
@@ -311,6 +335,7 @@ const COPYRIGHT = `/*-----------------------------------------------------------
 
 const EXPERIMENTAL_ATTRIBUTE = "[Experimental(Diagnostics.Experimental)]";
 const OBSOLETE_ATTRIBUTE = `[Obsolete("This member is deprecated and will be removed in a future version.")]`;
+const STRING_ENUM_RESERVED_MEMBER_NAMES = new Set(["Value", "Equals", "GetHashCode", "ToString", "Converter"]);
 
 function experimentalAttribute(indent = ""): string {
     return `${indent}${EXPERIMENTAL_ATTRIBUTE}`;
@@ -374,9 +399,11 @@ function getOrCreateEnum(
     lines.push(`    }`, "");
     lines.push(`    /// <summary>Gets the value associated with this <see cref="${enumName}"/>.</summary>`);
     lines.push(`    public string Value => _value ?? string.Empty;`, "");
+    const usedMemberNames = new Set(STRING_ENUM_RESERVED_MEMBER_NAMES);
     for (const value of values) {
+        const memberName = uniqueCSharpIdentifier(value, usedMemberNames, "Value");
         lines.push(`    /// <summary>Gets the <c>${escapeXml(value)}</c> value.</summary>`);
-        lines.push(`    public static ${enumName} ${toPascalCaseEnumMember(value)} { get; } = new("${value}");`, "");
+        lines.push(`    public static ${enumName} ${memberName} { get; } = new("${escapeCSharpStringLiteral(value)}");`, "");
     }
     lines.push(`    /// <summary>Returns a value indicating whether two <see cref="${enumName}"/> instances are equivalent.</summary>`);
     lines.push(`    public static bool operator ==(${enumName} left, ${enumName} right) => left.Equals(right);`, "");

scripts/codegen/csharp.ts

@@ -52,7 +52,8 @@ const wrapGoCommentText = wordwrap(goCommentTextWrapLength);
 
 function toPascalCase(s: string): string {
     return s
-        .split(/[._]/)
+        .split(/[^A-Za-z0-9]+/)
+        .filter((word) => word.length > 0)
         .map((w) => goInitialisms.has(w.toLowerCase()) ? w.toUpperCase() : w.charAt(0).toUpperCase() + w.slice(1))
         .join("");
 }
@@ -92,7 +93,7 @@ function splitGoIdentifierWords(name: string): string[] {
     return name
         .replace(/([A-Z]+)([A-Z][a-z])/g, "$1_$2")
         .replace(/([a-z0-9])([A-Z])/g, "$1_$2")
-        .split(/[._-]/)
+        .split(/[^A-Za-z0-9]+/)
         .filter((word) => word.length > 0);
 }
 
@@ -547,8 +548,17 @@ function getOrCreateGoEnum(
     const consts = values
         .map((value) => ({ value, constSuffix: goEnumConstSuffix(value) }))
         .sort((left, right) => `${enumName}${left.constSuffix}`.localeCompare(`${enumName}${right.constSuffix}`));
+    const usedConstNames = new Map<string, string>();
     for (const { value, constSuffix } of consts) {
-        lines.push(`\t${enumName}${constSuffix} ${enumName} = "${value}"`);
+        const constName = `${enumName}${constSuffix}`;
+        const existingValue = usedConstNames.get(constName);
+        if (existingValue !== undefined) {
+            throw new Error(
+                `Generated Go enum const identifier "${constName}" is not unique for values "${existingValue}" and "${value}". Add an explicit naming rule instead of stabilizing an arbitrary public const name.`
+            );
+        }
+        usedConstNames.set(constName, value);
+        lines.push(`\t${constName} ${enumName} = "${value}"`);
     }
     lines.push(`)`);
 
@@ -558,14 +568,14 @@ function getOrCreateGoEnum(
 }
 
 function goEnumConstSuffix(value: string): string {
-    return value
-        .split(/[-_.]/)
+    const suffix = splitGoIdentifierWords(value)
         .map((word) =>
             goInitialisms.has(word.toLowerCase())
                 ? word.toUpperCase()
                 : word.charAt(0).toUpperCase() + word.slice(1)
         )
         .join("");
+    return suffix || "Value";
 }
 
 function goDiscriminatedUnionVariantTypeName(

scripts/codegen/go.ts

@@ -59,11 +59,39 @@ const STRING_NEWTYPE_OVERRIDES: Record<string, string> = {
 
 function toPascalCase(s: string): string {
 	return s
-		.split(/[._\-\s]+/)
+		.split(/[^A-Za-z0-9]+/)
+		.filter(Boolean)
 		.map((w) => w.charAt(0).toUpperCase() + w.slice(1))
 		.join("");
 }
 
+function toRustPascalIdentifier(value: string, fallback: string): string {
+	let identifier = toPascalCase(value);
+	if (!identifier) {
+		identifier = fallback;
+	} else if (!/^[A-Za-z_]/.test(identifier)) {
+		identifier = `${fallback}${identifier}`;
+	}
+
+	return RUST_KEYWORDS.has(identifier) ? `${identifier}Value` : identifier;
+}
+
+function uniqueRustPascalIdentifier(
+	value: string,
+	used: Set<string>,
+	fallback: string,
+	reserved: Set<string> = new Set(),
+): string {
+	const identifier = toRustPascalIdentifier(value, fallback);
+	if (used.has(identifier) || reserved.has(identifier)) {
+		throw new Error(
+			`Generated Rust enum variant identifier "${identifier}" is not unique for value "${value}". Add an explicit naming rule instead of stabilizing an arbitrary public variant name.`,
+		);
+	}
+	used.add(identifier);
+	return identifier;
+}
+
 function toSnakeCase(s: string): string {
 	return s
 		.replace(/([A-Z])/g, "_$1")
@@ -233,10 +261,16 @@ function tryEmitRustDiscriminatedUnion(
 	lines.push("#[serde(untagged)]");
 	lines.push(`pub enum ${enumName} {`);
 
+	const usedVariantNames = new Set<string>();
 	for (const { schema: variantSchema, typeName } of resolvedVariants) {
 		const kind = ((variantSchema.properties?.kind as JSONSchema7 | undefined)
 			?.const ?? typeName) as string;
-		lines.push(`    ${toPascalCase(kind)}(${stripOption(typeName)}),`);
+		const variantName = uniqueRustPascalIdentifier(
+			kind,
+			usedVariantNames,
+			"Variant",
+		);
+		lines.push(`    ${variantName}(${stripOption(typeName)}),`);
 	}
 
 	lines.push("}");
@@ -617,8 +651,15 @@ function emitRustStringEnum(
 	lines.push("#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]");
 	lines.push(`pub enum ${enumName} {`);
 
+	const usedVariantNames = new Set<string>();
+	const reservedVariantNames = new Set(["Unknown"]);
 	for (const value of values) {
-		const variantName = toPascalCase(value);
+		const variantName = uniqueRustPascalIdentifier(
+			value,
+			usedVariantNames,
+			"Value",
+			reservedVariantNames,
+		);
 		if (variantName !== value) {
 			lines.push(`    #[serde(rename = "${value}")]`);
 		}
@@ -651,7 +692,7 @@ function emitRustConstStringEnum(
 	}
 	lines.push("#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]");
 	lines.push(`pub enum ${enumName} {`);
-	const variantName = toPascalCase(value);
+	const variantName = toRustPascalIdentifier(value, "Value");
 	if (variantName !== value) {
 		lines.push(`    #[serde(rename = "${value}")]`);
 	}
@@ -927,6 +968,8 @@ function generateApiTypesCode(apiSchema: ApiSchema): string {
 				schema.description,
 				isSchemaExperimental(schema),
 			);
+		} else if (getUnionVariants(schema)) {
+			tryEmitRustDiscriminatedUnion(schema, name, "", ctx);
 		} else if (isObjectSchema(schema)) {
 			emitRustStruct(name, schema, ctx, schema.description);
 		}

scripts/codegen/rust.ts

@@ -23,3 +23,30 @@ conversations:
             function:
               name: view
               arguments: '{"path":"${workdir}/protected.txt"}'
+  - messages:
+      - role: system
+        content: ${system}
+      - role: user
+        content: Edit protected.txt and replace 'protected' with 'hacked'.
+      - role: assistant
+        content: I'll help you edit protected.txt to replace 'protected' with 'hacked'. Let me first view the file and then make
+          the change.
+        tool_calls:
+          - id: toolcall_0
+            type: function
+            function:
+              name: report_intent
+              arguments: '{"intent":"Editing protected.txt file"}'
+          - id: toolcall_1
+            type: function
+            function:
+              name: view
+              arguments: '{"path":"${workdir}/protected.txt"}'
+      - role: tool
+        tool_call_id: toolcall_0
+        content: Intent logged
+      - role: tool
+        tool_call_id: toolcall_1
+        content: Permission denied and could not request permission from user
+      - role: assistant
+        content: I don't have permission to view or edit protected.txt, so I can't make that change.

test/snapshots/multi_client/one_client_rejects_permission_and_both_see_the_result.yaml

@@ -22,3 +22,29 @@ conversations:
             function:
               name: view
               arguments: '{"path":"${workdir}/protected.txt"}'
+  - messages:
+      - role: system
+        content: ${system}
+      - role: user
+        content: Edit protected.txt and replace 'protected' with 'hacked'.
+      - role: assistant
+        content: I'll view the file first, then make the edit.
+        tool_calls:
+          - id: toolcall_0
+            type: function
+            function:
+              name: report_intent
+              arguments: '{"intent":"Editing protected.txt file"}'
+          - id: toolcall_1
+            type: function
+            function:
+              name: view
+              arguments: '{"path":"${workdir}/protected.txt"}'
+      - role: tool
+        tool_call_id: toolcall_0
+        content: Intent logged
+      - role: tool
+        tool_call_id: toolcall_1
+        content: Permission denied and could not request permission from user
+      - role: assistant
+        content: I don't have permission to view or edit protected.txt, so I can't make that change.