Our changes on top of upstream buildkit (review view)

[kmannislands]

Same branch as #14, but based on the moby merge point (fa19659) so the diff shows only EarthBuild's changes, not all of upstream.

This is a review-only view — the actual merge to main is #14. Do not merge this one.

The base branch moby-merge-base is a throwaway pointer to the moby commit we merged in; keep it around while this PR is open.

[socket-security]

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	golang/​github.com/​docker/​distribution@​v2.8.3+incompatible
	golang/​github.com/​grpc-ecosystem/​go-grpc-middleware@​v1.4.0

View full report

[gemini-code-assist]

Code Review

This pull request introduces several Earthly-specific enhancements to Buildkit, including an embedded registry proxy, worker GC analytics, configurable session timeouts, and localhost execution capabilities. The review feedback highlights several critical issues: potential data loss and deadlocks in the registry proxy, severe performance overhead on the scheduler's hot path due to eager string formatting, a potential out-of-bounds panic in the client dialer, and file corruption risks in localhost file transfers. Additionally, there are concurrency bugs in image registration cleanup, session history tracking issues, and unstable API response ordering in the session history endpoint.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

The current Read implementation does not shrink s.last after copying its contents to p. If len(s.last) > len(p), the uncopied bytes are completely lost when s.stream.Recv() is called and overwrites s.last. Additionally, if s.last already has buffered data, the function should satisfy the read request immediately without blocking on s.stream.Recv().

Suggested change

	func (s *StreamRW) Read(p []byte) (int, error) {
	l := 0
	if len(s.last) > 0 {
	l = copy(p, s.last)
	}
	msg, err := s.stream.Recv()
	if err != nil {
	return 0, err
	}
	s.last = msg.GetData()
	n := copy(p, s.last)
	s.last = s.last[n:]
	return n + l, nil
	}
	func (s *StreamRW) Read(p []byte) (int, error) {
	if len(s.last) > 0 {
	n := copy(p, s.last)
	s.last = s.last[n:]
	return n, nil
	}
	msg, err := s.stream.Recv()
	if err != nil {
	return 0, err
	}
	s.last = msg.GetData()
	n := copy(p, s.last)
	s.last = s.last[n:]
	return n, nil
	}

[gemini-code-assist]

Eagerly appending to helpMe on the hot path should be removed to avoid performance overhead.

Suggested change

	helpMe = append(helpMe, fmt.Sprintf("hasActiveOutgoing %v", e.hasActiveOutgoing))
	// earthly-specific: removed eager helpMe append

[gemini-code-assist]

If address does not contain ://, strings.Split(address, "://") returns a slice of length 1. Accessing split[1] without checking len(split) > 1 will cause an index out of bounds panic.

Suggested change

	if useDefaultDialer {
	split := strings.Split(address, "://")
	if len(split) > 0 {
	address = split[1]
	}
	}
	if useDefaultDialer {
	split := strings.Split(address, "://")
	if len(split) > 1 {
	address = split[1]
	}
	}

[gemini-code-assist]

Opening the destination file with os.O_CREATE|os.O_WRONLY without os.O_TRUNC will cause data corruption if the file already exists and is larger than the new file being copied, as the leftover bytes of the old file will remain at the end.

Suggested change

	f, err := os.OpenFile(dest, os.O_CREATE|os.O_WRONLY, os.FileMode(mode))
	f, err := os.OpenFile(dest, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, os.FileMode(mode))

[gemini-code-assist]

Eagerly constructing helpMe with fmt.Sprintf and formatting the entire edge struct (%+v) on every single dispatch call introduces massive CPU overhead and GC pressure on the scheduler's hot path. This should be lazily constructed only when the validation error actually occurs.

Suggested change

	helpMe := []string{"v1"} // earthly specific
	helpMe = append(helpMe, fmt.Sprintf("%s: %+v", e.edge.Vertex.Name(), e))
	helpMe = append(helpMe, fmt.Sprintf("%d %d %d %d", len(s.incoming), len(s.outgoing), len(s.incoming[e]), len(s.outgoing[e])))
	// earthly-specific helpMe is lazily constructed only on error to avoid overhead

[gemini-code-assist]

Construct the helpMe diagnostic slice lazily only when the scheduler validation actually fails.

		e.keysDidChange = false
	}

	// validation to avoid deadlocks/resource leaks:
	// TODO: if these start showing up in error reports they can be changed
	// to error the edge instead. They can only appear from algorithm bugs in
	// unpark(), not for any external input.
	if len(openIncoming) > 0 && len(openOutgoing) == 0 {
		helpMe := []string{"v1"}
		helpMe = append(helpMe, fmt.Sprintf("%s: %+v", e.edge.Vertex.Name(), e))
		helpMe = append(helpMe, fmt.Sprintf("%d %d %d %d", len(s.incoming), len(s.outgoing), len(s.incoming[e]), len(s.outgoing[e])))
		helpMe = append(helpMe, fmt.Sprintf("hasActiveOutgoing %v", e.hasActiveOutgoing))
		helpMe = append(helpMe, fmt.Sprintf("make openIncoming %d", len(inc)))
		for _, r := range s.incoming[e] {
			helpMe = append(helpMe, fmt.Sprintf("%+v", r.Sender.Status()))
		}
		helpMe = append(helpMe, fmt.Sprintf("in: %d out: %d", len(openIncoming), len(openOutgoing)))
		bklog.G(context.TODO()).Errorf("return leaving incoming open help me: %s\n", strings.Join(helpMe, ";"))
		e.markFailed(pf, errors.New("buildkit scheduler error: return leaving incoming open. Please report this with BUILDKIT_SCHEDULER_DEBUG=1"))
	}
	if len(openIncoming) == 0 && len(openOutgoing) > 0 {
		helpMe := []string{"v1"}
		helpMe = append(helpMe, fmt.Sprintf("%s: %+v", e.edge.Vertex.Name(), e))
		helpMe = append(helpMe, fmt.Sprintf("%d %d %d %d", len(s.incoming), len(s.outgoing), len(s.incoming[e]), len(s.outgoing[e])))
		helpMe = append(helpMe, fmt.Sprintf("hasActiveOutgoing %v", e.hasActiveOutgoing))
		helpMe = append(helpMe, fmt.Sprintf("make openOutgoing %d", len(out)))
		for _, r := range s.outgoing[e] {
			helpMe = append(helpMe, fmt.Sprintf("%+v", r.Receiver.Status()))
		}
		helpMe = append(helpMe, fmt.Sprintf("in: %d out: %d", len(openIncoming), len(openOutgoing)))
		bklog.G(context.TODO()).Errorf("return leaving outgoing open help me: %s\n", strings.Join(helpMe, ";"))
		e.markFailed(pf, errors.New("buildkit scheduler error: return leaving outgoing open. Please report this with BUILDKIT_SCHEDULER_DEBUG=1"))
	}

[gemini-code-assist]

The cleanup goroutine deletes the image by name (imgName) when the context is canceled. However, if a new image with the same name is registered under a different context before the old context is canceled, the old context's cancellation will incorrectly delete the new image from the provider. We should check that the image currently in the map is the same instance before deleting it.

	go func() {
		<-ctx.Done()
		mmp.mu.Lock()
		defer mmp.mu.Unlock()
		if mmp.imgs[imgName] == imgData {
			mmp.maybeDelete(imgName)
		}
	}()

[gemini-code-assist]

If BUILDKIT_SESSION_HISTORY_DURATION is not configured, sm.historyDuration defaults to 0. This causes time.Since(history.Start) > sm.historyDuration to always evaluate to true, wiping out the entire session history immediately when any session ends. We should only perform the cleanup if historyDuration is greater than zero.

	if sm.historyDuration > 0 {
		for id, history := range sm.history {
			if time.Since(history.Start) > sm.historyDuration {
				delete(sm.history, id)
			}
		}
	}

[gemini-code-assist]

Because of the check if len(img.platforms) != 0 at line 530, len(img.platforms) is guaranteed to be 0 in the else branch. Therefore, the condition if len(img.platforms) > 0 at line 536 is dead code and can never be executed. We can simplify this block by directly assigning ref = img.expSrc.Ref.

Suggested change

	if img.expSrc.Ref != nil { // This is a copy and paste of the above code
	if len(img.platforms) != 0 {
	return nil, nil, nil, errors.New("img.platforms should not be set when a single ref is used")
	}
	var ref cache.ImmutableRef
	var p exptypes.Platform
	if len(img.platforms) > 0 {
	p = img.platforms[0]
	if r, ok := img.expSrc.FindRef(p.ID); ok {
	ref = r
	} else {
	return nil, nil, nil, errors.Errorf("img.expSrc.FindRef failed on %s", p.ID)
	}
	} else {
	ref = img.expSrc.Ref
	}
	if img.expSrc.Ref != nil {
	if len(img.platforms) != 0 {
	return nil, nil, nil, errors.New("img.platforms should not be set when a single ref is used")
	}
	ref := img.expSrc.Ref

[gemini-code-assist]

The SessionHistory API returns session history entries from a map, which has a randomized iteration order in Go. This makes the returned list unstable and inconsistent across calls. Sorting the history chronologically by Start time provides a deterministic and stable response. Note that you will need to import the "sort" package in control/control.go.

func (c *Controller) SessionHistory(ctx context.Context, r *controlapi.SessionHistoryRequest) (*controlapi.SessionHistoryResponse, error) {
	history := c.opt.SessionManager.GetSessionHistory()
	var resp []*controlapi.SessionHistoryResponse_History
	for id, h := range history {
		resp = append(resp, &controlapi.SessionHistoryResponse_History{
			SessionID: id,
			Start:     timestamppb.New(h.Start),
			End:       timestamppb.New(h.End),
		})
	}
	sort.Slice(resp, func(i, j int) bool {
		return resp[i].Start.AsTime().Before(resp[j].Start.AsTime())
	})
	return &controlapi.SessionHistoryResponse{History: resp}, nil
}

[github-actions]

⚠️ Are we earthbuild yet?

Warning: "earthly" occurrences have increased by 28 (12.9%)

📈 Overall Progress

Branch	Total Count
main	217
This PR	245
Difference	+28 (12.9%)

📁 Changes by file type:

File Type	Change
Go files (.go)	❌ +25
Documentation (.md)	➖ No change
Earthfiles	➖ No change

---

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.