widget-creator: resilient Copilot CLI build + refresh vendored MXC binaries - #906
Conversation
…naries Make the GitHub.Copilot.SDK build-time CLI acquisition resilient and internal/external-aware, and refresh the vendored MXC sandbox binaries. Copilot CLI acquisition (build/Reactor.CopilotCli.targets, imported from Directory.Build.targets): - The SDK's targets download copilot.exe with an unauthenticated MSBuild DownloadFile hardcoded to registry.npmjs.org and <Error> on failure (MSB3923), breaking `dotnet build`/`run` on offline/proxied/TLS-intercepted networks. The CLI is a runtime-only artifact. CI skips it; local dev did not. - New shared target takes over acquisition (evaluation-time, so it gates the SDK's own targets) and obtains the CLI best-effort: cached -> COPILOT_CLI_PATH -> public DownloadFile (external/OSS) -> `npm pack` honoring an authenticated .npmrc (internal Azure Artifacts feed) -> else build succeeds WITHOUT a bundled CLI plus a REACTORCLI001 warning. Inert when CopilotSkipCliDownload=true. - CopilotSdkClient resolves a CLI at run time (bundled -> COPILOT_CLI_PATH -> installed) and passes RuntimeConnection.ForStdio(path:), since the SDK default only looks for a bundled binary and ignores COPILOT_CLI_PATH. - Internal npm feed modernized to the pkgs.dev.azure.com domain (matching the NuGet feed's org/project); README documents the internal/external behavior. MXC binaries (samples/apps/widget-creator/tools/mxc/win-arm64/*): - Refresh the vendored wxc-exec + helper binaries from the latest mxc build and update the C-3 SHA-256 integrity pins in MxcBinaryManifest.cs to match. The previously vendored wxc-exec was stale; the refreshed binary enforces the network-block policy correctly (verified: default-block config denies egress, internet-allowed config permits it). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 37ad0486-7427-4b45-8216-b11a2d06d00a
There was a problem hiding this comment.
Pull request overview
This PR improves the reliability of Copilot-powered samples/tools by preventing GitHub.Copilot.SDK’s build-time copilot.exe acquisition from hard-failing on restricted/offline networks, and refreshes the integrity pins for the vendored MXC sandbox binaries used by the widget-creator sample.
Changes:
- Add a repo-wide MSBuild override (
build/Reactor.CopilotCli.targets, imported fromDirectory.Build.targets) that best-effort acquires/bundles the Copilot CLI (public download ornpm pack) and otherwise emitsREACTORCLI001instead of failing the build. - Update
widget-creator’s runtime Copilot client to explicitly pass a resolved CLI path to the SDK stdio transport when no bundled CLI is present. - Refresh
win-arm64MXC SHA-256 pins and modernize the internal npm registry URL in pipeline templates; document internal vs external behavior in the sample README.
Show a summary per file
| File | Description |
|---|---|
| samples/apps/widget-creator/Services/MxcBinaryManifest.cs | Updates win-arm64 SHA-256 pins for refreshed vendored MXC binaries. |
| samples/apps/widget-creator/Services/CopilotSdkClient.cs | Adds runtime CLI-path resolution and explicit stdio connection wiring for Copilot SDK. |
| samples/apps/widget-creator/README.md | Documents internal vs external/offline Copilot CLI acquisition behavior. |
| Directory.Build.targets | Imports the new resilient Copilot CLI acquisition targets late in the build. |
| build/Reactor.CopilotCli.targets | Implements best-effort CLI acquisition (cache/env/public download/npm pack) and graceful warning fallback. |
| build/pipelines/templates/reactor-build-steps.yml | Updates default internal npm registry endpoint to pkgs.dev.azure.com. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
samples/apps/widget-creator/Services/CopilotSdkClient.cs:114
- Enumerating and executing
copilot.exefrom every directory onPATHexpands the attack surface: when the bundled CLI is absent (e.g. offline build), any unintended/maliciouscopilot.exeearlier on PATH would be launched. Prefer requiring an explicitCOPILOT_CLI_PATH(or checking only well-known install locations) instead of probing PATH.
// Anything named copilot.exe on PATH.
var path = Environment.GetEnvironmentVariable("PATH") ?? string.Empty;
foreach (var dir in path.Split(Path.PathSeparator))
{
if (string.IsNullOrWhiteSpace(dir))
continue;
string full;
try { full = Path.Combine(dir.Trim(), "copilot.exe"); }
catch { continue; }
yield return full;
}
- Files reviewed: 6/12 changed files
- Comments generated: 2
- Review effort level: Low
| /// <summary> | ||
| /// Resolves a Copilot CLI to spawn when the SDK's bundled binary is absent. | ||
| /// Returns <c>null</c> when the bundled CLI is present (let the SDK use it) or | ||
| /// when nothing could be found (let the SDK throw its descriptive error). | ||
| /// Order: bundled next to the app → <c>COPILOT_CLI_PATH</c> → a locally | ||
| /// installed Copilot CLI (GitHub CLI install, then <c>PATH</c>). | ||
| /// </summary> |
There was a problem hiding this comment.
Done in 8704eb1 — removed the PATH probing entirely (resolution is now bundled → COPILOT_CLI_PATH → the well-known per-user GitHub CLI install location only) and updated the doc comment to match, calling out that PATH is intentionally not scanned.
| - **Offline / no feed** — the build still succeeds *without* a bundled CLI (a | ||
| `REACTORCLI001` warning is emitted). Set `COPILOT_CLI_PATH` to a local | ||
| `copilot.exe` to run generation, or just rely on `gh auth`'s CLI at run time. |
There was a problem hiding this comment.
Reworded in 8704eb1 — the offline bullet now says you need a Copilot CLI at run time (bundled or via COPILOT_CLI_PATH) and explicitly notes that gh auth only supplies credentials, not a runnable copilot.exe.
- CopilotSdkClient.ResolveCliPath no longer scans PATH for copilot.exe (a malicious/unrelated copilot.exe earlier on PATH could otherwise be launched). It now resolves only the bundled CLI, COPILOT_CLI_PATH, and the well-known per-user GitHub CLI install location; updated the doc comment to match. - README: reword the offline bullet so it no longer reads as if `gh auth` itself provides a runnable copilot.exe (it only supplies credentials). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 37ad0486-7427-4b45-8216-b11a2d06d00a
| <ContentWithTargetPath Include="$(_ReactorCopilotCacheBin)" | ||
| TargetPath="runtimes\$(_CopilotRid)\native\$(_CopilotBinary)" | ||
| CopyToOutputDirectory="PreserveNewest" /> | ||
| <ContentWithTargetPath Include="$(_ReactorCopilotRuntimeNode)" | ||
| TargetPath="runtimes\$(_CopilotRid)\native\$(_CopilotRuntimeLib)" | ||
| CopyToOutputDirectory="PreserveNewest" | ||
| Condition="Exists('$(_ReactorCopilotRuntimeNode)')" /> |
Summary
Makes the
widget-creatorsample's GitHub.Copilot.SDK build-time CLI acquisition resilient (it previously hard-faileddotnet build/dotnet runon restricted networks) and refreshes the stale vendored MXC sandbox binaries.Copilot CLI acquisition
The SDK's
GitHub.Copilot.SDK.targetsdownloadcopilot.exevia an unauthenticated MSBuildDownloadFilehardcoded toregistry.npmjs.org, then<Error>on failure (MSB3923). That breaks the build for anyone offline / behind a proxy / on a TLS-intercepted network, even though the CLI is a runtime-only artifact. CI already skips it (CopilotSkipCliDownload=true); local dev did not.build/Reactor.CopilotCli.targets(new, imported fromDirectory.Build.targets) — takes over acquisition at evaluation time (the only lever that gates the SDK's own targets) and obtains the CLI best-effort, in order:COPILOT_CLI_PATHDownloadFile(external / OSS)npm packhonoring the ambient authenticated.npmrc— the supported path for the internal Azure Artifacts npm feed (which requires authDownloadFilecan't provide)REACTORCLI001warningCopilotSkipCliDownload=true, so all CI/official/nightly pipelines are unchanged — only previously-broken local builds are fixed.CopilotSdkClient.cs— resolves a CLI at run time (bundled →COPILOT_CLI_PATH→ installed) and passesRuntimeConnection.ForStdio(path:), because the SDK's default stdio transport only looks for a bundled binary and ignoresCOPILOT_CLI_PATH.reactor-build-steps.yml— internal npm feed modernized to thepkgs.dev.azure.comdomain (matching the NuGet feed's org/project; the preferred/required endpoint). Equivalent to the legacygithub-private.pkgs.visualstudio.comform.MXC binary refresh
wxc-exec+ helper binaries undertools/mxc/win-arm64/from the latest MXC build, and updates the C-3 SHA-256 integrity pins inMxcBinaryManifest.csto match (required, or the sandbox refuses to launch). The previously vendoredwxc-execwas stale.Validation
GitHub.Copilot.SDK 1.0.7on a restricted network (gracefulREACTORCLI001skip, ~9s, no hang);COPILOT_CLI_PATHcorrectly bundles the CLI into the output.wxc-execenforces the network policy: a default block config denies egress (curl→HTTPCODE:000), an internet-allowed config permits it (200) — verified by runningwxc-execdirectly, outside the app.Notes / out of scope
win-arm64MXC binaries + pins were refreshed (this machine's arch);win-x64is unchanged.timestampconverter + accepting protocol v2) lives in the upstreamgithub/copilot-sdkrepo and is not part of this PR; a localVersionOverridebridge was intentionally left out.