[kmannislands]

Fixes EarthBuild/earthbuild#1033.

Problem

An imported registry cache (--cache-from, or the import side of --remote-cache) is never used below a local COPY once part of that state has been solved on its own first. The triggers are a LET/ARG with $(...), an IF, a FOR, a WAIT block, or the per-target solves of VERSION 0.8. The import itself succeeds. Every step below that point re-executes on a fresh buildkitd anyway.

Two facts set it up:

• The key of a local source is session-scoped (random:) and is never exported. The registry cache can only match the steps below a local COPY through that COPY's content-based key. That's the checksum of the copied files, which the solver computes from the source's result ("slow cache").
• EarthBuild emits new vertices for the same steps in each solve of a session. The solver merges each new edge into the existing equivalent edge once their index keys match.

flowchart LR
    base["FROM alpine<br/>image key"]
    src["local source f.txt<br/>random:S<br/>(session-scoped)"]
    content["content key C<br/>(checksum of f.txt)"]
    kloc["COPY key K_loc<br/>local cache only"]
    kreg["COPY key K_reg<br/>registry cache"]
    run["RUN record<br/>in registry cache"]

    base --> kloc
    src --> kloc
    base --> kreg
    content --> kreg
    kreg --> run
    kloc -. "no link: random:S is never exported" .-x run

Loading

The sequence that loses the cache hit:

sequenceDiagram
    autonumber
    participant F as earth (frontend)
    participant A as COPY edge A<br/>(sub-solve)
    participant B as COPY edge B<br/>(main solve)
    participant R as RUN edge
    participant L as local cache
    participant G as registry cache

    Note over F,G: Sub-solve for LET V="$(cat f.txt)"
    F->>A: solve
    A->>G: query via random:S
    G-->>A: no match
    A->>G: query via content key C
    G-->>A: K_reg
    A->>L: load the COPY from G, store it locally
    Note over L: random:S and C both now link to K_loc

    Note over F,G: Main solve, with new vertices for the same steps
    F->>B: solve
    B->>G: query via random:S
    G-->>B: no match
    B->>L: query via random:S
    L-->>B: K_loc
    B->>R: keys [K_loc]
    R->>G: probe K_loc
    G-->>R: no match
    Note over A,B: B has the same index key as A, so B merges into A
    A->>R: keys [K_reg]
    rect rgb(255, 235, 235)
        Note over R: Before: seen 1 key, update has 1 key, so nothing new. K_reg is never probed and RUN executes
    end
    rect rgb(230, 245, 230)
        Note over R: After: K_reg was not seen, so it is probed
        R->>G: probe K_reg
        G-->>R: RUN record, loaded from cache
    end

Loading

The new edge B reports K_loc to its consumer. B then merges into edge A, which finished in the sub-solve. The consumer's request is redirected to A, and its next update carries A's keys, [K_reg]. But processUpdate only probed the keys past the length it had already seen (len(dep.keys) < len(state.keys)). That assumes a dependency's keys only grow. They do within one edge, but not across a merge. Here both lists have length 1, so K_reg was never probed. A RUN's rootfs input has no slow-cache function, so noCacheMatchPossible latches and the step executes. Every step after it misses too.

The issue proposed forcing the content key to be recomputed when a match came only through random: keys. That isn't needed. The edge that computed the content key is still alive in the same job, and the new edges merge into it. The only gap is that the consumer ignored the merged edge's keys. This keeps the change minimal, as AGENTS.md asks.

Change

solver/edge.go: processUpdate probes every key in a dependency update that it hasn't seen before. Keys are compared by identity (*CacheKey).

• The new unseenKeys helper returns the tail exactly as before when the update extends the keys already seen, which is the usual case, and doesn't allocate.
• Only after a merge does it build a set and return the keys that weren't seen.

flowchart TD
    upd["Update from merged edge A<br/>keys seen so far: [K_loc]<br/>state.keys: [K_reg]"]
    subgraph before ["Before"]
        b1{"len(seen) less than<br/>len(state.keys)?"}
        b2["no, 1 vs 1: nothing probed<br/>RUN executes"]
        b1 --> b2
    end
    subgraph after ["After: unseenKeys"]
        a1{"does state.keys<br/>extend the seen keys?"}
        a2["probe the new tail<br/>(the usual case, same as before)"]
        a3["probe every key not seen,<br/>compared by identity: K_reg<br/>registry hit, RUN loaded from cache"]
        a1 -- yes --> a2
        a1 -- no --> a3
    end
    upd --> b1
    upd --> a1

Loading

Probing the merged edge's keys can't produce a false match:

• Edges are merged only when their index keys match.
• After the merge, the consumer's dependency result already comes from that edge. Probing its keys is exactly what a consumer subscribed to it directly would do.
• mergeTo refuses to merge an ignore-cache edge into one that uses the cache. An ignore-cache edge never probes, so it reports no keys.

The diff is solver/edge.go +36/−2 (call site, plus the helper and its doc comment), and a new test.

Upstream

moby/buildkit master (656dabf81, after v0.34.0-rc2) has the same length comparison in processDepReq, and the same bug:

• The new test, ported unchanged, fails 10/10 there.
• The same patch fixes it, and the rest of upstream's solver tests still pass.

I found no upstream issue or PR for it. Related upstream work, all separate from this fix:

• 12d4b6375 "solver: advertise committed cache keys after exec" (v0.32.0) is a neighbouring case: a consumer gets zero keys from a completed shared edge.
• 224061dff / e7050b9ea fix silently dropped links when importing a remote cache (v0.33.0), another cause of cache misses.

None of these are in the fork. This PR is not being sent upstream yet.

Tests

New TestCacheSourceLookupAfterDepMerge in solver/scheduler_test.go replays the scenario with in-memory caches:

• an "imported" cache filled by an earlier session, whose storage ignores new links like a registry import
• a sub-solve
• a main solve with new vertices over the same cache keys

It checks that nothing in the main solve executes.

Results:

• Without the fix: fails 20/20 (RUN executes).
• With the fix, in golang:1.21 (the CI Go version), linux/arm64:
  • go test -mod=vendor -run TestCacheSourceLookupAfterDepMerge -count=20 ./solver/: pass. Also passes 20/20 with -race.
  • go test -mod=vendor -count=3 -skip TestJobsIntegration ./solver/: pass. TestJobsIntegration needs the integration sandbox and fails on main with missing requirements.
  • go vet -mod=vendor ./solver/: clean.
• docker buildx bake lint: pass.
• -race on all of ./solver/: about 35 tests fail on main both with and without this change, with identical failing sets. The causes are pre-existing races in the fork's graph-state debug tracker and dispatch instrumentation, plus a few upstream races. Those are being handled separately.

End to end (EarthBuild main CLI with a buildkitd image built from this branch). This is the issue's round trip: export with --max-remote-cache from a fresh daemon, wipe it, then rebuild with --cache-from on another fresh daemon.

buildkitd	pass 2
current pin (343d8bfee)	RUNs after LET $(...), IF, FOR and the WAIT chain re-execute, in 3/3 runs
this branch	all RUNs *cached*, in 4/4 runs; the controls are unchanged

The companion EarthBuild change adds test4 to tests/remote-cache. It runs this round trip earth-in-earth and asserts that the steps after LET, IF, FOR and WAIT are cached. It fails against the current pin and passes with BUILDKIT_PROJECT pointed at this branch.

Companion PR

EarthBuild/earthbuild#1036 pins both buildkit references to this branch's commit (517b9677a) and adds tests/remote-cache+test4, so the integration tests run against this change. Per EarthBuild's CONTRIBUTING, the two should merge together, with this branch fast-forwarded onto main to keep the SHA.

Prepared with AI assistance (Claude Code).

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Corrected cache lookup after dependency updates and merges, allowing eligible imported results to be reused instead of rerunning completed work.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 78d73e09-3b59-4e41-99a1-13e281755257

📥 Commits

Reviewing files that changed from the base of the PR and between f0b332c and 517b967.

📒 Files selected for processing (2)

• solver/edge.go
• solver/scheduler_test.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The solver now detects dependency cache keys that were not present in the prior key list, including when the list does not grow by appending. A new regression test checks imported-cache lookup after dependency merging.

Changes

Imported cache lookup

Layer / File(s)	Summary
Identify unseen dependency keys solver/edge.go	Dependency updates use unseenKeys to identify new keys. The helper returns an appended suffix when possible, or filters by CacheKey pointer identity otherwise.
Test lookup after dependency merging solver/scheduler_test.go	A test builds graphs in the same session and checks that the final result uses the imported cache without executing the run vertex. The test adds read-only cache-key storage whose AddLink method ignores links.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 517b9

The change is mergeable after normal checks; no actionable cache-lookup risk was established.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main solver fix: probing dependency-reported keys after an edge merge.
Linked Issues check	✅ Passed	The PR addresses the coding requirement in #1033. solver/edge.go replaces length-only dependency-key handling with identity-based unseenKeys handling. It probes keys that become visible after an e…
Out of Scope Changes check	✅ Passed	The changes stay within #1033. The production change is limited to dependency-key probing in solver/edge.go. The added solver/scheduler_test.go cache storage and regression test directly validate …

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

🔧 golangci-lint (2.13.2)

golangci-lint (2.13.2) skipped for .golangci.yml: configuration requires version: "2". Migrate older configurations using golangci-lint migrate and review the result; see https://golangci-lint.run/docs/product/migration-guide/.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

➖ Are we earthbuild yet?

No change in "earthly" occurrences

📈 Overall Progress

Branch	Total Count
main	219
This PR	219
Difference	+0

---

Keep up the great work migrating from Earthly to Earthbuild! 🚀

💡 Tips for finding more occurrences

Run locally to see detailed breakdown:

./.github/scripts/count-earthly.sh

Note that the goal is not to reach 0.
There is anticipated to be at least some occurences of earthly in the source code due to backwards compatibility with config files and language constructs.