Go codegen: per-event-type data structs#1037
Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
This PR fixes Go SDK breakages caused by quicktype’s “flat union” Data struct by switching Go session-event codegen to a custom generator that emits per-event-type *XxxData structs behind a SessionEventData interface, aligning Go with the C# approach and making additions of new event types additive-only.
Changes:
- Replaced Go session-events quicktype generation with custom codegen producing per-event data structs plus custom
SessionEvent(un)marshaling and forward-compatibleRawSessionEventData. - Updated Go SDK runtime code, samples, README docs, and tests/E2E to use type assertions / type switches on
event.Data. - Updated Go RPC generated types to follow Go initialism casing (
UI,URI,MIME) and re-enabled codegen-check determinism (plus.gitattributesLF enforcement).
Show a summary per file
| File | Description |
|---|---|
| scripts/codegen/go.ts | Implements custom Go session-events codegen (per-event structs, interface payloads, custom JSON (un)marshal). |
| go/generated_session_events.go | Regenerated Go session-events types with per-event payload structs and SessionEventData interface. |
| go/session.go | Updates event handling and examples to use type switches/assertions; updates RPC UI casing usage. |
| go/session_test.go | Updates unit tests to construct events with new per-event data structs. |
| go/samples/chat.go | Updates sample to read event payloads via type assertions. |
| go/rpc/generated_rpc.go | Renames RPC API/types for Go initialisms (UIApi, FormatURI, SessionRpc.UI). |
| go/README.md | Updates documentation examples to use type switches on event.Data. |
| go/client.go | Updates inline usage example to use type assertion on event.Data. |
| go/internal/e2e/testharness/helper.go | Updates harness helpers to interpret event payloads via type assertions. |
| go/internal/e2e/tools_test.go | Updates E2E tool tests to use *AssistantMessageData payloads. |
| go/internal/e2e/tool_results_test.go | Updates tool-results E2E checks to use typed payloads. |
| go/internal/e2e/streaming_fidelity_test.go | Updates streaming delta/message checks to use typed payloads. |
| go/internal/e2e/skills_test.go | Updates skill-marker assertions to use typed payloads. |
| go/internal/e2e/session_test.go | Updates session start/message assertions and helper to extract message from typed payloads. |
| go/internal/e2e/permissions_test.go | Updates permission/tool-complete checks to use typed payloads. |
| go/internal/e2e/multi_client_test.go | Updates multi-client assertions for assistant/perms to use typed payloads. |
| go/internal/e2e/mcp_and_agents_test.go | Updates MCP/agent E2E assertions to use typed payloads. |
| go/internal/e2e/compaction_test.go | Updates compaction-success/message assertions to use typed payloads. |
| go/internal/e2e/commands_and_elicitation_test.go | Updates commands/capabilities checks to use typed payloads. |
| .github/workflows/codegen-check.yml | Removes the prior workaround that excluded go/generated_session_events.go from codegen checks. |
| .gitattributes | Forces LF for generated files to reduce cross-platform diffs and marks them as generated. |
Copilot's findings
- Files reviewed: 20/21 changed files
- Comments generated: 3
071c3f8 to
98127fb
Compare
This comment has been minimized.
This comment has been minimized.
Replace the flat quicktype-generated Data union struct with per-event-type data structs, matching the C# codegen approach. Adding a new session event type can no longer change the struct shape of existing event types. - Replace generateSessionEvents() in scripts/codegen/go.ts with custom codegen that produces 74 per-event data structs, a SessionEventData interface, and custom UnmarshalJSON/MarshalJSON on SessionEvent - Update go/session.go to use idiomatic Go type switches for event dispatch - Update samples, tests, and doc examples for the new API - Add type/const aliases for PermissionRequest, Attachment, and related types to ease migration - Add .gitattributes eol=lf for generated files to prevent cross-platform line ending diffs - Re-enable the codegen-check CI workflow (remove go/generated_session_events.go revert workaround) - Add ui, uri, mime to Go initialisms for correct field naming
Prefix all generated type comments with the type name per Go convention (e.g., '// SessionStartData session initialization metadata.').
This reverts commit 8e5423b.
a9f904e to
385e6df
Compare
This comment has been minimized.
This comment has been minimized.
385e6df to
a8aa961
Compare
Update all Go code examples in docs/, test/scenarios/, and one E2E test to use per-event type assertions instead of the old flat event.Data.FieldName pattern. Fix staticcheck SA5011 lint warning in tools_test.go (nil check must precede type assertion).
a8aa961 to
f5ed23e
Compare
Cross-SDK Consistency ReviewThis PR is Go-specific, addressing a genuine codegen bug by replacing the quicktype-generated flat ✅ Aligned with .NETThe new Go approach ( ✅ Node.js/TypeScript already type-safeNode.js uses TypeScript discriminated union types for
|
Fixes #1031.
Problem
The Go codegen for session events used quicktype, which represented the discriminated union of all session event
Datapayloads as a single flat struct with every property from every event type merged together — all as optional pointers. Adding a new event type with a same-named-but-different-typed property silently changed the shared field's type, breaking all existing consumers.Solution
Replace quicktype-based session events generation with custom codegen that produces per-event-type data structs, matching the C# codegen approach. Each event type gets its own data struct (e.g.,
SessionStartData,AssistantMessageData), andSessionEvent.Datais now aSessionEventDatainterface accessed via type switches:Adding event type B never changes the struct shape of event type A.
Changes
scripts/codegen/go.ts— ReplacedgenerateSessionEvents()with custom codegen producing 74 per-event data structs,SessionEventDatainterface, customUnmarshalJSON/MarshalJSONonSessionEvent, andRawSessionEventDatafor forward-compatible handling of unknown event types.go/generated_session_events.go— Regenerated output with per-event structs, shared nested types, enums, and type/const aliases for backward compatibility (PermissionRequest,Attachment,AttachmentType*,PermissionRequestKind*).go/session.go— UpdatedhandleBroadcastEvent()andSendAndWait()to use idiomatic Go type switches. Updated doc examples.go/samples/chat.go— Updated to use type switches.go/session_test.go— Updated to construct events with per-event data structs.event.Data.*access patterns to use type assertions.go/rpc/generated_rpc.go— Minor renames from addingui,uri,mimeto Go initialisms (Ui→UI,Uri→URI— correct Go convention)..github/workflows/codegen-check.yml— Removed thegit checkout -- go/generated_session_events.goworkaround..gitattributes— Addedeol=lffor generated files to prevent cross-platform line ending diffs.Breaking changes
event.Datais now aSessionEventDatainterface instead of a flatDatastruct. Consumers must use type switches or type assertions to access per-event fields.SessionRpc.Ui→SessionRpc.UI,UiApi→UIApi,FormatUri→FormatURI(correct Go initialism convention).Testing
go build ./...— cleango vet ./...— clean