Skip to content

Fix update-protocol-version.ts templates that break the Go and .NET builds - #2554

Open
rinceyuan wants to merge 1 commit into
github:mainfrom
rinceyuan:fix/protocol-version-generator-drift
Open

Fix update-protocol-version.ts templates that break the Go and .NET builds#2554
rinceyuan wants to merge 1 commit into
github:mainfrom
rinceyuan:fix/protocol-version-generator-drift

Conversation

@rinceyuan

@rinceyuan rinceyuan commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

nodejs/scripts/update-protocol-version.ts is the only generator for the five SdkProtocolVersion constant files, and it is wired up as npm run update:protocol-version. Its Go and C# templates have drifted from the files they are supposed to produce, so running it today reverts two deliberate fixes and breaks the Go and .NET builds.

Template Emits Committed file has
Go SdkProtocolVersion / GetSdkProtocolVersion SDKProtocolVersion / GetSDKProtocolVersion
C# namespace GitHub.Copilot.SDK; namespace GitHub.Copilot;

How it drifted

Both outputs are marked DO NOT EDIT, but both were hand-corrected later — for good reasons — without updating the templates:

Date PR Change
2026-05-06 #1164 Last change to the generator (added the Rust output)
2026-05-21 #1343 C# API review fixes — namespace settled on GitHub.Copilot
2026-06-01 #1527 Consolidate Go initialism casing — SdkSDK

Nothing caught it: codegen-check.yml runs scripts/codegen, which is a different generator and does not produce these files. No workflow runs update:protocol-version.

Reproducing the breakage

On main, before this change:

$ cd nodejs && npm run update:protocol-version
$ cd ../go && go build ./...
# github.com/github/copilot-sdk/go
.\client.go:1974:16: undefined: GetSDKProtocolVersion
$ dotnet build src/GitHub.Copilot.SDK.csproj
dotnet/src/Client.cs(2174,26): error CS0103: The name 'SdkProtocolVersion' does not exist in the current context
  [GitHub.Copilot.SDK.csproj::TargetFramework=netstandard2.0]
  [GitHub.Copilot.SDK.csproj::TargetFramework=net8.0]
  [GitHub.Copilot.SDK.csproj::TargetFramework=net10.0]

SdkProtocolVersion is an internal static class, and Client.cs is in GitHub.Copilot with no using for the nested namespace, so moving it breaks the reference on every target framework.

Verification after this change

Regenerating is now a no-op — the generator reproduces all five committed files exactly:

$ cd nodejs && npm run update:protocol-version
Generating SDK protocol version constants for version 3...
  ✓ nodejs/src/sdkProtocolVersion.ts
  ✓ go/sdk_protocol_version.go
  ✓ python/copilot/_sdk_protocol_version.py
  ✓ dotnet/src/SdkProtocolVersion.cs
  ✓ rust/src/sdk_protocol_version.rs
Done!

$ git status --porcelain
                      # (no output — nothing regenerated differently)

(On a Windows checkout the five files show as modified after regenerating, because the generator writes LF while the working tree is CRLF. The content is identical — confirm with git diff --ignore-all-space.)

Builds pass with the regenerated files:

$ cd go && go build ./... && go vet ./...      # exit 0
$ cd dotnet && dotnet build src/GitHub.Copilot.SDK.csproj   # succeeds, all 3 TFMs

Also included

The header comment listed four output files; the generator has written five since #1164 added the Rust output. Added rust/src/sdk_protocol_version.rs to the list.

Not addressed here

java/sdk/src/main/java/com/github/copilot/SdkProtocolVersion.java carries the same Code generated by update-protocol-version.ts. DO NOT EDIT. header, but this script does not write it — codegen-check.yml value-checks it separately instead. That looked like a distinct concern, so I left it alone.

The generator's existing Prettier deviations are also untouched: nodejs only runs format:check/lint over src/** and test/**, and the file does not satisfy prettier --check on main either, so reformatting would have been unrelated noise.

…mitted files

The Go template emitted SdkProtocolVersion/GetSdkProtocolVersion and the C# template emitted namespace GitHub.Copilot.SDK. Both were corrected in the committed outputs by github#1527 (Go initialism casing) and github#1343 (C# API review) without updating this generator, so running it reverted them and broke the Go and .NET builds. Also add the Rust output, generated since github#1164, to the header comment.
@rinceyuan
rinceyuan requested a review from a team as a code owner September 7, 2026 02:20
Copilot AI balanced review requested due to automatic review settings September 7, 2026 02:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused changes match existing call sites and regenerate all outputs exactly.

Pull request overview

Aligns the protocol-version generator with canonical Go and .NET APIs.

Changes:

  • Fixes Go identifier casing and the C# namespace.
  • Documents the Rust output.
  • Verified all five templates match committed outputs.
File summaries
File Description
nodejs/scripts/update-protocol-version.ts Corrects generated Go/C# code and output documentation.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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.

2 participants