fix(registry): end a proxied response on half-close, not 50ms of silence

[kmannislands]

The session-registry proxy decided that an HTTP response had ended when its socket went quiet for 50ms. docker pull from the embedded registry therefore fails intermittently with a bare EOF and no status, which is the server half of EarthBuild/earthbuild#912.

failed to copy: httpReadSeeker: failed open: failed to do request: Get
  "https://127.0.0.1:PORT/v2/sess-ID/pullping/blobs/sha256:...": EOF

What was wrong

CopyWithDeadline set a 50ms read deadline before every read and, on the timeout, declared the response complete and returned; Proxy then closed both the loopback connection and the gRPC stream. A registry that pauses longer than 50ms mid-body has not finished, and on a loaded runner — the -race suite pulling a 27-layer image — it pauses for longer routinely. The client gets a body shorter than the Content-Length it was promised, and reports the only thing it can: EOF.

Because that one timer governed the whole connection, keep-alive was broken too: the tunnel was dismantled after its first response, so a second request on the same connection was never answered. It hung rather than failed, because Proxy also discarded errgroup.WithContext's derived context, leaving the request direction blocked in Recv forever. And both copy directions assigned to a single captured err, a data race — under -race, which is the suite this flake shows up in.

The fix

A byte tunnel does not need to know where a response ends, and this one now doesn't ask. Copy ends each direction when its source ends it — io.EOF from the socket, or a peer that closed its send direction — and passes that end on as a half-close, so the far side can finish what it still owes.

That is not a new design here: it is the shape session/sshforward/copy.go has used to carry forwarded agent sockets for years, in this repository, and session/socketforward alongside it. The same problem outside this repo is solved the same way — kubectl port-forward opens a stream per accepted connection and terminates it by close (client-go/tools/portforward/portforward.go, handleConnection); there is no idle heuristic anywhere in it. Nothing in the registry ecosystem hand-parses HTTP framing to proxy it, so neither do we: net/http frames the bytes at both ends, and the tunnel stays opaque, which keeps it indifferent to ranges, chunked pushes, and the TLS probe docker sends before falling back to HTTP on a loopback registry.

StreamRW goes with it, and its removal is a relief rather than a loss. Its Read copied leftover bytes into the caller's buffer, then overwrote them with a fresh Recv and returned the sum of both copies — a count that can exceed len(p), which makes an io.Copy caller slice past its buffer's capacity and panic. Leftovers were dropped on error, including at a clean end of stream. It was latent only because a leftover appears only when a message exceeds the reader's buffer, and both pumps read 32KiB while both senders sent at most 32KiB.

Tests

The package had none. The first commit adds them and fails: a real net/http server stands in for the embedded registry, a real net/http client drives it, the production Proxy sits between the two, and the client half of the tunnel is written out in the test rather than borrowed from earthly's regproxy, so the server is exercised against an independently correct peer.

	before	after
a response the registry pauses 300ms mid-body	truncated at 65536 of 131072 bytes	intact
a second request on a kept-alive connection	never answered; hangs until the client's timeout	answered on the same connection
serving any response, under -race	data race on a shared err	clean
a 256KiB request body	byte-exact	byte-exact

One test earned its keep by first being wrong: without a deliberate gap between the two requests, both fit inside a single 50ms idle window and it passed against the broken code.

Behaviour worth knowing about

The proxy now holds the stream and the loopback connection for as long as the client holds its connection, instead of dropping them 50ms after the first response. That is the same lifetime port-forward gives a forwarded connection, and teardown still arrives with the session context.

Lockstep

CopyWithDeadline and StreamRW are gone, and earthly's regproxy called both, so this needs a companion earthly PR that moves regproxy.handle onto registry.Copy and bumps the pin. That change is written and verified locally against this branch — regproxy builds and its tests pass — and I'll raise it once this merges and there is a commit to pin. EarthBuild/earthbuild#884 (client-side pull retry) is complementary and worth keeping: it covers daemons that predate this fix, and transport failures this one doesn't address.

🤖 Generated with Claude Code

[github-actions]

⚠️ Are we earthbuild yet?

Warning: "earthly" occurrences have increased by 2 (0.92%)

📈 Overall Progress

Branch	Total Count
main	217
This PR	219
Difference	+2 (0.92%)

📁 Changes by file type:

File Type	Change
Go files (.go)	❌ +2
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.

[kmannislands]

@coderabbitai review pls

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== Changed files =='
git diff --name-status HEAD^ HEAD

echo '== Diff stat =='
git diff --stat HEAD^ HEAD

echo '== Diff =='
git diff --find-renames --find-copies --unified=80 HEAD^ HEAD

echo '== Candidate source maps =='
fd -t f . | rg '(^|/)(registry|copy|stream|socketforward|sshforward).*\.go$' | sort | while IFS= read -r f; do
  echo "--- $f"
  ast-grep outline "$f" --items all --type function,method,struct,interface 2>/dev/null || true
done

echo '== Related symbol references =='
rg -n -C 5 '\b(CopyWithDeadline|StreamRW|registry\.Copy|Copy\s*\(|Proxy\s*\()' --glob '*.go' . || true

echo '== Module and Go version =='
if [ -f go.mod ]; then sed -n '1,80p' go.mod; fi

Length of output: 201251

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== Changed paths =='
git diff --name-status HEAD^ HEAD

echo '== Diff name/status with numstat =='
git diff --numstat HEAD^ HEAD

echo '== Matching non-vendor paths =='
fd -t f -E vendor . | rg '(^|/)(registry|socketforward|sshforward)(/|$)|(^|/)(registry|copy|stream).*\.go$' | sort

echo '== Registry-related changed diff =='
git diff --find-renames --unified=100 HEAD^ HEAD -- 'session/registry/**' ':!vendor/**'

echo '== Current registry source and tests =='
for f in $(fd -t f -E vendor . session | rg '^session/registry/.*\.go$' | sort); do
  echo "--- $f"
  cat -n "$f"
done

echo '== Parent registry source =='
for f in $(git diff --name-only HEAD^ HEAD -- 'session/registry/*.go'); do
  echo "--- $f (HEAD^)"
  git show "HEAD^:$f" | cat -n || true
done

echo '== Analogous forwarding copies =='
for f in session/sshforward/copy.go session/socketforward/copy.go; do
  if [ -f "$f" ]; then
    echo "--- $f"
    cat -n "$f"
  fi
done

echo '== Non-vendor references to removed or changed API =='
rg -n -C 4 --glob '*.go' -g '!vendor/**' '\b(CopyWithDeadline|StreamRW)\b|\bregistry\.Copy\s*\(' . || true

Length of output: 7523

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== Exact diff =='
git diff --unified=120 HEAD^ HEAD -- api/services/registry/copy.go

echo '== Current copy implementation =='
cat -n api/services/registry/copy.go

echo '== Parent copy implementation =='
git show HEAD^:api/services/registry/copy.go | cat -n

echo '== Registry server structure =='
ast-grep outline api/services/registry/server.go --items all --type function,method,struct,interface

echo '== Proxy and stream call context =='
rg -n -C 60 --glob '*.go' -g '!vendor/**' '\bfunc \(.*\) Proxy\s*\(|\bCopy\s*\(' api/services/registry

echo '== Protocol stream definitions =='
rg -n -C 20 --glob '*.{proto,go}' -g '!vendor/**' 'service .*Registry|rpc .*Proxy|type .*Proxy.*Server|type .*Proxy.*Client|CloseSend' api/services/registry

echo '== Registry tests =='
cat -n api/services/registry/proxy_test.go

echo '== All package-local API references, including tests =='
rg -n -C 8 --glob '*.go' -g '!vendor/**' '\b(Copy|CopyWithDeadline|StreamRW|Proxy)\b' api/services/registry

Length of output: 76755

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== Pull request metadata =='
gh pr view 24 --repo EarthBuild/buildkit --json number,baseRefName,baseRefOid,headRefName,headRefOid,commits,files --jq '{number,baseRefName,baseRefOid,headRefName,headRefOid, commits:[.commits[]|{oid,messageHeadline}], files:[.files[]|{path,additions,deletions}]}'

echo '== Local refs and merge base =='
git branch -a --contains HEAD || true
git show -s --format='HEAD %H%nparents %P%nsubject %s' HEAD
git merge-base HEAD origin/main 2>/dev/null || true
git log --oneline --decorate -12 HEAD

echo '== Full PR diff from GitHub =='
gh pr diff 24 --repo EarthBuild/buildkit --color=never

echo '== io.Reader contract =='
go env GOROOT
grep -n -A35 -B5 'type Reader interface' "$(go env GOROOT)/src/io/io.go"

Length of output: 25760

---

@kmannislands: I found one correctness issue.

Copy discards bytes when conn.Read(buf) returns n > 0 with a non-io.EOF error. The io.Reader contract requires callers to process returned bytes before they process the error. A deadline or transport error can occur after valid bytes were read. Line 112 returns before send(n), so the peer can receive a truncated HTTP message.

Send buf[:n] when n > 0 before returning the read error. Keep the current io.EOF half-close logic after that send.

The regression tests cover n > 0 with io.EOF, but they do not cover n > 0 with another read error. Add a focused Copy unit test for that case.

You are interacting with an AI system.