Fix: give actions/setup-go a cache-dependency-path in the cross-language workflow - #393
Open
AmaadMartin wants to merge 1 commit into
Open
Fix: give actions/setup-go a cache-dependency-path in the cross-language workflow#393AmaadMartin wants to merge 1 commit into
AmaadMartin wants to merge 1 commit into
Conversation
actions/setup-go@v5 enables caching by default, but with no cache-dependency-path it probes for a literal go.sum at the repository root. adk-js has no root Go module (and .gitignore ignores go.sum tree-wide), so findDependencyFile() throws, main.ts downgrades it to "Restore cache failed: Dependencies file is not found in ...", and no primary key is ever recorded -- so the post-job save no-ops too. Supplying cache-dependency-path skips that probe entirely: the value goes straight to @actions/glob's hashFiles, which has no filename policy, so the two tracked fixture go.mod files are a valid key source. Listing the two paths literally (rather than a glob) keeps them adjacent to the Install Go dependencies step, which hardcodes the same two directories.
This was referenced Jul 31, 2026
Ci: pin Node 24, install with npm ci, and fail validation when package-lock.json is out of date
#467
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
N/A — no existing issue.
Problem: Every run of
.github/workflows/cross-language-integration.ymlemits a warning annotation and then re-downloads the entire Go module graph.actions/setup-go@v5declarescachewithdefault: true, so caching is already on. But with nocache-dependency-path,restoreCache()falls back tofindDependencyFile(), which does areaddirSync(GITHUB_WORKSPACE)and looks for a literalgo.sumat the repository root. This repo has no root Go module —git ls-filesreturns exactly two tracked Go manifests and nogo.sumanywhere:tests/cross_language/a2a/go_ts/go_client/go.modtests/cross_language/a2a/ts_go/go_backend/go.mod(
.gitignoreline 11 is a bare, unanchoredgo.sum, so nogo.sumis ever committed.)So the lookup throws,
main.tscatches it and downgrades it to a warning, and the job proceeds with no cache:Because
restoreCache()throws beforecore.saveState(State.CachePrimaryKey, ...), no primary key is recorded, so the post-job save is a no-op too —Post Setup GoprintsPrimary key was not generated.Go caching in this workflow is currently inert in both directions: nothing is restored and nothing is saved, soInstall Go dependenciesdoes a coldgo mod tidyin both fixture directories on every run.Solution: Give the step the two dependency files to key the cache on. +3 lines, one file, one step:
Why
go.modworks as a key source even though the input is documented asgo.sum. Whencache-dependency-pathis set,actions/setup-goskips the root-go.sumprobe entirely and hashes the given paths directly. Fromactions/setup-go@v5src/cache-restore.ts:findDependencyFile()— the only thing that has ago.sumfilename policy — is never called.@actions/glob'shashFileshas no filename policy at all: it globs, skips anything outsideGITHUB_WORKSPACE, skips directories, and SHA-256s the rest. Patterns are newline-separated, hence the|block scalar.Why two literal paths instead of a
tests/cross_language/**/go.modglob. TheInstall Go dependenciesstep five lines below hardcodes the same two directories. Keeping both lists literal and adjacent makes the coupling visible: adding a third fixture module already requires editing that step, and now it requires editing this one too, in the same review.Deliberately not done (each is a separate change):
cache: true— it is already the default.Use Node.jsstep is left byte-identical.go-version: '1.25'stays as-is; pinning it or switching togo-version-fileis out of scope..gitignoreinggo.sumfor the two fixtures would give a stronger, checksum-verified key and a supply-chain-verifiable dependency set — but.gitignoreline 11 is an unanchoredgo.sumcovering the whole tree, so that is a maintainer policy call, deliberately left out.Collision check. Three open PRs on this fork touch the same file; none touches the
Setup Gostep, and none addscache-dependency-path:chore: cache npm downloads in the cross-language workflow@@ -16,6 +16,8 @@— addscache: npmto Use Node.jsfeat/node-version-source-of-truth@@ -16,6 +16,8 @@— addsnode-version-file: .nvmrcto Use Node.jsFix: install CI dependencies with npm ci@@ -23,7 +23,7 @@— changesnpm install→npm cion file line 26This change inserts after file line 23 (
go-version: '1.25'). #306 and #133 change line 18; #338 changes line 26. No changed line range overlaps or abuts, so a three-way merge with any of them is clean. (#306 and #133 conflict with each other — identical insertion point — which is not this change's problem and is not resolved here.)Testing Plan
Please describe the tests that you ran to verify your changes. This is required for all PRs that are not small documentation or typo fixes.
Unit Tests:
[ ] I have added or updated unit tests for my change.
[x] All unit tests pass locally.
No unit test was added, deliberately. This change adds zero executable lines. The
vitestcoverageincludelist invitest.config.tsiscore/src/**/*.ts,dev/src/**/*.ts,integrations/src/**/*.ts— a.github/workflows/*.ymlfile is not in it and cannot be. The repo has no workflow-schema test and noactionlintstep, and nothing anywhere reads.github/workflows(grep across*.ts|*.js|*.json|*.shreturns nothing). A test that parses this YAML and asserts on the stringcache-dependency-pathwould assert the diff back to itself, prove nothing about whetheractions/setup-gocaches anything, and add a file the maintainers carry forever. The evidence below is strictly stronger.Local verification against the real implementation (not a re-implementation): the two patterns were run through
@actions/glob0.7.0'shashFiles— the exact functioncache-restore.tscalls — withGITHUB_WORKSPACEset to the repo root, reading the patterns out of the committed workflow file rather than retyping them:A non-empty hash is exactly the condition
cache-restore.tsrequires (if (!fileHash) throw new Error('Some specified paths were not resolved, ...')).Negative controls — proof the check can fail. Two mutations were run through the same
hashFilescall:go.sum(whatfindDependencyFile()probes for at the root) →::debug::No matches found for glob, empty hash. This is the root cause:readdirSync(workspace).includes('go.sum')is false, sofindDependencyFile()throwsDependencies file is not found in ... Supported file pattern: go.sum— verbatim the warning in the run logs.go.moinstead ofgo.mod) →::debug::No matches found for glob, empty hash →Some specified paths were not resolved, unable to cache dependencies.The paths are load-bearing, not decorative; a misspelling reintroduces the failure under a different message.Also run locally:
python3 -c "import yaml; yaml.safe_load(open('.github/workflows/cross-language-integration.yml'))"parses, andsecretlint(the only repo hook that sees.yml, via the"*"lint-staged entry andnpx secretlint "**/*"invalidation.yaml) exits 0 on the changed file.npm run format/lintareprettier "**/*.ts"and ESLint on{js,ts}, so neither sees this file.Manual End-to-End (E2E) Tests:
Please provide instructions on how to manually test your changes, including any necessary setup or configuration.
The workflow run is the integration test. To verify:
Restore cache failed:, and it must printCache is not found(a miss — proving a key was computed and a lookup actually happened) orCache restored from key: setup-go-….Cache saved with the key: setup-go-macOS-arm64-go-…and must not printPrimary key was not generated.gh run rerun <run-id>) and re-check: Setup Go should now printCache restored from key: …with the same key, Post Setup Go should printCache hit occurred on the primary key …, not saving cache., and thego: downloadingcount in Install Go dependencies should drop.CI evidence (all four runs below are real, on this fork)
Baseline (pre-change). Run 30638145428, job
91180931362, 2026-07-31T14:19:59Z — the Cross-Language run for PR #392, 17 minutes before mine. That branch's diff is two TypeScript files (core/src/models/interactions_utils.ts,core/test/models/interactions_utils_test.ts), so its copy of this workflow is byte-identical tomain. Same workflow, samemacos-latestrunner class, no cache-dependency-path. It reproduces the bug verbatim:Run #1 — cold key. Run 30639356372 attempt 1, job
91185030271. All four required assertions hold:No line matching
Restore cache failed:; noPrimary key was not generated.; job conclusionsuccess. The cached directories in the log are/Users/runner/go/pkg/mod(GOMODCACHE) and/Users/runner/Library/Caches/go-build(GOCACHE) — the build cache is cached too, not just modules.Note the hash in that key —
4439a2d96dba9207fb8cb78986153e0e1d2a8cc3af453a8a9fc7a91cee83b98f— is byte-identical to the hash computed locally by@actions/globbefore pushing (quoted in the Testing Plan above). The local verification reproduced CI's key exactly.Runs #2 and #3 — warm key. Same commit re-run twice (
gh run rerun 30639356372), jobs91185574669and91186063882. Both identical:Annotation diff — the headline fix
Job-level annotations, straight from
GET /check-runs/<id>/annotations:Restore cache failed: Dependencies file is not found in …Node.js 20 is deprecated … actions/setup-go@v5The Node-20 deprecation notice is pre-existing on the baseline and is unrelated to this change (it is about the action's runtime, not its inputs); removing it would mean bumping
actions/setup-go, which is a different PR. This change removes exactly one standing warning, permanently, from every future run.Timings — honest measurement
Seconds, from
GET /actions/runs/<id>/jobs. One baseline sample, one cold sample, two warm samples.go: downloadingcountThe cold run is slower, as expected: 110 s vs 101 s (+9 s).
Post Setup Gogoes from 1 s to 10 s because it now actually compresses and uploads both cache directories instead of silently no-op'ing. That cost is paid once per(Go patch version × go.mod content)key.The warm runs are 61 s vs a 101 s baseline (−40 s, −40%), reproduced identically twice. Where that comes from, per step:
Install Go dependencies15 s → 0–1 s (−14 s):go mod tidyis served from the restoredGOMODCACHE.Run cross-language integration tests43 s → 11–12 s (−31 s): this is the larger term. The fixtures shell out togo run ., which compiles the module graph; the restoredGOCACHE(Go build cache, not just the module cache) serves that compilation.Setup Go2 s → 8 s (+6 s): the restore itself costs time.Two caveats I want to state rather than round away:
Install Go dependencies(15 s of a 101 s job) caps the win at ~15%. The measured 40% comes mostly fromGOCACHEaccelerating the test step, which is easy to overlook because that step is not named after Go. I would not have believed this without the per-step numbers.proxy.golang.orgper run are avoided — removing a per-run dependency on an external service and one network-flake surface.No extrapolation to other workflows, other runners, or "CI minutes saved" is offered; there is no data for that here.
One unrelated CI flake, disclosed
The first attempt at
run-tests (windows-latest)(thevalidation.yamlmatrix, not the workflow this PR edits) failed withError: Test timed out in 5000ms.atcore/test/code_executors/unsafe_local_code_executor_test.ts:145— 1 failed, 2674 passed. That test spawns a real Python interpreter and passes no vitest timeout, so it inherits the 5000 ms default; Windows process creation exceeds that on a cold runner.It is not caused by this change, and I verified that rather than asserting it: re-running the identical commit passed (job
91187171827). This PR's whole diff is three lines incross-language-integration.yml, a workflow that runs only onmacos-latestand thatvalidation.yamlnever reads. I have left the flake alone rather than bundling an unrelated fix into this PR; it is filed as separate follow-up work.All checks are green on the final commit:
run-tests(cross-language)pass,run-tests (macos-latest / ubuntu-latest / windows-latest)pass,check-licensepass.Checklist
[x] I have read the CONTRIBUTING.md document.
[x] I have performed a self-review of my own code.
[x] I have commented my code, particularly in hard-to-understand areas.
[x] I have added tests that prove my fix is effective or that my feature works.
[x] New and existing unit tests pass locally with my changes.