From: Stefan Sperling <stsp@stsp.name>
Subject: fix serving a branch of unrelated history with gotd
To: gameoftrees@openbsd.org
Date: Wed, 13 May 2026 18:17:27 +0200

Fix a case where gotd started sending pack progress output in response
to the client sending 'done', without any ACK or NAK in-between.

I saw this happen when trying to fetch an unrelated main branch from
gotd, with completely different history and initial root commit,
into an existing repository with a different main branch history.
  
Add test cases covering got fetch against a git-upload-pack server (is
always passing), and 'got fetch' and 'git fetch' against gotd (was not
passing, i.e. both got and git fail to fetch unless this fix is applied).

ok?
 
M  lib/serve.c                 |    3+  2-
M  regress/cmdline/fetch.sh    |  102+  0-
M  regress/gotd/repo_write.sh  |  183+  0-

3 files changed, 288 insertions(+), 2 deletions(-)

commit - 91c7e219d492ac01fd8fb77133a4d5a3d7f712f2
commit + 66474b7c1b327523f0423ec5f4921d6e9ae6c841
blob - e36084ae30ef3cd5c8813f82e04ba0c23a52a299
blob + 5d19ebd9dd67070635de77c325debade916ca4e7
--- lib/serve.c
+++ lib/serve.c
@@ -803,7 +803,7 @@ serve_read(int infd, int outfd, int gotd_sock, const c
 		STATE_DONE,
 	};
 	enum protostate curstate = STATE_EXPECT_WANT;
-	int have_ack = 0, use_sidebands = 0, seen_have = 0;
+	int have_ack = 0, use_sidebands = 0, seen_have = 0, sent_nak = 0;
 	int packfd = -1;
 	size_t pack_chunksize;
 
@@ -864,6 +864,7 @@ serve_read(int infd, int outfd, int gotd_sock, const c
 				err = send_nak(outfd, chattygot);
 				if (err)
 					goto done;
+				sent_nak = 1;
 			}
 			if (curstate == STATE_EXPECT_MORE_WANT)
 				curstate = STATE_EXPECT_HAVE_OR_DONE;
@@ -908,7 +909,7 @@ serve_read(int infd, int outfd, int gotd_sock, const c
 		}
 	}
 
-	if (!seen_have) {
+	if (!seen_have || (!have_ack && !sent_nak)) {
 		err = send_nak(outfd, chattygot);
 		if (err)
 			goto done;
blob - aac6a219e2b443b516724c8444a2c326b6bdcf49
blob + ebc64209e918302ec7f357d2a756cb1ea495c017
--- regress/cmdline/fetch.sh
+++ regress/cmdline/fetch.sh
@@ -2153,6 +2153,107 @@ test_fetch_basic_http() {
 	test_done "$testroot" "$ret"
 }
 
+test_fetch_unrelated_history() {
+	local testroot=`test_init fetch_unrelated_history`
+	local testurl=ssh://127.0.0.1/$testroot
+	local commit_id=`git_show_head $testroot/repo`
+
+	got clone -q $testurl/repo $testroot/repo-clone
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "got clone command failed unexpectedly" >&2
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	got ref -l -r $testroot/repo-clone > $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "got ref command failed unexpectedly" >&2
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	echo "HEAD: refs/heads/master" > $testroot/stdout.expected
+	echo "refs/heads/master: $commit_id" >> $testroot/stdout.expected
+	echo "refs/remotes/origin/HEAD: refs/remotes/origin/master" \
+		>> $testroot/stdout.expected
+	echo "refs/remotes/origin/master: $commit_id" \
+		>> $testroot/stdout.expected
+
+	rm -rf $testroot/repo
+	mkdir $testroot/repo
+	git_init $testroot/repo
+	echo 'new history with just one file' > $testroot/repo/alpha
+	git -C $testroot/repo add alpha
+	git_commit $testroot/repo -m "adding the test tree"
+	touch $testroot/repo/.git/git-daemon-export-ok
+	local commit_id2=`git_show_head $testroot/repo`
+
+	got fetch -q -r $testroot/repo-clone > $testroot/stdout \
+		2> $testroot/stderr
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "got fetch command failed unexpectedly" >&2
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	echo -n > $testroot/stdout.expected
+
+	cmp -s $testroot/stdout $testroot/stdout.expected
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	got ref -l -r $testroot/repo > $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "got ref command failed unexpectedly" >&2
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	echo "HEAD: refs/heads/master" > $testroot/stdout.expected
+	echo "refs/heads/master: $commit_id2" >> $testroot/stdout.expected
+
+	cmp -s $testroot/stdout $testroot/stdout.expected
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	got ref -l -r $testroot/repo-clone > $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "got ref command failed unexpectedly" >&2
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	echo "HEAD: refs/heads/master" > $testroot/stdout.expected
+	echo "refs/heads/master: $commit_id" >> $testroot/stdout.expected
+	echo "refs/remotes/origin/HEAD: refs/remotes/origin/master" \
+		>> $testroot/stdout.expected
+	echo "refs/remotes/origin/master: $commit_id2" \
+		>> $testroot/stdout.expected
+
+	cmp -s $testroot/stdout $testroot/stdout.expected
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+	test_done "$testroot" "$ret"
+}
+
+
 test_parseargs "$@"
 run_test test_fetch_basic			no-sha256
 run_test test_fetch_list			no-sha256
@@ -2172,3 +2273,4 @@ run_test test_fetch_delete_remote_refs		no-sha256
 run_test test_fetch_honor_wt_conf_bflag		no-sha256
 run_test test_fetch_from_out_of_date_remote	no-sha256
 run_test test_fetch_basic_http			no-sha256
+run_test test_fetch_unrelated_history		no-sha256
blob - b24dd5fd78d5c9570776c47220b98aa18dc4e3f1
blob + 79fd816a3d8d965fa50236f65eb132af7ec334b3
--- regress/gotd/repo_write.sh
+++ regress/gotd/repo_write.sh
@@ -554,9 +554,192 @@ EOF
 	test_done "$testroot" 0
 }
 
+test_fetch_unrelated_history() {
+	local testroot=`test_init fetch_unrelated_history 1`
+
+	got clone -q ${GOTD_TEST_REPO_URL} $testroot/repo-clone
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "got clone failed unexpectedly" >&2
+		test_done "$testroot" "1"
+		return 1
+	fi
+	local commit_id0=`git_show_head $testroot/repo-clone`
+	
+	mkdir $testroot/new-repo
+	git_init $testroot/new-repo
+	git -C $testroot/new-repo symbolic-ref HEAD refs/heads/main
+
+	echo 'new history with just one file' > $testroot/new-repo/alpha
+	git -C $testroot/new-repo add alpha
+	git_commit $testroot/new-repo -m "adding the test tree"
+	for i in 1 2 3; do
+		echo 'line $i' >> $testroot/new-repo/alpha
+		git -C $testroot/new-repo add alpha
+		git_commit $testroot/new-repo -m "more changes" 
+	done
+	got tag -r $testroot/new-repo -m "tagging 1.0" 1.0 >/dev/null
+	local commit_id2=`git_show_head $testroot/new-repo`
+	local tag_id=`got ref -r "$testroot/new-repo" -l refs/tags/1.0 | \
+		awk '{print $2}'`
+
+	cp $testroot/repo-clone/got.conf $testroot/new-repo/.git/got.conf
+
+	got send -q -b main -t 1.0 -r $testroot/new-repo -f
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "got send failed unexpectedly" >&2
+		test_done "$testroot" "1"
+		return 1
+	fi
+
+	# Verify that the send operation worked fine.
+	got clone -q ${GOTD_TEST_REPO_URL} $testroot/repo-clone2
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "got fetch failed unexpectedly" >&2
+		test_done "$testroot" "1"
+		return 1
+	fi
+
+	got tree -R -r $testroot/repo-clone2 > $testroot/stdout
+	cat > $testroot/stdout.expected <<EOF
+alpha
+EOF
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	# This test checks for server-side behaviour where progress output
+	# gets generated before any ACK or NAK arrive.
+	# To trigger the problem we should not be sending any refs the
+	# server already knows about. So delete the following:
+	got ref -d -r $testroot/repo-clone refs/tags/1.0 > /dev/null
+	got fetch -X -r $testroot/repo-clone origin > /dev/null
+
+	# Rewrite main branch hisotry to avoid sending a HAVE with a
+	# commit ID which the server can find locally.
+	got ref -r $testroot/repo-clone -d refs/heads/main > /dev/null
+	mkdir $testroot/import-tree
+	echo 'parallel universe alpha' > $testroot/import-tree/alpha
+
+	got import -r $testroot/repo-clone -b main -m init \
+		$testroot/import-tree > /dev/null
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "got import failed unexpectedly" >&2
+		test_done "$testroot" 1
+		return 1
+	fi
+	local commit_id=`git_show_head $testroot/repo-clone`
+
+	cp -r $testroot/repo-clone $testroot/repo-clone3
+	cp $testroot/repo-clone/config $testroot/repo-clone3/config
+
+	# verify that unrelated history can be fetched into the initial clone.
+	# This used to fail with:
+	# got-fetch-pack: readpkt: 48:    [0x02]1 commits colored, \
+	#    0 objects found, deltify 0%[0x0d]
+	# got-fetch-pack: unexpected message from server
+	got fetch -q -r $testroot/repo-clone > $testroot/stdout \
+		2> $testroot/stderr
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "got fetch command failed unexpectedly" >&2
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	echo -n > $testroot/stdout.expected
+
+	cmp -s $testroot/stdout $testroot/stdout.expected
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	got ref -l -r $testroot/repo-clone > $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "got ref command failed unexpectedly" >&2
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	cat > $testroot/stdout.expected <<EOF
+HEAD: refs/heads/main
+refs/heads/main: $commit_id
+refs/remotes/origin/HEAD: refs/remotes/origin/main
+refs/remotes/origin/main: $commit_id2
+refs/tags/1.0: $tag_id
+EOF
+	cmp -s $testroot/stdout $testroot/stdout.expected
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	# Now try the same fetch operation against gotd with Git.
+	# This used to fail with:
+	# fatal: git fetch-pack: expected ACK/NAK, got '?1 commits colored, \
+	#     0 objects found, deltify 0%?'
+	git -C $testroot/repo-clone3 fetch > $testroot/stdout \
+		2> $testroot/stderr
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "git fetch command failed unexpectedly" >&2
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	echo -n > $testroot/stdout.expected
+
+	cmp -s $testroot/stdout $testroot/stdout.expected
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	got ref -l -r $testroot/repo-clone3 > $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "got ref command failed unexpectedly" >&2
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	cat > $testroot/stdout.expected <<EOF
+HEAD: refs/heads/main
+refs/heads/main: $commit_id
+refs/remotes/origin/HEAD: refs/remotes/origin/main
+refs/remotes/origin/main: $commit_id2
+refs/tags/1.0: $tag_id
+EOF
+	cmp -s $testroot/stdout $testroot/stdout.expected
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	test_done "$testroot" "$ret"
+}
+
 test_parseargs "$@"
 run_test test_send_basic
 run_test test_fetch_more_history
 run_test test_send_new_empty_branch
 run_test test_delete_branch
 run_test test_rewind_branch
+run_test test_fetch_unrelated_history