feat(setup): add Copilot CLI toolcache resolver with scoped release-mode opt-in#34918
feat(setup): add Copilot CLI toolcache resolver with scoped release-mode opt-in#34918salmanmkc wants to merge 14 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
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 |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Resolves conflicts in compiler_yaml_step_generation.go where main extracted generateOTLPOIDCMintStep helper and introduced a setupLines slice. Kept the 7-param generateSetupStep signature (installCopilot bool) and routed INPUT_INSTALL_COPILOT / INPUT_GH_AW_VERSION emission through setupLines so they nest under the setup step env block. Resolves test conflict in setup_step_version_test.go by keeping both TestGenerateSetupStepEmitsInstallCopilotGate and the three new OTLP mint tests, updating the OTLP tests to use the 7-param signature. Regenerated workflow lock files via make recompile to refresh fixtures for TestHashConsistencyAcrossLockFiles.
This comment has been minimized.
This comment has been minimized.
|
Did you consider updating the script that installs copilot instead? |
This comment has been minimized.
This comment has been minimized.
|
🧪 Test Quality Sentinel completed test quality analysis. |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. |
|
✅ smoke-ci: safeoutputs CLI comment + comment-memory run (26582319885)
|
🧪 Test Quality Sentinel Report✅ Test Quality Score: 100/100 — Excellent
📊 Metrics & Test Classification (26 tests analyzed)
Test Classification Details
Language SupportTests analyzed:
Verdict
📖 Understanding Test ClassificationsDesign Tests (High Value) verify what the system does:
Implementation Tests (Low Value) verify how the system does it:
Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.
|
🏗️ Design Decision Gate — ADR RequiredThis PR introduces 160 new lines in business-logic directories (over the 100-line default threshold) but does not link an Architecture Decision Record (ADR). 📄 Draft ADR committed:
📋 What to do next
Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision. ❓ Why ADRs Matter
ADRs create a searchable, permanent record of why the codebase looks the way it does — including the alternatives you rejected. Future contributors (and your future self) will thank you when they need to understand why 📋 Michael Nygard ADR Format ReferenceAn ADR must contain these four sections to be considered complete:
The draft uses the two-part variant: a human-friendly narrative on top, plus a Normative Specification in RFC 2119 language that translates the decision into testable All ADRs are stored in References: §26582322159
|
There was a problem hiding this comment.
Pull request overview
This PR adds a feature-flagged Copilot CLI “toolcache resolver” to the actions/setup action and updates the workflow compiler to (a) opt Copilot jobs into the resolver and (b) skip the existing bash-based Copilot CLI installer step when a compatible cached CLI is found on the runner.
Changes:
- Emit Copilot-only setup env (
INPUT_INSTALL_COPILOT,INPUT_GH_AW_VERSION) for jobs that actually execute Copilot (main + threat-detection). - Add
copilot-cached/copilot-pathoutputs to the setup action and gate Copilot installer steps onsteps.setup.outputs.copilot-cached != 'true'. - Update locked workflows and WASM golden fixtures to reflect the new env vars and gated installer step.
Show a summary per file
| File | Description |
|---|---|
| actions/setup/action.yml | Declares new setup-step outputs for Copilot CLI cache resolution. |
| actions/setup/compat.json | Adds bundled fallback compatibility matrix used by the resolver. |
| actions/setup/setup.sh | Invokes the resolver when opted-in via INPUT_INSTALL_COPILOT=true (non-fatal). |
| actions/setup/js/install_copilot_cli.cjs | Implements toolcache resolver (live compat fetch + bundled fallback + cache scan). |
| actions/setup/js/install_copilot_cli.test.cjs | Adds vitest coverage for resolver selection/scanning behavior. |
| pkg/workflow/compiler_yaml_step_generation.go | Extends generateSetupStep to optionally emit Copilot resolver env vars. |
| pkg/workflow/copilot_engine_installation.go | Gates Copilot installer step(s) on setup output copilot-cached. |
| pkg/workflow/compiler_main_job.go | Opts the main Copilot agent job into the resolver via installCopilot=true. |
| pkg/workflow/threat_detection.go | Opts the threat-detection job into the resolver via installCopilot=true. |
| pkg/workflow/cache.go | Updates generateSetupStep call signature (passes installCopilot=false). |
| pkg/workflow/compiler_activation_job_builder.go | Updates generateSetupStep call signature (passes installCopilot=false). |
| pkg/workflow/compiler_experiments.go | Updates generateSetupStep call signature (passes installCopilot=false). |
| pkg/workflow/compiler_pre_activation_job.go | Updates generateSetupStep call signature (passes installCopilot=false). |
| pkg/workflow/compiler_safe_outputs_job.go | Updates generateSetupStep call signature (passes installCopilot=false). |
| pkg/workflow/compiler_unlock_job.go | Updates generateSetupStep call signature (passes installCopilot=false). |
| pkg/workflow/notify_comment.go | Updates generateSetupStep call signature (passes installCopilot=false). |
| pkg/workflow/publish_assets.go | Updates generateSetupStep call signature (passes installCopilot=false). |
| pkg/workflow/repo_memory.go | Updates generateSetupStep call signature (passes installCopilot=false). |
| pkg/workflow/testdata/TestWasmGolden_AllEngines/copilot.golden | Updates golden output for Copilot: new env vars + gated installer. |
| pkg/workflow/testdata/TestWasmGolden_CompileFixtures/basic-copilot.golden | Updates golden output for Copilot: new env vars + gated installer. |
| pkg/workflow/testdata/TestWasmGolden_CompileFixtures/playwright-cli-mode.golden | Updates golden output for Copilot: new env vars + gated installer. |
| pkg/workflow/testdata/TestWasmGolden_CompileFixtures/smoke-copilot.golden | Updates golden output for Copilot: new env vars + gated installer. |
| pkg/workflow/testdata/TestWasmGolden_CompileFixtures/with-imports.golden | Updates golden output for Copilot: new env vars + gated installer. |
| .github/workflows/ab-testing-advisor.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/ace-editor.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/agent-performance-analyzer.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/agent-persona-explorer.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/agentic-token-audit.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/agentic-token-optimizer.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/archie.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/architecture-guardian.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/artifacts-summary.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/auto-triage-issues.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/bot-detection.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/brave.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/breaking-change-checker.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/chaos-pr-bundle-fuzzer.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/ci-coach.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/cli-consistency-checker.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/code-scanning-fixer.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/code-simplifier.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/constraint-solving-potd.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/contribution-check.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/copilot-cli-deep-research.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/copilot-opt.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/copilot-pr-merged-report.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/copilot-pr-nlp-analysis.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/copilot-pr-prompt-analysis.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/craft.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/daily-agent-of-the-day-blog-writer.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/daily-architecture-diagram.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/daily-assign-issue-to-user.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/daily-cli-performance.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/daily-cli-tools-tester.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/daily-community-attribution.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/daily-compiler-quality.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/daily-compiler-threat-spec-optimizer.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/daily-experiment-report.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/daily-file-diet.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/daily-firewall-report.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/daily-geo-optimizer.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/daily-hippo-learn.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/daily-issues-report.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/daily-malicious-code-scan.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/daily-mcp-concurrency-analysis.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/daily-model-inventory.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/daily-news.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/daily-performance-summary.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/daily-regulatory.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/daily-repo-chronicle.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/daily-safe-output-integrator.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/daily-secrets-analysis.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/daily-security-observability.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/daily-semgrep-scan.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/daily-sentrux-report.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/daily-skill-optimizer.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/daily-spdd-spec-planner.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/daily-syntax-error-quality.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/daily-team-status.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/daily-testify-uber-super-expert.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/daily-workflow-updater.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/dead-code-remover.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/delight.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/dependabot-campaign.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/dependabot-go-checker.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/dependabot-repair.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/dependabot-worker.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/deployment-incident-monitor.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/dev-hawk.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/dictation-prompt.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/discussion-task-miner.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/docs-noob-tester.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/draft-pr-cleanup.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/example-permissions-warning.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/firewall-escape.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/firewall.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/functional-pragmatist.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/github-remote-mcp-auth-test.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/glossary-maintainer.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/gpclean.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/hippo-embed.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/issue-monster.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/issue-triage-agent.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/jsweep.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/layout-spec-maintainer.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/lint-monster.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/linter-miner.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/mattpocock-skills-reviewer.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/mcp-inspector.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/mergefest.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/metrics-collector.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/notion-issue-summary.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/org-health-report.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/otlp-data-quality-validator.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/outcome-collector.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/pdf-summary.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/plan.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/poem-bot.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/pr-code-quality-reviewer.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/pr-description-caveman.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/pr-nitpick-reviewer.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/pr-sous-chef.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/pr-triage-agent.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/python-data-charts.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/q.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/refactoring-cadence.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/refiner.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/release.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/repo-audit-analyzer.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/repo-tree-map.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/repository-quality-improver.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/research.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/security-compliance.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/security-review.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/slide-deck-maintainer.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/smoke-ci.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/smoke-copilot-arm.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/smoke-copilot.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/smoke-create-cross-repo-pr.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/smoke-multi-pr.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/smoke-otel-backends.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/smoke-project.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/smoke-service-ports.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/smoke-temporary-id.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/smoke-test-tools.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/smoke-update-cross-repo-pr.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/smoke-workflow-call-with-inputs.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/smoke-workflow-call.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/spec-extractor.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/spec-librarian.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/stale-pr-cleanup.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/stale-repo-identifier.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/sub-issue-closer.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/super-linter.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/technical-doc-writer.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/terminal-stylist.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/test-dispatcher.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/test-project-url-default.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/test-quality-sentinel.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/test-workflow.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/tidy.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/uk-ai-operational-resilience.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/ubuntu-image-analyzer.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/update-astro.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/video-analyzer.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/visual-regression-checker.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/weekly-blog-post-writer.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/weekly-editors-health-check.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/weekly-issue-summary.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/weekly-safe-outputs-spec-review.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/workflow-generator.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/workflow-health-manager.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/workflow-normalizer.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
| .github/workflows/workflow-skill-extractor.lock.yml | Regenerates locked workflow to include resolver env + gated installer. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 178/178 changed files
- Comments generated: 0
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Non-blocking observations on 3 issues
Overall design is sound — resolver never exits non-zero, copilot-cached != 'true' gate is correctly wired in both the main agent job and the threat-detection job, and 23 unit tests cover the core resolution paths. Three non-blocking issues worth addressing before or after merge:
📋 Issue summary
-
gateStepsOnCopilotCachedfragile step-shape assumption (pkg/workflow/copilot_engine_installation.go:127) — steps that don't start with- name:silently skip theif:gate; no warning emitted, no unit test directly coversgateStepsOnCopilotCached. -
Bundled
compat.jsonceiling becomes stale (actions/setup/compat.json:9) —max-agent: "1.0.54"is a narrow window; once runner images bake in1.0.55+, the bundled fallback produces no cache hits. Consider a high ceiling (e.g."2.0.0") or a release-automation step to keep it in sync. -
Network fetch before toolcache existence check (
actions/setup/js/install_copilot_cli.cjs:286) —fetchLiveMatrix()runs even when$RUNNER_TOOL_CACHE/copilot-cli/is empty. On restricted networks this burns the full 5 s timeout for every cache miss. Checking for directory entries first would short-circuit the common no-cache case instantly.
🔎 Code quality review by PR Code Quality Reviewer · sonnet46 3.9M
| } | ||
| if !strings.HasPrefix(step[0], " - name:") { | ||
| out = append(out, step) | ||
| continue |
There was a problem hiding this comment.
Silent gate bypass when installer step format changes: if step[0] ever stops matching " - name:" (e.g. GenerateCopilotInstallerSteps is refactored to lead with - run: or - uses:), the condition is silently NOT injected — the installer runs unconditionally and the feature silently regresses to the old behaviour with no error, log warning, or CI signal.
💡 Suggested fix
Add an explicit warning log when a step is skipped so regressions surface in CI output:
if !strings.HasPrefix(step[0], " - name:") {
// gateStepsOnCopilotCached needs updating if GenerateCopilotInstallerSteps
// ever changes its step shape.
copilotInstallLog.Printf("WARNING: skipping copilot-cached gate for step with unexpected shape: %q", step[0])
out = append(out, step)
continue
}A dedicated unit test asserting the if: line appears at index 1 of the gated step would also catch this without relying solely on golden tests that can be silently updated.
| { | ||
| "max-gh-aw": "*", | ||
| "min-agent": "1.0.50", | ||
| "max-agent": "1.0.54" |
There was a problem hiding this comment.
Bundled fallback matrix becomes stale after the first release past 1.0.54: max-agent: "1.0.54" is currently the ceiling of what the bundled matrix covers. Once runner images bake in 1.0.55+, any Copilot workflow where the live fetch fails (network outage, corporate proxy, self-hosted runner) will get zero cache hits from the bundled matrix and fall back to npm install — silently defeating the purpose of having a bundled fallback.
💡 Suggested fix
Either:
- Set
max-agentto a deliberately high ceiling (e.g."2.0.0") to make the bundled row long-lived. Combined withmax-gh-aw: "*", this means the bundled matrix is always a safe fallback regardless of which version is cached. - Add a release automation step that bumps
min-agent/max-agentincompat.jsonas part of the gh-aw release process, keeping it in sync with what runner images bake.
Option 1 is simpler and lower maintenance. The version ceiling in the live matrix (at gh-aw-actions) is the authoritative source for fine-grained compatibility; the bundled matrix is only a last-resort fallback.
| } | ||
|
|
||
| // Try live matrix first, fall back to bundled. Either may be null. | ||
| let matrix = await fetchLiveMatrix(); |
There was a problem hiding this comment.
Network fetch happens before checking if any cached Copilot binary exists: fetchLiveMatrix() is called unconditionally whenever INPUT_INSTALL_COPILOT=true, even on runners where $RUNNER_TOOL_CACHE/copilot-cli/ is empty or missing. On a cache miss, the workflow waits up to 5 seconds for the raw.githubusercontent.com fetch to time out before concluding there is nothing to resolve.
💡 Suggested fix
Check whether $RUNNER_TOOL_CACHE/copilot-cli contains any entries before incurring the network round-trip:
async function resolve() {
// ...
// Fast-path: if the copilot-cli tool cache directory doesn't exist or is
// empty, skip the network fetch entirely.
const copilotCacheDir = path.join(toolCacheRoot, "copilot-cli");
let hasEntries = false;
try {
hasEntries = fs.readdirSync(copilotCacheDir).length > 0;
} catch { /* ENOENT or permission error — no cache */ }
if (!hasEntries) {
log("no copilot-cli in tool cache; skipping live matrix fetch");
writeOutput("copilot-cached", "false");
writeOutput("copilot-path", "");
return;
}
let matrix = await fetchLiveMatrix();
// ...
}This makes the common case (runner images without the cache pre-populated) nearly instantaneous.
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /tdd and /grill-with-docs — requesting changes on two correctness risks.
📋 Key Themes & Findings
Blocking
-
v-prefix inparseSemver(comment 1): if runner-images name toolcache directoriesv1.0.52rather than1.0.52, every cache lookup silently misses. This needs confirmation against the actual runner-image toolcache naming before merge. -
gateStepsOnCopilotCachedhas no Go unit tests (comment 2): the function uses fragilestrings.HasPrefixon raw YAML strings. A shape change inGenerateCopilotInstallerStepswould silently drop theif:gate from all Copilot workflows.
Non-blocking (but worth fixing)
-
Bundled
compat.jsonwill go stale (comment 3):max-agent: "1.0.54"is already close to the current version (1.0.52) with no documented update process. Recommend widening the ceiling or automating the bump on release. -
resolve()has no unit tests (comment 4): the 23 vitest cases exercise helpers thoroughly but leave the top-level orchestration unwired. A fewvi.stubEnv-based tests would prevent silent regressions. -
conditionconstant lacks trailing\n(comment 5): cosmetic inconsistency with every other YAML-line string in the compiler, but the golden tests confirm it works correctly today.
Positive Highlights
- ✅ Zero-dependency resolver — native
fetch+fsonly, no npm install required to run the resolver itself - ✅ Excellent fail-safe design: every error path exits 0 and writes
copilot-cached=false, preserving the existing installer as fallback - ✅ Golden diff confined to Copilot fixtures only — compiler change is clearly mechanical and scoped
- ✅ JSDoc typedefs and inline comments throughout
install_copilot_cli.cjsmake the intent clear - ✅
findCachedCopilottests cover the toolcache edge cases (no.completemarker, missing binary, non-semver dirs) comprehensively
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 3.9M
| */ | ||
| function parseSemver(v) { | ||
| if (typeof v !== "string") return null; | ||
| const m = v.match(/^(\d+)\.(\d+)\.(\d+)(?:-([0-9A-Za-z.-]+))?$/); |
There was a problem hiding this comment.
[/tdd] parseSemver rejects v-prefixed version strings (e.g. "v1.0.52"), but runner-image toolcache directories are commonly named with a v prefix. If that is the naming convention used by install-copilot-cli.sh in runner-images, every toolcache lookup will silently miss, returning copilot-cached=false and falling back to npm install — defeating the feature entirely.
💡 Suggested fix + test
Strip a leading v before matching:
function parseSemver(v) {
if (typeof v !== "string") return null;
const normalised = v.startsWith("v") ? v.slice(1) : v;
const m = normalised.match(/^(\d+)\.(\d+)\.(\d+)(?:-([0-9A-Za-z.-]+))?$/);
if (!m) return null;
return [Number(m[1]), Number(m[2]), Number(m[3]), m[4] || ""];
}Add a test case:
it("strips leading v-prefix", () => {
expect(parseSemver("v1.2.3")).toEqual([1, 2, 3, ""]);
});Before merging, verify the actual directory names that install-copilot-cli.sh in runner-images creates (1.0.52 vs v1.0.52). The existing test expect(parseSemver("v1.2.3")).toBeNull() should be updated or removed once the convention is confirmed.
| out = append(out, step) | ||
| continue | ||
| } | ||
| if !strings.HasPrefix(step[0], " - name:") { |
There was a problem hiding this comment.
[/tdd] gateStepsOnCopilotCached has zero Go unit tests. It performs a subtle string-manipulation on the raw YAML lines of a GitHubActionStep — if GenerateCopilotInstallerSteps ever changes its step format (e.g., adds a comment line before - name:), the gate is silently dropped and every Copilot workflow skips the if: condition, potentially breaking the fallback path.
💡 Suggested test
func TestGateStepsOnCopilotCached(t *testing.T) {
input := []GitHubActionStep{
{" - name: Install GitHub Copilot CLI\n", " run: echo hello\n"},
}
got := gateStepsOnCopilotCached(input)
require.Len(t, got, 1)
assert.Equal(t, " - name: Install GitHub Copilot CLI\n", got[0][0])
assert.Contains(t, got[0][1], "if: steps.setup.outputs.copilot-cached != 'true'")
assert.Equal(t, " run: echo hello\n", got[0][2])
}
func TestGateStepsOnCopilotCached_NonNameStep(t *testing.T) {
// Steps without - name: opener must be returned unmodified
input := []GitHubActionStep{
{" - uses: actions/checkout@abc\n"},
}
got := gateStepsOnCopilotCached(input)
require.Len(t, got, 1)
assert.Equal(t, input[0], got[0], "non-name step should be unmodified")
}| "max-gh-aw": "*", | ||
| "min-agent": "1.0.50", | ||
| "max-agent": "1.0.54" | ||
| } |
There was a problem hiding this comment.
[/grill-with-docs] The bundled fallback matrix has max-agent: "1.0.54" with the project currently at 1.0.52. There is no documented process for bumping this ceiling. Once the project ships 1.0.55+, any runner without network access (or where the live matrix fetch fails) will silently miss the cached 1.0.55 CLI and fall back to npm install — the opposite of the feature's goal.
💡 Suggestions
- Document the update process: add a note in the
_commentfield or aCONTRIBUTING.mdentry explaining thatmax-agentmust be bumped on each release. - Wider ceiling: consider setting
max-agentto a generous upper bound (e.g."99.99.99") and rely solely onmax-gh-awfor compatibility gating. The fallback matrix is already behind a live-matrix fetch, so a stale ceiling is the primary failure mode. - Automate: add a release step that updates
compat.jsonmax-agentto the new version before tagging.
| }); | ||
| }); | ||
|
|
||
| describe("findCachedCopilot", () => { |
There was a problem hiding this comment.
[/tdd] The resolve() orchestration function is exported but has no unit tests. The 23 cases cover individual helpers (semver, matrix parsing, cache scanning) well, but the end-to-end wiring is invisible to tests. Key paths that could regress silently include: RUNNER_TOOL_CACHE unset, both live and bundled matrices failing to load (null returned from both), and the case where rows.length === 0.
💡 Suggested test skeleton
describe("resolve", () => {
beforeEach(() => {
vi.stubEnv("RUNNER_TOOL_CACHE", "");
vi.stubEnv("GITHUB_OUTPUT", "");
vi.stubEnv("GITHUB_PATH", "");
vi.stubEnv("INPUT_GH_AW_VERSION", "");
});
afterEach(() => vi.unstubAllEnvs());
it("writes copilot-cached=false when RUNNER_TOOL_CACHE is unset", async () => {
await resolve(); // should not throw
// assert no output written (mocked GITHUB_OUTPUT not set)
});
});Even a smoke test that calls resolve() without throwing would prevent regressions in the top-level wiring.
| // Steps without a recognisable ` - name:` opener are returned unmodified; | ||
| // any future refactor of the installer step shape should re-verify this here. | ||
| func gateStepsOnCopilotCached(steps []GitHubActionStep) []GitHubActionStep { | ||
| const condition = " if: steps.setup.outputs.copilot-cached != 'true'" |
There was a problem hiding this comment.
[/grill-with-docs] The condition constant is the only YAML line string in this file (and likely in the codebase) that lacks a trailing \n. Every other YAML line emitted by the compiler ends with \n (e.g. " INPUT_INSTALL_COPILOT: 'true'\n"). The golden tests verify the output is correct today, but this inconsistency could confuse a future author who copies the pattern and inadvertently introduces a missing-newline bug when the surrounding step format changes.
Consider adding the trailing \n for consistency:
const condition = " if: steps.setup.outputs.copilot-cached != 'true'\n"
|
|
|
|
|
|
| GH_AW_INFO_AWF_VERSION: "v0.25.56" | ||
| GH_AW_INFO_ENGINE_ID: "copilot" | ||
| INPUT_INSTALL_COPILOT: 'true' | ||
| INPUT_GH_AW_VERSION: "947e72fa-dirty" |
There was a problem hiding this comment.
@copilot only inject this field in release mode
There was a problem hiding this comment.
Addressed in 606f46e. INPUT_INSTALL_COPILOT / INPUT_GH_AW_VERSION are now emitted only in release/action mode (with install opt-in), and removed from dev-mode generated outputs (including ace-editor.lock.yml).
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Summary
Adds a Copilot CLI toolcache resolver to
actions/setupso eligible Copilot jobs can 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 workflow runs and depends on npm registry availability. Hosted runners now bake@github/copilotinto toolcache ($RUNNER_TOOL_CACHE/copilot-cli/<version>/<arch>/). This change enables safe cached resolution while preserving existing fallback behavior.How it works
For Copilot engine workflows:
INPUT_INSTALL_COPILOT: 'true'INPUT_GH_AW_VERSION=<compiler version>setup.shchecksINPUT_INSTALL_COPILOT=trueand then invokesactions/setup/js/install_copilot_cli.cjs(guarded withcommand -v nodeand non-fatal behavior).gh-aw-actions/.github/aw/compat.json(5s timeout viaAbortSignal.timeout).actions/setup/compat.jsonon error.<dir>/binto$GITHUB_PATH, setscopilot-cached=trueandcopilot-path=<dir>.copilot-cached=falseand exits 0.if: steps.setup.outputs.copilot-cached != 'true', so it skips on cache hit and runs on miss.Diff shape
actions/setup/action.yml— addscopilot-cached,copilot-pathoutputs.actions/setup/setup.sh— runs resolver only when explicitly opted in (INPUT_INSTALL_COPILOT=true).actions/setup/compat.json(new) — fallback matrix.actions/setup/js/install_copilot_cli.cjs(new) — resolver using nativefetch.actions/setup/js/install_copilot_cli.test.cjs(new) — resolver unit tests.pkg/workflow/copilot_engine_installation.go— gates installer step on setup output.pkg/workflow/compiler_yaml_step_generation.go— emits Copilot resolver env only for opted-in jobs and only in release/action mode.pkg/workflow/setup_step_version_test.go— adds mode-sensitive coverage for env emission.Backward compatibility
Tests
make lintgo test ./pkg/workflow -count=1make testmake update-wasm-golden(to refresh affected fixtures)install_copilot_cli.test.cjs).