ci: sticky-disk Docker layer caching for compose-built backend images#3885
Conversation
|
Claude finished @insipx's task —— View job PR Review - July 22, 2026 18:52 UTC
Review SummaryThe implementation is solid and addresses the stated goals. The Macroscope feedback regarding Code Quality ✅Positive observations:
Security ✅Known caveat acknowledged: This is an acceptable tradeoff for test infrastructure optimization—untrusted PRs could only poison the validation-service/anvil layer cache, not production artifacts. Performance ✅The 10GB cache limit chosen by the author is well-reasoned for this use case. Validation service cargo build layers consume several GB, and too-small limits cause prune-thrash (evict → full rebuild every run). Test CoverageNo automated test coverage for CI infrastructure changes is expected or typical. The test plan in the PR description outlines manual verification:
Recommendation: Compare "Start backend" step times before/after to validate cache effectiveness. Minor Observations
ConclusionLGTM - No blocking issues found. The implementation is clean, well-scoped, and properly addresses the optimization goal while maintaining security boundaries. |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
ApprovabilityVerdict: Approved Minor CI/CD optimization adding Docker layer caching to test workflows. No runtime or production impact - purely build performance improvement. Author owns all changed files. You can customize Macroscope's approvability policy. Learn more. |
just backend up rebuilds the validation-service and anvil images from scratch every job (pull_policy: never, no layer cache). Opt-in docker-builder input on setup-nix wires up useblacksmith/setup-docker-builder, enabled for the 8 jobs that start the backend.
4e7c6f0 to
3c5a7bc
Compare
Dismissing prior approval to re-evaluate 3c5a7bc
…lStatistics (#3886) ## Summary `DebugInformation > should return network API statistics` failed 4× today (PR #3883/#3885 CI + main): `expected 1n to be 0n` at the post-`clearAllStatistics()` assert on `getIdentityUpdatesV2`. A background identity refresh fires between clear and re-read — the same nondeterministic call the pre-clear asserts already tolerate with lower bounds (see comment in the test). Faster Blacksmith runners shifted the race odds enough to make it a repeat blocker. One-line fix: `toBe(0n)` → `toBeLessThanOrEqual(1n)` on that single counter; all other post-clear asserts stay exact. ## Test plan - [ ] browser-sdk test suite green in CI 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- Macroscope's pull request summary starts here --> <!-- Macroscope will only edit the content between these invisible markers, and the markers themselves will not be visible in the GitHub rendered markdown. --> <!-- If you delete either of the start / end markers from your PR's description, Macroscope will append its summary at the bottom of the description. --> > [!NOTE] > ### Tolerate background identity refresh calls in `DebugInformation` network statistics test > The test for `clearAllStatistics` was asserting exact zeros for all counters after clearing, but background workers can make one additional network call between the clear and the stat read. Counter assertions for `apiStatistics()` and `apiIdentityStatistics()` are relaxed from `.toBe(0n)` to `.toBeLessThanOrEqual(1n)` in [DebugInformation.test.ts](https://github.com/xmtp/libxmtp/pull/3886/files#diff-32e813a0aed87dbacec9e1e78223bc93acc1058077f73db5c74436653ebe2f1d). > > <!-- Macroscope's review summary starts here --> > > <sup><a href="https://app.macroscope.com">Macroscope</a> summarized 0a5a3d9.</sup> > <!-- Macroscope's review summary ends here --> > <!-- macroscope-ui-refresh --> <!-- Macroscope's pull request summary ends here -->
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3885 +/- ##
=======================================
Coverage 85.64% 85.64%
=======================================
Files 426 426
Lines 68005 68005
=======================================
Hits 58245 58245
Misses 9760 9760 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
2 participants
Summary
just backend uprebuilds the validation-service image (local.Dockerfile, full-repo context,pull_policy: never) and anvil image from scratch in every test job — no layer cachedocker-builderinput onsetup-nixwires upuseblacksmith/setup-docker-builder@v1(buildx builder backed by a sticky-disk layer cache, scoped per Dockerfile per repo, LWW commits)Follow-up to #3883.
Known caveat
setup-docker-builderhas no per-event commit control, so PR jobs write to the shared layer cache. Blast radius is limited to test-infra images (local.Dockerfile/anvil.Dockerfile) — the released image builds from a different Dockerfile and layer caches are per-Dockerfile.Test plan
🤖 Generated with Claude Code
Note
Add sticky-disk Docker layer caching to CI workflows via Blacksmith builder
Adds a
docker-builderinput to the setup-nix composite action that provisions a Blacksmith Docker buildx builder with a 10GB sticky-disk cache on Linux runners. All affected CI workflows (Android, Node, Browser SDK, WASM, workspace, ignored-tests tracker) opt in by passingdocker-builder: "true"to the action.Macroscope summarized 3c5a7bc.