feat(setup): add toolcache resolver for Copilot CLI behind feature flag#34918
feat(setup): add toolcache resolver for Copilot CLI behind feature flag#34918salmanmkc wants to merge 6 commits into
Conversation
Adds an opt-in path that skips the runtime `npm install -g @github/copilot`
when a compatible build is already present in the runner tool cache.
Behavior changes:
- New `setup-copilot-resolver` feature flag (default off). When enabled in a
workflow's frontmatter, the compiler emits `INPUT_INSTALL_COPILOT: 'true'`
on the setup step and adds `if: steps.setup.outputs.copilot-cached != 'true'`
to the compiler-emitted install step. With the flag off, the compiler emits
identical YAML to before (verified: golden fixtures unchanged).
- New `actions/setup/js/install_copilot_cli.cjs` resolver runs from setup.sh
when `INPUT_INSTALL_COPILOT=true`. On a hit, it appends the toolcache bin
dir to $GITHUB_PATH and writes `copilot-cached=true`. On any miss or error
(no toolcache entry, version out of range, network failure, malformed
matrix, etc.) it writes `copilot-cached=false` and exits 0 so the existing
bash installer runs as before.
- Resolver has zero npm dependencies (uses only fs/path/https) so it cannot
introduce a new install step itself.
- Two new step outputs on the setup action: `copilot-cached` and
`copilot-path`.
Resolution logic:
- Fetches compat matrix from gh-aw-actions main, falls back to bundled
`actions/setup/compat.json` on any error (5s timeout).
- Picks the first matrix row whose `max-gh-aw` covers the current compiler
version, then selects the highest cached version in
[min-agent, max-agent].
- Toolcache layout matches runner-images convention:
$RUNNER_TOOL_CACHE/copilot-cli/<version>/<arch>/{bin/copilot, ..}.complete
Tests: 23 vitest cases for the resolver covering semver parsing/comparison,
matrix row matching, range selection, arch detection, and toolcache scanning
(hit, miss, missing marker, missing binary, non-semver dir, empty cache).
All existing Go workflow tests pass unchanged.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| }; | ||
| let req; | ||
| try { | ||
| req = https.get(COMPAT_URL, { timeout: FETCH_TIMEOUT_MS }, res => { |
There was a problem hiding this comment.
Done in 3026304 — swapped https.get + buffer for fetch with AbortSignal.timeout(5000). Matches check_version_updates.cjs / send_otlp_span.cjs. Same contract (returns parsed JSON or null on any error, never throws); 23/23 unit tests still pass.
|
Just have it always on. It is more complicated to add a feature flag for this than anything. |
Node 24 ships fetch globally, so the manual https.get + chunk buffer + timeout wiring isn't needed. Replace it with fetch + AbortSignal.timeout, matching the convention in check_version_updates.cjs and send_otlp_span.cjs. Same contract: returns parsed JSON on 2xx + valid JSON, returns null on any error (network, timeout, non-200, parse failure). Never throws.
This comment has been minimized.
This comment has been minimized.
|
Hey However, this repository has a specific contribution process for community members that this PR doesn't follow:
To get this idea into the codebase, please close this PR and open a GitHub Issue instead, describing the feature in detail (the why, the how, the constraints). Your write-up here is already excellent and would make a great agentic plan! If you'd like a head start on drafting that issue, you can use this prompt with your AI assistant:
|
pelikhan
left a comment
There was a problem hiding this comment.
Add type annotations and typecheck
| const FETCH_TIMEOUT_MS = 5000; | ||
|
|
||
| function log(msg) { | ||
| console.log(`[install_copilot_cli] ${msg}`); |
There was a problem hiding this comment.
You can require shim.cjs and use core.debug/info... for logging. Makes things more consistent for agents.
There was a problem hiding this comment.
Done in 8a55a3a — require shim.cjs at the top, swapped console.log/console.error for core.info/core.warning. Same [install_copilot_cli] prefix preserved for grep-ability.
The setup-copilot-resolver feature flag added a layer of plumbing more complex than the value it provided. Drop the flag, the boolean parameter on generateSetupStep, and the dedicated INPUT_INSTALL_COPILOT env. The compiler now emits INPUT_GH_AW_VERSION for any Copilot-engine workflow, which setup.sh treats as the signal to invoke the resolver. The bash installer step gate (skip on copilot-cached=true) is now always applied for Copilot workflows, which is a no-op when the resolver reports a miss. Non-Copilot workflows are unchanged: no env var emitted, no resolver run, no golden diff.
|
Done — dropped the feature flag entirely in fb6b0ef. The resolver now runs unconditionally for any Copilot-engine workflow (signal is the compiler-emitted |
This comment has been minimized.
This comment has been minimized.
|
Actually... my bad this feature is only needed to run in agent/detection jobs + copilot engine selected. |
- Require shim.cjs for global core/context so logging routes through core.info / core.warning instead of console.log / console.error, matching the convention used across the other setup/js modules. - Add // @ts-check directive (now passes tsc --noEmit with the existing strictNullChecks config) and JSDoc type annotations on every exported function plus the ParsedSemver and CompatRow shared typedefs. - Tighten error narrowing in catch blocks (Error instance guards) and add explicit casts where the matrix payload is treated as unknown. Resolver tests (23/23) and full typecheck still pass.
This comment has been minimized.
This comment has been minimized.
|
Type annotations + typecheck addressed in 8a55a3a — added |
This comment has been minimized.
This comment has been minimized.
|
On it — bringing the flag back as compiler-emitted |
Re-introduce INPUT_INSTALL_COPILOT as the explicit opt-in for the toolcache resolver in actions/setup/setup.sh. The compiler emits this flag (alongside INPUT_GH_AW_VERSION) on the setup step env block only for jobs that actually invoke the Copilot CLI: the main agent job and the threat-detection job. Other jobs that share the setup action (activation, pre-activation, cache, unlock, safe-outputs, notify-comment, publish-assets, repo-memory, experiments) opt out, so the resolver stays a no-op there. Adds TestGenerateSetupStepEmitsInstallCopilotGate covering the three cases: copilot engine + opt-in emits both env vars; copilot engine without opt-in suppresses them; non-copilot engine ignores the opt-in.
|
Pushed
Verified across 6 parallel review passes — no blockers; one acknowledged note that custom-command Copilot workflows (workflows that set |
|
✅ smoke-ci: safeoutputs CLI comment + comment-memory run (26456672392)
|
Summary
Adds a Copilot CLI toolcache resolver to
actions/setupso workflows running the Copilot engine pick up a compatible CLI baked into the runner image instead of runningnpm install -g @github/copilotat job start.Why
Runtime
npm install -g @github/copilotadds ~10s to every workflow run and depends on the npm registry being reachable. Hosted runners now bake@github/copilotinto the tool cache ($RUNNER_TOOL_CACHE/copilot-cli/<version>/<arch>/). This change lets every Copilot-engine workflow pick up the cached binary, with a safe fallback when no compatible cached version is present.How it works
For any workflow whose engine is
copilot:INPUT_GH_AW_VERSION=<compiler version>.setup.shseesINPUT_GH_AW_VERSIONis set and invokes the resolver,actions/setup/js/install_copilot_cli.cjs.gh-aw-actions/.github/aw/compat.json(5s timeout viaAbortSignal.timeout).actions/setup/compat.jsonon any network/parse error.max-gh-awcovers the current compiler version (*always matches).[min-agent, max-agent]under$RUNNER_TOOL_CACHE/copilot-cli/.<dir>/binto$GITHUB_PATH, sets step outputscopilot-cached=trueandcopilot-path=<dir>.copilot-cached=falseand exits 0.if: steps.setup.outputs.copilot-cached != 'true', so it skips on a cache hit and runs as before on a miss.The resolver uses Node 24 native
fetchplusfsandpathonly — no new dependencies, nonpm install.Diff shape
actions/setup/action.yml— declare two new outputs:copilot-cached,copilot-path. No new inputs.actions/setup/setup.sh— invoke the resolver whenINPUT_GH_AW_VERSIONis set. Wrapped incommand -v nodeand|| trueso it cannot fail the setup step.actions/setup/compat.json(new) — bundled fallback matrix.actions/setup/js/install_copilot_cli.cjs(new) — the resolver (~270 LOC, zero deps).actions/setup/js/install_copilot_cli.test.cjs(new) — 23 vitest cases.pkg/workflow/copilot_engine_installation.go—gateStepsOnCopilotCachedhelper wraps the Copilot install step with theif:gate.pkg/workflow/compiler_yaml_step_generation.go— whenengineID == "copilot", emitINPUT_GH_AW_VERSIONon the setup step.Backward compatibility
copilot-cached=false, bash installer runs as today.command -v nodeguard), bash installer runs as today.Tests
go test ./pkg/workflow/ -count=1— passes aftermake update-goldenandmake update-wasm-golden.INPUT_GH_AW_VERSION, each Copilot installer step picks up theif:gate.npx vitest run install_copilot_cli.test.cjs— 23 cases pass; covers semver parse/compare, matrix row matching, range selection, arch detection, and toolcache scanning (hit, miss, missing marker, missing binary, non-semver dir name, empty cache).