feat(cgo): cache repository checkout to reduce GitHub API calls per run - #48699
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Add a checkout-cache job that checks out with persist-credentials: false and saves to actions/cache. All ubuntu-latest jobs restore from cache instead of calling actions/checkout. Jobs needing fetch-depth: 0 or running on non-Linux platforms keep direct checkout. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
Thanks for working on optimizing CI performance, A couple of things that would strengthen this PR:
The logic looks sound — the If you would like to add validation, here is a prompt for your agent: Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "patchdiff.githubusercontent.com"See Network Configuration for more information.
|
|
@copilot lint go does not require fetch 0. |
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
✅ Test Quality Sentinel completed test quality analysis. No test files were added or modified in this PR. Test Quality Sentinel skipped. |
|
@copilot you missing "build & test" jobs who still checkoutg. |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #48699 does not have the 'implementation' label and has 0 new lines of code in business logic directories (threshold: 100). |
|
@copilot do the same optimzation for cjs.yml |
There was a problem hiding this comment.
Pull request overview
Caches one Linux checkout for reuse across CGO jobs, reducing redundant authenticated clones.
Changes:
- Adds a checkout-cache producer and restores it in 16 jobs.
- Retains direct checkout where full history or cross-platform support is required.
- Includes an unrelated generated workflow update.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/cgo.yml |
Adds shared checkout caching and job dependencies. |
.github/workflows/daily-spending-forecast.lock.yml |
Updates generated sink-visibility handling. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Medium
| with: | ||
| persist-credentials: false | ||
| - name: Cache repository checkout | ||
| uses: actions/cache/save@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 |
| GH_AW_SAFE_OUTPUTS: ${{ steps.set-runtime-paths.outputs.GH_AW_SAFE_OUTPUTS }} | ||
| GH_AW_SAFE_OUTPUTS_CONFIG_PATH: ${{ steps.set-runtime-paths.outputs.GH_AW_SAFE_OUTPUTS_CONFIG_PATH }} | ||
| GH_AW_SAFE_OUTPUTS_TOOLS_PATH: ${{ steps.set-runtime-paths.outputs.GH_AW_SAFE_OUTPUTS_TOOLS_PATH }} | ||
| GH_AW_SINK_VISIBILITY: ${{ steps.determine-automatic-lockdown.outputs.visibility }} |
|
@pelikhan Unfortunately I hit an unexpected error while processing your comment. I've automatically reported this to GitHub. You can ask me to try again later by mentioning me in a new comment. If you want to contact GitHub about this error, please mention the following identifier so they can better serve you: Sorry for the inconvenience! |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /codebase-design — requesting changes on resilience and cache efficiency.
📋 Key Themes & Highlights
Issues to Address
-
Cache eviction causes total failure (line 73):
fail-on-cache-miss: truewith no fallback means a stale/evicted cache (7-day TTL or 10 GB quota pressure) instantly fails all 16 downstream jobs with no recovery path. A checkout fallback step is needed. -
.gitdirectory bloats the cache (line 55):path: .includes the full.gitdirectory. Since none of the cache-consuming jobs use git commands, excluding.gitreduces cache size and I/O. -
Missing concurrency group on
checkout-cache(line 42): Every other job in this workflow has aconcurrencyblock. Without one, concurrent pushes can race to write duplicate cache entries.
Positive Highlights
- ✅ Clean use of
persist-credentials: false— no token stored in the shared cache - ✅ Correct preservation of direct checkout for
impacted-go-tests,lint-go(needfetch-depth: 0) andcross-platform-build(cross-OS) - ✅
fail-on-cache-miss: trueis good intent — the fix is adding a fallback, not removing the signal
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 42.8 AIC · ⌖ 7.78 AIC · ⊞ 6.7K
Comment /matt to run again
| with: | ||
| path: . | ||
| key: checkout-${{ github.sha }} | ||
| fail-on-cache-miss: true |
There was a problem hiding this comment.
[/codebase-design] fail-on-cache-miss: true with no fallback means all 16 downstream jobs fail hard if the cache is evicted (GitHub evicts after 7 days or at 10 GB). Authors see a cryptic "Cache not found" error with no self-healing path.
💡 Suggested fix
Add a checkout fallback step after the restore in each consumer job:
- name: Restore checkout
id: restore-checkout
uses: actions/cache/restore@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
with:
path: .
key: checkout-${{ github.sha }}
- name: Checkout (cache-miss fallback)
if: steps.restore-checkout.outputs.cache-hit != 'true'
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
persist-credentials: falseThis preserves the optimisation on warm cache while making the workflow resilient to cold starts.
@copilot please address this.
| - name: Cache repository checkout | ||
| uses: actions/cache/save@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 | ||
| with: | ||
| path: . |
There was a problem hiding this comment.
[/codebase-design] Caching path: . includes the .git directory, which can be hundreds of MB for a repository with history. Since no downstream job that uses this cache needs git history or git commands, excluding .git would make the cache leaner and faster to save/restore.
💡 Suggested fix
- name: Cache repository checkout
uses: actions/cache/save@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
with:
path: |
.
!.git
key: checkout-${{ github.sha }}Apply the same exclusion to all cache/restore steps. This can meaningfully reduce cache I/O for large repositories.
@copilot please address this.
| - 'install-gh-aw.sh' | ||
| workflow_dispatch: | ||
| jobs: | ||
| checkout-cache: |
There was a problem hiding this comment.
[/codebase-design] The checkout-cache job has no concurrency group, while all other jobs in this workflow do. If multiple commits are pushed in quick succession, multiple checkout-cache jobs will run in parallel and write duplicate cache entries for the same SHA — wasting compute and cache storage quota.
💡 Suggested fix
checkout-cache:
runs-on: ubuntu-latest
concurrency:
group: ci-${{ github.ref }}-checkout-cache
cancel-in-progress: true
timeout-minutes: 5@copilot please address this.
|
@copilot run pr-finisher skill |
| uses: actions/cache/save@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 | ||
| with: | ||
| path: . | ||
| key: checkout-${{ github.sha }} |
There was a problem hiding this comment.
Review: feat(cgo): cache repository checkout
Summary: Well-structured optimization — replacing ~16 independent actions/checkout calls with a shared actions/cache keyed on github.sha. The approach is sound: persist-credentials: false ensures no tokens are stored in the cache, jobs requiring fetch-depth: 0 correctly keep their own checkout, and cross-platform jobs are excluded. The daily-spending-forecast.lock.yml diff is an unrelated compiled change.
One non-blocking suggestion (inline comment): path: . includes the .git directory in the cache bundle. Since all jobs that restore the cache only build/test source files and don't use git history, caching .git adds size without benefit. Excluding it via ! prefix pattern would reduce cache size and upload/restore time.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 76.1 AIC · ⌖ 4.66 AIC · ⊞ 5K
| - name: Cache repository checkout | ||
| uses: actions/cache/save@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 | ||
| with: | ||
| path: . |
There was a problem hiding this comment.
Potential issue: path: . caches the .git directory
Caching path: . includes the .git directory. For repos with non-trivial history, this can make the cache artifact unexpectedly large and slow to upload/restore, potentially negating the I/O savings from skipping actions/checkout.
Consider excluding .git:
path: |
.
!.gitactions/cache supports exclude patterns with ! prefixes. Since none of the downstream restore jobs rely on git history (they already skip jobs that need fetch-depth: 0), the .git directory in the cache is unused weight.
@copilot please address this.
There was a problem hiding this comment.
🔎 Code quality review by PR Code Quality Reviewer · sonnet50 · 127 AIC · ⌖ 6.95 AIC · ⊞ 7.4K
Comment /review to run again
| - name: Cache repository checkout | ||
| uses: actions/cache/save@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 | ||
| with: | ||
| path: . |
There was a problem hiding this comment.
Caching path: . stores the entire working tree (including .git history) instead of just checked-out source, needlessly inflating usage of the shared 10GB per-repo actions/cache quota.
💡 Details
A plain actions/checkout (with default shallow clone) is typically much smaller than the full working directory tree cached here. With ~16 jobs restoring this cache every run, and other jobs also depending on Go module/npm caches sharing the same repo-wide quota, this checkout cache risks evicting more valuable dependency caches as the repo grows.
Consider excluding .git from the cached path (e.g. git archive the tracked files instead) and monitor cache eviction/usage after rollout.
| timeout-minutes: 15 | ||
| needs: | ||
| - checkout-cache | ||
| - test |
There was a problem hiding this comment.
canary-go gates on needs.test.result == 'success' but not on checkout-cache's result, so if the cache save failed upstream the job still runs and hard-fails at fail-on-cache-miss: true instead of skipping cleanly.
💡 Details
This pattern repeats for every job using if: always()/conditional needs.*.result checks combined with needs: [checkout-cache, ...] (e.g. notify-failure at the bottom of the file). Since checkout-cache's own result isn't checked in the if: condition, a checkout-cache failure surfaces as an opaque "cache miss" error in every downstream job rather than a clear "upstream checkout-cache failed" signal, making CI failures harder to triage.
Consider adding && needs.checkout-cache.result == 'success' to these conditional jobs, or fail-on-cache-miss: false plus an explicit early exit step when the cache is missing.
|
🎉 This pull request is included in a new release. Release: |
Each of the ~17 jobs in
cgo.ymlwas independently callingactions/checkout, making redundant authenticated git clone requests against GitHub on every run. This adds a singlecheckout-cachejob that stores the checkout inactions/cacheso all downstream jobs restore from cache instead.What changed
checkout-cachejob: runs first, checks out withpersist-credentials: false(no token in cache), saves working directory toactions/cachekeyed bygithub.shaactions/checkoutwithactions/cache/restore(fail-on-cache-miss: true) and gainneeds: [checkout-cache]actions/checkout:impacted-go-tests,lint-go— requirefetch-depth: 0(full git history not cacheable this way)cross-platform-build— runs on macOS/Windows; Linux cache is not safely reusable cross-platformnotify-failuregainscheckout-cachein itsneedsso it surfaces cache job failuresNet effect: ~17 authenticated git clone calls per run → 4.