Use GitHub Releases for SDK runtime artifacts - #2505
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Runtime asset staging has unresolved cache-integrity, Go installation, and cross-platform test issues.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 1
New issues introduced by this change (4)
| Severity | Finding |
|---|---|
 |
dotnet/src/build/GitHub.Copilot.SDK.targets — An incomplete cache is reacquired by overlaying the verified archive onto the existing directory:… |
 |
go/internal/embeddedcli/embeddedcli.go — When RuntimeExecutable/RuntimeAssets are configured without the optional in-process… |
|
go/internal/embeddedcli/embeddedcli_test.go — This test is named for Path, but it calls RuntimePath, which goes through the pre-existing… |
|
python/test_cli_download.py — This assertion hard-codes POSIX separators, but the Python test matrix includes Windows, where… |
What changed in this PR
Moves Python, Go, and .NET runtime acquisition to checksum-verified Copilot CLI Release artifacts while removing Java’s remaining npm dependency.
Changes:
- Adds unified Release-backed runtime staging and caching.
- Fetches Java schemas from verified Release archives.
- Updates tests, documentation, and CI workflows.
| File | Description |
|---|---|
python/test_cli_download.py |
Expands runtime provisioning tests. |
python/README.md |
Documents Release-based staging. |
python/copilot/_ffi_runtime_host.py |
Updates layout terminology. |
python/copilot/_cli_version.py |
Defines Release asset naming. |
python/copilot/_cli_download.py |
Implements verified bundle staging. |
java/sdk/src/main/java/com/github/copilot/ffi/NativeRuntimeLoader.java |
Updates layout documentation. |
java/sdk/pom.xml |
Removes npm schema-update profile. |
java/scripts/codegen/package.json |
Adds schema fetching and tests. |
java/scripts/codegen/package-lock.json |
Removes Copilot npm dependencies. |
java/scripts/codegen/java.ts |
Reads staged Release schemas. |
java/scripts/codegen/fetch-schemas.test.mjs |
Tests schema extraction. |
java/scripts/codegen/fetch-schemas.mjs |
Downloads and verifies schemas. |
java/pom.xml |
Removes the Java npm version pin. |
java/copilot-native/scripts/fetch-native.mjs |
Uses Release artifact terminology. |
java/copilot-native/pom.xml |
Updates packaging documentation. |
go/README.md |
Documents Release-backed bundling. |
go/internal/embeddedcli/embeddedcli.go |
Installs the adjacent runtime pair. |
go/internal/embeddedcli/embeddedcli_test.go |
Adds runtime-pair coverage. |
go/cmd/bundler/main.go |
Downloads verified Release bundles. |
go/cmd/bundler/main_test.go |
Tests checksums and Release downloads. |
dotnet/test/Unit/MSBuildTargetsTests.cs |
Adds Release server integration tests. |
dotnet/src/build/GitHub.Copilot.SDK.targets |
Implements verified Release acquisition. |
dotnet/README.md |
Documents runtime acquisition options. |
.github/workflows/update-copilot-dependency.yml |
Simplifies shared CLI updates. |
.github/workflows/java-smoke-test.yml |
Uses shared Copilot setup. |
.github/workflows/java-codegen-fix.md |
Updates agent codegen guidance. |
.github/workflows/java-codegen-fix.lock.yml |
Regenerates workflow metadata. |
.github/workflows/java-codegen-check.yml |
Tests and runs schema fetching. |
.github/workflows/java-adapt-handwritten-code-to-accept-upgrade-changes.md |
Updates CLI-upgrade instructions. |
.github/workflows/java-adapt-handwritten-code-to-accept-upgrade-changes.lock.yml |
Regenerates workflow metadata. |
.github/actions/setup-copilot/action.yml |
Exposes the JavaScript CLI path. |
Files not reviewed (1)
- java/scripts/codegen/package-lock.json: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Thanks for the migration work here — I manually validated two concrete behaviors:
|
|
@SteveSandersonMS Addressed in 2cf1b9c: incomplete .NET runtime caches are now removed entirely before reacquisition, and the regression test verifies that a stale retained asset survives neither in the refreshed cache nor in build output. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: daec7b3b-799c-4396-b372-6eb638d0faf8
Remove Java and Rust changes because those SDKs were already release-backed, simplify Python's direct runtime staging, and drop the misleading .NET npm URL alias. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: daec7b3b-799c-4396-b372-6eb638d0faf8
Fetch Java codegen schemas from the checksum-verified CLI release artifact and use the shared release-backed CLI setup for smoke tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: daec7b3b-799c-4396-b372-6eb638d0faf8
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: daec7b3b-799c-4396-b372-6eb638d0faf8
2cf1b9c to
29128c6
Compare
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
Review tier: Lite
Findings: 3
New issues introduced by this change (3)
| Severity | Finding |
|---|---|
|
go/cmd/bundler/main.go — The bundler uses http.Get without a client timeout. If the remote endpoint stalls (or a proxy… |
|
java/scripts/codegen/fetch-schemas.mjs — This schema fetcher hard-depends on a tar executable being available and functional on the host.… |
|
python/copilot/_cli_download.py — The hostless extraction path sanitizer does not explicitly reject backslashes (\\\\) in… |
Issues resolved since last review (4)
| Severity | Finding |
|---|---|
|
python/test_cli_download.py — This assertion hard-codes POSIX separators, but the Python test matrix includes Windows, where… View resolved comment |
|
go/internal/embeddedcli/embeddedcli_test.go — This test is named for Path, but it calls RuntimePath, which goes through the pre-existing… View resolved comment |
|
go/internal/embeddedcli/embeddedcli.go — When RuntimeExecutable/RuntimeAssets are configured without the optional in-process… View resolved comment |
|
dotnet/src/build/GitHub.Copilot.SDK.targets — An incomplete cache is reacquired by overlaying the verified archive onto the existing directory:… View resolved comment |
Files not reviewed (1)
- java/scripts/codegen/package-lock.json: Generated file
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: daec7b3b-799c-4396-b372-6eb638d0faf8
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: daec7b3b-799c-4396-b372-6eb638d0faf8
SDK Consistency ReviewReviewed the full PR diff (via Summary of changes: This PR is a documentation/naming cleanup (npm-registry terminology → GitHub-release terminology) plus completing an in-progress migration of the CLI/runtime download mechanism from npm-based distribution to GitHub Releases-based distribution ( Cross-SDK check:
Conclusion: No cross-SDK consistency issues found. The change maintains parity — it's precisely the kind of change that removes prior inconsistency (Go/Python/Java/.NET previously depended on npm registry access; now all six SDKs consistently use GitHub Releases).
|
Continues #2463, completing the transition away from consuming the
@github/copilotnpm package from the SDKs, instead getting thegithub-copilot-*.tgzasset from GitHub Releases, which already packages the needed SDK-facing runtime assets. This paves the way to removing the duplicated runtime.node from the npm package, which is getting dangerously big and close to the npmjs limit.#2463 did this for the Node.js SDK, Rust and Java (partially); this completes the work for the other SDKs.
The language SDKs should obtain Copilot CLI runtime components from one canonical source instead of extracting them from
@github/copilotnpm packages. This keeps runtime acquisition consistent and avoids carrying SDK-facing runtime payloads in the CLI npm package.This change moves Python, .NET, and Go to the checksum-verified
github-copilot-<version>-<platform>.tgzassets published bygithub/copilot-cli. Java and Rust runtime packaging were already Release-backed after #2463; Java's remaining schema-codegen and smoke-test dependency on@github/copilotis also removed. Java codegen now extracts schemas from the same verified Release artifact, while smoke tests use the shared Release-backed CLI setup.Runtime compatibility entrypoints continue to use
copilot-runtime[.exe]besideruntime.node, preserving existing SDK behavior. Downloads are verified against the Release'sSHA256SUMS.txt, and incomplete caches are not treated as valid.Validation