Skip to content

Seal generated session event types#1330

Merged
stephentoub merged 1 commit into
mainfrom
stephentoub/seal-event-types
May 19, 2026
Merged

Seal generated session event types#1330
stephentoub merged 1 commit into
mainfrom
stephentoub/seal-event-types

Conversation

@stephentoub
Copy link
Copy Markdown
Collaborator

The session event generator currently emits most generated C# DTOs as inheritable classes even when they are leaf types. Sealing those generated types better communicates the intended object model and avoids unnecessary inheritance surface area.

Summary

  • Emits sealed classes for session event leaf events, event data DTOs, nested DTOs, JSON-union containers, polymorphic union leaf variants, and the source-generated JSON context.
  • Keeps polymorphic bases unsealed because they are required for generated derived types and System.Text.Json polymorphic deserialization.
  • Regenerates dotnet/src/Generated/SessionEvents.cs from the updated generator.

Validation

  • dotnet build src\GitHub.Copilot.SDK.csproj --no-restore
  • dotnet test test\GitHub.Copilot.SDK.Test.csproj --no-build --filter FullyQualifiedName~GitHub.Copilot.SDK.Test.Unit

Generated by Copilot.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@stephentoub stephentoub requested a review from a team as a code owner May 19, 2026 02:22
Copilot AI review requested due to automatic review settings May 19, 2026 02:22
@github-actions
Copy link
Copy Markdown
Contributor

Cross-SDK Consistency Review ✅

This PR modifies only .NET-specific files:

  • scripts/codegen/csharp.ts — C# code generator
  • dotnet/src/Generated/SessionEvents.cs — regenerated .NET output

The change adds sealed to generated leaf C# classes, which is a C#-specific language feature and best practice. There is no equivalent concept that needs to be mirrored in other SDKs:

  • Go: Structs cannot be inherited at all — effectively already "sealed" by the language
  • TypeScript/Node.js: No native sealed class concept
  • Python: No native sealed class concept

No cross-SDK consistency issues identified. The change is appropriately scoped to the .NET SDK.

Generated by SDK Consistency Review Agent for issue #1330 · ● 103.9K ·

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the C# session-event type generator to emit sealed for generated leaf DTOs (events, event payload/data types, nested object DTOs, JSON-union containers, and discriminated-union leaf variants) and regenerates the SessionEvents.cs output accordingly. This reduces unnecessary inheritance surface area while keeping polymorphic base types unsealed for System.Text.Json polymorphic deserialization.

Changes:

  • Added DiscriminatedUnionGenerationOptions plumbing to allow sealing discriminated-union leaf variants during generation.
  • Updated the generator to emit sealed for nested DTOs, event payload DTOs, event leaf types, and the source-generated JsonSerializerContext.
  • Regenerated dotnet/src/Generated/SessionEvents.cs to reflect the new sealing behavior.
Show a summary per file
File Description
scripts/codegen/csharp.ts Adds an option to seal discriminated-union leaf classes and applies sealed to generated leaf DTO classes and the JSON context.
dotnet/src/Generated/SessionEvents.cs Regenerated output marking applicable session-event DTO types as sealed.

Copilot's findings

  • Files reviewed: 1/2 changed files
  • Comments generated: 0

@stephentoub stephentoub merged commit c021797 into main May 19, 2026
33 checks passed
@stephentoub stephentoub deleted the stephentoub/seal-event-types branch May 19, 2026 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants