[PATCH 0/3] Fix promisor-remote reply missing from fetch

[PATCH 0/3] Fix promisor-remote reply missing from fetch

[Christian Couder]

This small series fixes a brown paper bag bug in the "promisor-remote"
protocol v2 capability found by Matt Cote during the NVIDIA hackathon
following the Gerrit User Summit 2026.

The client doesn't send a "promisor-remote=..." reply with the `fetch`
protocol commands. It does send such a reply with the `ls-refs` and
`bundle-uri` protocol commands. So this works by luck with a Git
server over file:// or ssh://, as the same `upload-pack` process
handles all the requests and remembers the reply from `ls-refs`.

But it doesn't work with servers that handle each request on its own,
like Git over HTTP or JGit (so Gerrit). It also doesn't work when only
object IDs are fetched, as no `ls-refs` is sent then.

In these cases, the server doesn't know that the client accepted a
promisor remote, so it doesn't allow the objects available on the
accepted promisor remotes to be missing from the pack, and fails with
"could not fetch <oid> from promisor remote".

This has been the case since the capability was introduced in v2.49,
so it would be nice to have the fix in maint too.

Other related issues were found during the hackathon, but this one is
the most annoying. The others will be addressed in separate series.

Overview of the patches
=======================

  - Patch 1/3 lets write_command_and_capabilities() send a
    promisor-remote reply, without any change in behavior.

  - Patch 2/3 is the actual fix, with new tests in t5710, and in a new
    t5711 test script for HTTP.

  - Patch 3/3 documents that the client should send the reply with
    `fetch`.

CI tests
========

Some failed, others didn't even started, see:

https://github.com/chriscool/git/actions/runs/43285535901

But the failures all seem related to the test infrastructure.

Christian Couder (3):
  connect: let write_command_and_capabilities() send a promisor-remote
    reply
  fetch-pack: ensure fetch sends a promisor-remote reply
  doc: gitprotocol-v2: send the promisor-remote reply with fetch

 Documentation/gitprotocol-v2.adoc          |   6 +
 connect.c                                  |   6 +-
 connect.h                                  |   6 +-
 fetch-object-info.c                        |   3 +-
 fetch-pack.c                               |  17 ++-
 t/meson.build                              |   1 +
 t/t5710-promisor-remote-capability.sh      |  40 +++++++
 t/t5711-promisor-remote-capability-http.sh | 126 +++++++++++++++++++++
 8 files changed, 196 insertions(+), 9 deletions(-)
 create mode 100755 t/t5711-promisor-remote-capability-http.sh


base-commit: 1891de46f79dc444a47db1c87bd0c8c76b5463f2
-- 
2.56.0

[PATCH 1/3] connect: let write_command_and_capabilities() send a promisor-remote reply

[Christian Couder]

In protocol v2, the `promisor-remote=...` reply is only written by
send_capabilities(), which is used to send the `ls-refs` and
`bundle-uri` protocol commands.

The request for the `fetch` protocol command is built by
write_command_and_capabilities() called by send_fetch_request() in
"fetch-pack.c" which cannot send that reply.

In a following commit, we will need send_fetch_request() to send that
reply.

Let's prepare for that by adding a new `promisor_remote_reply` argument
to write_command_and_capabilities(). When it is not NULL, a
`promisor-remote=...` capability with that value is written into the
request.

For now, all the callers pass NULL, so there is no change in behavior.

Signed-off-by: Christian Couder <christian.couder@gmail.com>
---
 connect.c           | 6 +++++-
 connect.h           | 6 ++++--
 fetch-object-info.c | 3 ++-
 fetch-pack.c        | 6 ++++--
 4 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/connect.c b/connect.c
index 87d2e8bccd..32243948b4 100644
--- a/connect.c
+++ b/connect.c
@@ -724,7 +724,8 @@ int server_supports(const char *feature)
 }
 
 void write_command_and_capabilities(struct strbuf *req_buf, const char *command,
-				    const struct string_list *server_options)
+				    const struct string_list *server_options,
+				    const char *promisor_remote_reply)
 {
 	const char *hash_name;
 	int advertise_sid = 0;
@@ -754,6 +755,9 @@ void write_command_and_capabilities(struct strbuf *req_buf, const char *command,
 		die(_("the server does not support algorithm '%s'"),
 		    the_hash_algo->name);
 	}
+	if (promisor_remote_reply)
+		packet_buf_write(req_buf, "promisor-remote=%s",
+				 promisor_remote_reply);
 	packet_buf_delim(req_buf);
 }
 
diff --git a/connect.h b/connect.h
index 957e5fe2b9..8914d131c7 100644
--- a/connect.h
+++ b/connect.h
@@ -37,9 +37,11 @@ void check_stateless_delimiter(int stateless_rpc,
 struct string_list;
 /*
  * Write a protocol v2 command request, along with the capability
- * advertisements, into req_buf.
+ * advertisements, into req_buf. When `promisor_remote_reply` is not
+ * NULL, a `promisor-remote` capability is sent with that value.
  */
 void write_command_and_capabilities(struct strbuf *req_buf, const char *command,
-				    const struct string_list *server_options);
+				    const struct string_list *server_options,
+				    const char *promisor_remote_reply);
 
 #endif
diff --git a/fetch-object-info.c b/fetch-object-info.c
index 0a58308f9b..7fe6e4e864 100644
--- a/fetch-object-info.c
+++ b/fetch-object-info.c
@@ -18,7 +18,8 @@ static void send_object_info_request(const int fd_out,
 {
 	struct strbuf req_buf = STRBUF_INIT;
 
-	write_command_and_capabilities(&req_buf, "object-info", server_options);
+	write_command_and_capabilities(&req_buf, "object-info",
+				       server_options, NULL);
 
 	if (ask_size)
 		packet_buf_write(&req_buf, "size");
diff --git a/fetch-pack.c b/fetch-pack.c
index 55d311c0c3..50c97f561e 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1386,7 +1386,8 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,
 	int done_sent = 0;
 	struct strbuf req_buf = STRBUF_INIT;
 
-	write_command_and_capabilities(&req_buf, "fetch", args->server_options);
+	write_command_and_capabilities(&req_buf, "fetch",
+				       args->server_options, NULL);
 
 	if (args->use_thin_pack)
 		packet_buf_write(&req_buf, "thin-pack");
@@ -2269,7 +2270,8 @@ void negotiate_using_fetch(const struct oid_array *negotiation_restrict_tips,
 					   the_repository, "%d",
 					   negotiation_round);
 		strbuf_reset(&req_buf);
-		write_command_and_capabilities(&req_buf, "fetch", server_options);
+		write_command_and_capabilities(&req_buf, "fetch",
+					       server_options, NULL);
 
 		packet_buf_write(&req_buf, "wait-for-done");
 
-- 
2.56.0

[PATCH 2/3] fetch-pack: ensure fetch sends a promisor-remote reply

[Christian Couder]

In protocol v2, when the client accepts promisor remotes, the
`promisor-remote=...` reply is only written by send_capabilities(),
which is used to send the `ls-refs` and `bundle-uri` protocol commands.

The request for the `fetch` protocol command is built by
write_command_and_capabilities() called by send_fetch_request() in
"fetch-pack.c" which doesn't send that reply. do_fetch_pack_v2() calls
promisor_remote_reply(), but only to mark the accepted remotes, as
e15a6b2f8b (promisor-remote: change promisor_remote_reply()'s
signature, 2026-02-16) assumed that fetch-pack would not need to send
a reply.

With file:// and ssh:// URLs on a Git server, one upload-pack process
handles both `ls-refs` and `fetch`. The acceptance recorded during
`ls-refs` by promisor_remote_receive() in "serve.c" is still there at
`fetch` time, so the clone succeeds. But it fails when each request is
handled on its own, for example with HTTP where different upload-pack
processes are used, or with other server implementations like JGit.

It also fails when only object IDs are fetched, as no `ls-refs`
command is then sent. In these cases, the server doesn't know that the
client accepted a promisor remote, so it doesn't allow promisor
objects to be missing from the pack it sends, and it fails with "could
not fetch <oid> from promisor remote".

Let's fix that by always properly sending the `promisor-remote=...`
reply when sending a `fetch` request from do_fetch_pack_v2().

The `fetch` requests sent by negotiate_using_fetch() don't ask for a
pack, so they don't need that reply.

Let's also add tests in t5710 to check that the reply is sent with the
`fetch` request, and that fetching only an object ID works. For HTTP,
let's add a new t5711 test script, copying the setup from t5710.

Reported-by: Matt Cote <mcote@nvidia.com>
Signed-off-by: Christian Couder <christian.couder@gmail.com>
---
 fetch-pack.c                               |  13 ++-
 t/meson.build                              |   1 +
 t/t5710-promisor-remote-capability.sh      |  40 +++++++
 t/t5711-promisor-remote-capability-http.sh | 126 +++++++++++++++++++++
 4 files changed, 176 insertions(+), 4 deletions(-)
 create mode 100755 t/t5711-promisor-remote-capability-http.sh

diff --git a/fetch-pack.c b/fetch-pack.c
index 50c97f561e..2f1bb9d62c 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1380,14 +1380,16 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,
 			      const struct ref *wants, struct oidset *common,
 			      int *haves_to_send, int *in_vain,
 			      int sideband_all, int seen_ack,
-			      struct oidset *negotiation_include_oids)
+			      struct oidset *negotiation_include_oids,
+			      const char *promisor_remote_reply)
 {
 	int haves_added;
 	int done_sent = 0;
 	struct strbuf req_buf = STRBUF_INIT;
 
 	write_command_and_capabilities(&req_buf, "fetch",
-				       args->server_options, NULL);
+				       args->server_options,
+				       promisor_remote_reply);
 
 	if (args->use_thin_pack)
 		packet_buf_write(&req_buf, "thin-pack");
@@ -1696,11 +1698,12 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
 	int i;
 	struct strvec index_pack_args = STRVEC_INIT;
 	const char *promisor_remote_config;
+	char *accepted = NULL;
 
 	fsck_options_init(&fsck_options, the_repository, FSCK_OPTIONS_MISSING_GITMODULES);
 
 	if (server_feature_v2("promisor-remote", &promisor_remote_config))
-		promisor_remote_reply(promisor_remote_config, NULL);
+		promisor_remote_reply(promisor_remote_config, &accepted);
 
 	if (args->filter_options.choice == LOFC_AUTO) {
 		struct strbuf errbuf = STRBUF_INIT;
@@ -1778,7 +1781,8 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
 					       &haves_to_send, &in_vain,
 					       reader.use_sideband,
 					       seen_ack,
-					       &negotiation_include_oids)) {
+					       &negotiation_include_oids,
+					       accepted)) {
 				trace2_region_leave_printf("negotiation_v2", "round",
 							   the_repository, "%d",
 							   negotiation_round);
@@ -1928,6 +1932,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
 	if (negotiator)
 		negotiator->release(negotiator);
 
+	free(accepted);
 	fsck_options_clear(&fsck_options);
 	oidset_clear(&common);
 	oidset_clear(&negotiation_include_oids);
diff --git a/t/meson.build b/t/meson.build
index 1cf1d95f70..84644703f1 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -766,6 +766,7 @@ integration_tests = [
   't5704-protocol-violations.sh',
   't5705-session-id-in-capabilities.sh',
   't5710-promisor-remote-capability.sh',
+  't5711-promisor-remote-capability-http.sh',
   't5730-protocol-v2-bundle-uri-file.sh',
   't5731-protocol-v2-bundle-uri-git.sh',
   't5732-protocol-v2-bundle-uri-http.sh',
diff --git a/t/t5710-promisor-remote-capability.sh b/t/t5710-promisor-remote-capability.sh
index 463e9e00b0..e5d6f9d1c6 100755
--- a/t/t5710-promisor-remote-capability.sh
+++ b/t/t5710-promisor-remote-capability.sh
@@ -999,6 +999,46 @@ test_expect_success "clone and fetch with --filter=auto" '
 	check_missing_objects server 1 "$oid"
 '
 
+test_expect_success "clone sends the promisor-remote reply with fetch" '
+	git -C server config promisor.advertise true &&
+	test_when_finished "rm -rf client trace" &&
+
+	GIT_TRACE_PACKET="$(pwd)/trace" git clone \
+		-c remote.lop.promisor=true \
+		-c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \
+		-c promisor.acceptfromserver=All \
+		--no-local --filter="blob:limit=5k" server client &&
+
+	# Check that the reply is part of the fetch request
+	sed -n "/clone> command=fetch/,/clone> 0001/p" trace >request &&
+	test_grep "clone> promisor-remote=lop" request &&
+
+	# Check that the largest object is still missing on the server
+	check_missing_objects server 1 "$oid"
+'
+
+test_expect_success "fetch by object ID sends the promisor-remote reply" '
+	git -C server config promisor.advertise true &&
+	test_when_finished "rm -rf client trace" &&
+
+	git init client &&
+	git -C client remote add origin "$TRASH_DIRECTORY_URL/server" &&
+	git -C client config remote.lop.promisor true &&
+	git -C client config remote.lop.url "$TRASH_DIRECTORY_URL/lop" &&
+	git -C client config promisor.acceptfromserver All &&
+
+	# Fetching only an object ID skips the "ls-refs" command, so the
+	# reply can only be sent with "fetch"
+	head=$(git -C server rev-parse HEAD) &&
+	GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch --no-tags \
+		--filter="blob:limit=5k" origin "$head" &&
+	test_grep ! "fetch> command=ls-refs" trace &&
+	test_grep "fetch> promisor-remote=lop" trace &&
+
+	# Check that the largest object is still missing on the server
+	check_missing_objects server 1 "$oid"
+'
+
 test_expect_success "clone with promisor.advertise set to 'true' but don't delete the client" '
 	git -C server config promisor.advertise true &&
 
diff --git a/t/t5711-promisor-remote-capability-http.sh b/t/t5711-promisor-remote-capability-http.sh
new file mode 100755
index 0000000000..c08cca3e51
--- /dev/null
+++ b/t/t5711-promisor-remote-capability-http.sh
@@ -0,0 +1,126 @@
+#!/bin/sh
+
+test_description='handling of promisor remote advertisement over HTTP'
+
+. ./test-lib.sh
+
+if ! test_have_prereq PERL_TEST_HELPERS
+then
+	skip_all='skipping promisor remote capabilities tests; Perl not available'
+	test_done
+fi
+
+. "$TEST_DIRECTORY"/lib-httpd.sh
+start_httpd
+
+GIT_TEST_MULTI_PACK_INDEX=0
+GIT_TEST_MULTI_PACK_INDEX_WRITE_INCREMENTAL=0
+
+# The setup and helpers below are copied from t5710, except that the
+# server repository is in the HTTP document root.
+
+SERVER="$HTTPD_DOCUMENT_ROOT_PATH/server.git"
+
+check_missing_objects () {
+	git -C "$1" rev-list --objects --all --missing=print > all.txt &&
+	perl -ne 'print if s/^[?]//' all.txt >missing.txt &&
+	test_line_count = "$2" missing.txt &&
+	test "$3" = "$(cat missing.txt)"
+}
+
+initialize_server () {
+	# Repack everything first
+	git -C "$SERVER" -c repack.writebitmaps=false repack -a -d &&
+
+	# Repack without the largest object and create a promisor pack on server
+	git -C "$SERVER" -c repack.writebitmaps=false repack -a -d \
+	    --filter=blob:limit=5k --filter-to="$(pwd)/pack" &&
+	promisor_file=$(ls "$SERVER"/objects/pack/*.pack | sed "s/\.pack/.promisor/") &&
+	>"$promisor_file" &&
+
+	# Check objects missing on the server
+	check_missing_objects "$SERVER" 1 "$1"
+}
+
+copy_to_lop () {
+	oid_path="$(test_oid_to_path $1)" &&
+	path="$SERVER/objects/$oid_path" &&
+	path2="lop/objects/$oid_path" &&
+	mkdir -p $(dirname "$path2") &&
+	cp "$path" "$path2"
+}
+
+# On Windows, `pwd` returns a path like 'D:/foo/bar'. Prepend '/' to turn
+# it into '/D:/foo/bar', which is what git expects in file:// URLs on Windows.
+# On Unix, the path already starts with '/', so this is a no-op.
+pwd_path=$(pwd)
+case "$pwd_path" in
+[a-zA-Z]:*) pwd_path="/$pwd_path" ;;
+esac
+
+TRASH_DIRECTORY_URL="file://$pwd_path"
+
+test_expect_success 'setup: create "template" repository' '
+	git init template &&
+	test_commit -C template 1 &&
+	test_commit -C template 2 &&
+	test_commit -C template 3 &&
+	test-tool genrandom foo 10k >template/foo &&
+	git -C template add foo &&
+	git -C template commit -m foo
+'
+
+# A bare repo will act as a server repo with unpacked objects.
+test_expect_success 'setup: create bare "server" repository' '
+	git clone --bare --no-local template "$SERVER" &&
+	mv "$SERVER"/objects/pack/pack-* . &&
+	packfile=$(ls pack-*.pack) &&
+	git -C "$SERVER" unpack-objects --strict <"$packfile"
+'
+
+test_expect_success "setup for testing promisor remote advertisement" '
+	# Create another bare repo called "lop" (for Large Object Promisor)
+	git init --bare lop &&
+
+	# Copy the largest object from server to lop
+	obj="HEAD:foo" &&
+	oid="$(git -C "$SERVER" rev-parse $obj)" &&
+	copy_to_lop "$oid" &&
+
+	initialize_server "$oid" &&
+
+	# Configure lop as promisor remote for server
+	git -C "$SERVER" remote add lop "$TRASH_DIRECTORY_URL/lop" &&
+	git -C "$SERVER" config remote.lop.promisor true &&
+
+	git -C lop config uploadpack.allowFilter true &&
+	git -C lop config uploadpack.allowAnySHA1InWant true &&
+	git -C "$SERVER" config uploadpack.allowFilter true &&
+	git -C "$SERVER" config uploadpack.allowAnySHA1InWant true &&
+	git -C "$SERVER" config promisor.advertise true
+'
+
+# Over HTTP, each request is handled by a separate upload-pack process,
+# so the server knows that the client accepted "lop" only if the reply
+# is sent with the "fetch" request itself.
+test_expect_success "clone over HTTP with an accepted promisor remote" '
+	test_when_finished "rm -rf client trace" &&
+
+	GIT_TRACE_PACKET="$(pwd)/trace" git -c protocol.version=2 clone \
+		-c remote.lop.promisor=true \
+		-c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \
+		-c promisor.acceptfromserver=All \
+		--filter="blob:limit=5k" "$HTTPD_URL/smart/server.git" client &&
+
+	# Check that the reply is part of the fetch request
+	sed -n "/clone> command=fetch/,/clone> 0001/p" trace >request &&
+	test_grep "clone> promisor-remote=lop" request &&
+
+	# Check that the largest object was lazily fetched from lop
+	test_cmp template/foo client/foo &&
+
+	# Check that the largest object is still missing on the server
+	check_missing_objects "$SERVER" 1 "$oid"
+'
+
+test_done
-- 
2.56.0

[PATCH 3/3] doc: gitprotocol-v2: send the promisor-remote reply with fetch

[Christian Couder]

Until a previous commit, a client sent its `promisor-remote=...` reply
only with the `ls-refs` and `bundle-uri` protocol commands.

Let's clearly document that the client should send that reply with the
`fetch` command request too.

Reported-by: Matt Cote <mcote@nvidia.com>
Signed-off-by: Christian Couder <christian.couder@gmail.com>
---
 Documentation/gitprotocol-v2.adoc | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/gitprotocol-v2.adoc b/Documentation/gitprotocol-v2.adoc
index dd52fd8110..b8732173cd 100644
--- a/Documentation/gitprotocol-v2.adoc
+++ b/Documentation/gitprotocol-v2.adoc
@@ -879,6 +879,12 @@ In this case, or if the client doesn't want to use any promisor remote
 the server advertised, the client shouldn't advertise the
 "promisor-remote" capability at all in its reply.
 
+As protocol v2 is stateless (see the "Capabilities" section above),
+the client should send this reply with the `fetch` command request,
+even if it has already sent it with an earlier command request, like
+`ls-refs`. Note that when only some object IDs are fetched, the client
+might not send any `ls-refs` command request at all.
+
 On the server side, the "promisor.advertise" and "promisor.sendFields"
 configuration options can be used to control what it advertises. On
 the client side, the "promisor.acceptFromServer" and
-- 
2.56.0

Re: [PATCH 1/3] connect: let write_command_and_capabilities() send a promisor-remote reply

[Junio C Hamano]

Christian Couder <christian.couder@gmail.com> writes:

>  void write_command_and_capabilities(struct strbuf *req_buf, const char *command,
> -				    const struct string_list *server_options)
> +				    const struct string_list *server_options,
> +				    const char *promisor_remote_reply)
>  {
>  	const char *hash_name;
>  	int advertise_sid = 0;
> @@ -754,6 +755,9 @@ void write_command_and_capabilities(struct strbuf *req_buf, const char *command,
>  		die(_("the server does not support algorithm '%s'"),
>  		    the_hash_algo->name);
>  	}
> +	if (promisor_remote_reply)
> +		packet_buf_write(req_buf, "promisor-remote=%s",
> +				 promisor_remote_reply);
>  	packet_buf_delim(req_buf);
>  }

This function was designed to have a fairly generic "here we send a
command and then capability exchange based on the settings stored in
the string list" interface for extensibility.  It is highly annoying
that we are now adding a specific option here to be sent outside the
framework.

Isn't the misdesign being corrected that the "promisor-remote=..."
string was not sent as part of these server options list of strings?
Can we fix it without contaminating the API like this patch does, or
is it too late?

Re: [PATCH 1/3] connect: let write_command_and_capabilities() send a promisor-remote reply

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Christian Couder <christian.couder@gmail.com> writes:
>
>>  void write_command_and_capabilities(struct strbuf *req_buf, const char *command,
>> -				    const struct string_list *server_options)
>> +				    const struct string_list *server_options,
>> +				    const char *promisor_remote_reply)
>>  {
>>  	const char *hash_name;
>>  	int advertise_sid = 0;
>> @@ -754,6 +755,9 @@ void write_command_and_capabilities(struct strbuf *req_buf, const char *command,
>>  		die(_("the server does not support algorithm '%s'"),
>>  		    the_hash_algo->name);
>>  	}
>> +	if (promisor_remote_reply)
>> +		packet_buf_write(req_buf, "promisor-remote=%s",
>> +				 promisor_remote_reply);
>>  	packet_buf_delim(req_buf);
>>  }
>
> This function was designed to have a fairly generic "here we send a
> command and then capability exchange based on the settings stored in
> the string list" interface for extensibility.  It is highly annoying
> that we are now adding a specific option here to be sent outside the
> framework.
>
> Isn't the misdesign being corrected that the "promisor-remote=..."
> string was not sent as part of these server options list of strings?
> Can we fix it without contaminating the API like this patch does, or
> is it too late?

By this I do not mean that promisor-remote=... has to be sent as one
of the server options (i.e., making the string sent to something
silly like "server-option=promisor-remote=...").  Rather, I am
wondering if the existing server_options string list should be made
into a more generalized form so that the string list can contain
elements whose keys other than "server-option" can also be
contained (like "promisor-remote").  IOW, the caller can decide to
put "promisor-remote=...promisor remote reply..." in the list,
instead of sending it as a totally separate parameter, whose
semantics is hardcoded (i.e. the fourth parameter to this function
will be sent as a value to "promistor-remote=...").