Fix codegen identifier sanitization#1285
Conversation
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>
There was a problem hiding this comment.
Pull request overview
This PR updates the Rust, Go, and C# code generators to better sanitize schema-derived identifiers (especially string-enum values containing path/URL separators) while preserving correct on-the-wire serialized values. It also adjusts Rust API type generation to emit top-level discriminated unions for union schema definitions, and refreshes/aligns Rust docs/examples with current generated request shapes.
Changes:
- Update identifier splitting/sanitization logic across Rust/Go/C# generators to handle non-alphanumeric separators (e.g.,
/chat/completions) and avoid invalid identifiers. - Add Rust enum-variant uniqueness handling for discriminated unions and string enums; emit top-level discriminated unions for union definitions.
- Refresh generated Rust API types ordering and update the Rust README example for
SessionsForkRequestto useto_event_id.
Show a summary per file
| File | Description |
|---|---|
| scripts/codegen/rust.ts | Adds Rust identifier sanitization + uniqueness helpers; improves union/string-enum variant naming; emits top-level union enums. |
| scripts/codegen/go.ts | Broadens identifier word splitting for Go naming and enum const suffix generation to handle more separators. |
| scripts/codegen/csharp.ts | Adds C# identifier sanitization + uniqueness helper and uses it for generated string-enum static members. |
| rust/src/generated/api_types.rs | Generated Rust API types re-ordered (no type body changes intended). |
| rust/README.md | Updates fork request example to use to_event_id and idiomatic .into(). |
Copilot's findings
- Files reviewed: 4/5 changed files
- Comments generated: 2
|
@stephentoub Were all the per-language generators re-run? Haven't gone through it line by line but seems unexpected that the Rust generated code has new content but the other languages don't. |
|
@stephentoub Optimally I think we should do the enum member normalization on the runtime side when we produce the JSON schema so that we can fail at that stage if it leads to a clash. |
I believe the generator changes just caused some of the output in Rust to be re-ordered, so the surface area is actually the same but it looks like a bunch of changes in the diff. |
Agreed. |
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>
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>
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>
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>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
String enum values from newer Copilot runtime schemas can contain path or URL separators, such as
/chat/completions. Those values must continue to round-trip on the wire, but C#, Go, and Rust generators also need to turn them into valid language identifiers.This updates the code generators to sanitize generated identifiers while preserving the original serialized values. Rust also now emits top-level discriminated unions from API schema definitions so newer schemas that reference union result types compile correctly.
Generated output was refreshed against the currently imported
@github/copilot1.0.46 schema. Therust/src/generated/api_types.rsdiff is declaration reordering only; comparing public type bodies by name showed no added, removed, or changed generated Rust API definitions.Validation performed:
cd scripts/codegen && npm run generatecd nodejs && npm run buildpython -m compileall -q python\copilotcd go && go test ./... -run '^$'cd dotnet && dotnet build test\GitHub.Copilot.SDK.Test.csproj --no-restorecd rust && cargo check --all-features --all-targets