Git development
 help / color / mirror / Atom feed
* [PATCH 0/2] push: fix --force-if-includes consulting wrong ref
@ 2026-09-04 21:01 Tyler Cipriani
  2026-09-04 21:01 ` [PATCH 1/2] push: check pushed ref for --force-if-includes Tyler Cipriani
                   ` (6 more replies)
  0 siblings, 7 replies; 22+ messages in thread
From: Tyler Cipriani @ 2026-09-04 21:01 UTC (permalink / raw)
  To: git
  Cc: Srinidhi Kaushik, Stefan Haller, D . Ben Knoble, Phillip Wood,
	Johannes Schindelin, Tyler Cipriani

--force-if-includes has been checking the reflog of the local branch named
after the destination branch regardless of what's being pushed. This can cause
false rejections or unintended data loss.

False rejection has been reported twice that I could find:

- 2023-07-26 - Stefan Haller reported local branch with a different name
               false rejection[0]
- 2025-05-08 - D. Ben Knoble reported detached HEAD false rejection[1]

The same root cause can result in data loss: when a same-name local branch
contains the remote tip but you --force-if-includes push an unrelated branch,
clobbering the remote repo. PoCs are in t/t5533-push-cas.sh -- new test cases
fail against maint, but pass with patches applied.

Existing tests covered refspecs with different names for --force-with-lease,
but missed --force-if-includes. New patches cover:

- allow forced-update using refspec with different-named local branch
- allow same as above, but with HEAD
- reject force-update using refspec with different-named local branch lacking
  branch tip
- reject same as above using HEAD
- reject detached HEAD

Open question: the detached HEAD case. I opted to reject, since it seems like
it might be surprising to allow in the case where you were just on a branch
without the the tip of a remote ref, removed the last commit with git checkout
HEAD^ and pushed with --force-if-includes and it allowed a destructive push.
I made a separate patch showing different advice for that case (since a
git pull won't help).

Based on maint since this is a bugfix. Happy to split patches any way
that's helpful.

[0]: <https://lore.kernel.org/git/f51c73ed-eb03-83ca-fb31-d3e2645c9a63@haller-berlin.de>
[1]: <https://lore.kernel.org/git/CALnO6CCk0SgwObQRnpd5Pt_DvCKF8dBmyVHivU6Nr_O-GusGLA@mail.gmail.com>

Tyler Cipriani (2):
  push: check pushed ref for --force-if-includes
  push: fix --force-if-includes detached HEAD advice

 Documentation/config/advice.adoc |  4 ++
 advice.c                         |  1 +
 advice.h                         |  1 +
 builtin/push.c                   | 15 +++++++
 builtin/send-pack.c              |  5 +++
 remote.c                         | 27 +++++++++++-
 remote.h                         | 10 +++--
 send-pack.c                      |  1 +
 t/t5533-push-cas.sh              | 70 +++++++++++++++++++++++++++++++-
 transport-helper.c               |  5 +++
 transport.c                      |  8 ++++
 transport.h                      |  1 +
 12 files changed, 143 insertions(+), 5 deletions(-)


base-commit: e9019fcafe0040228b8631c30f97ae1adb61bcdc
-- 
2.47.3


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 1/2] push: check pushed ref for --force-if-includes
  2026-09-04 21:01 [PATCH 0/2] push: fix --force-if-includes consulting wrong ref Tyler Cipriani
@ 2026-09-04 21:01 ` Tyler Cipriani
  2026-09-05 18:57   ` Ben Knoble
  2026-09-04 21:01 ` [PATCH 2/2] push: fix --force-if-includes detached HEAD advice Tyler Cipriani
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 22+ messages in thread
From: Tyler Cipriani @ 2026-09-04 21:01 UTC (permalink / raw)
  To: git
  Cc: Srinidhi Kaushik, Stefan Haller, D . Ben Knoble, Phillip Wood,
	Johannes Schindelin, Tyler Cipriani

"--force-if-includes" ensures, "tip of the remote-tracking ref is
reachable from one of the 'reflog' entries of the local branch."

But check_if_includes_upstream() uses the local per-branch reflog based
on the destination branch rather than the branch being pushed; using
ref->name vs. ref->peer_ref->name.

This can cause confusing rejections or unintended data loss.

Using a command like:

    git push --force-if-includes --force-with-lease origin src:main

False rejections: when src is an up-to-date branch, but main is
out-of-date or nonexistent, then the includes check will fail telling
users the remote ref has been updated since the last checkout.

Data loss: when src is an orphan/out-dated branch, but main is
up-to-date, then the if-includes check will allow the push, clobbering
the remote main.

Find local reflog using ref->peer_ref. When using a refspec like
HEAD:refs/heads/main, we resolve HEAD to a branch and use that reflog.
In a detached HEAD state, the reflog cannot tell us if the history
being pushed includes the tip of the remote, so the push is rejected.

Skip deletions:

    git push --force-if-includes --force-with-lease origin :main

ref->deletion is set after apply_push_cas (which triggers
check_if_includes_upstream). The ref->peer_ref name is "(delete)".
Instead check with is_null_oid to detect and allow deletion.

Reported-by: Stefan Haller <lists@haller-berlin.de>
Reported-by: D. Ben Knoble <ben.knoble@gmail.com>
Signed-off-by: Tyler Cipriani <tyler@tylercipriani.com>
---
 remote.c            | 24 ++++++++++++++++-
 t/t5533-push-cas.sh | 65 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 88 insertions(+), 1 deletion(-)

diff --git a/remote.c b/remote.c
index 00723b385e..326af76eeb 100644
--- a/remote.c
+++ b/remote.c
@@ -2806,7 +2806,29 @@ static int is_reachable_in_reflog(const char *local, const struct ref *remote)
  */
 static void check_if_includes_upstream(struct ref *remote)
 {
-	struct ref *local = get_local_ref(remote->name);
+	struct ref *local;
+	const char *name;
+	int flag;
+
+	if (!remote->peer_ref)
+		return;
+
+	/* A deletion has no local history to check against. */
+	if (is_null_oid(&remote->peer_ref->new_oid))
+		return;
+
+	name = remote->peer_ref->name;
+	if (!strcmp(name, "HEAD")) {
+		name = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
+					       "HEAD", 0, NULL, &flag);
+		if (!name || !(flag & REF_ISSYMREF)) {
+			/* detached HEAD: no per-branch reflog to consult */
+			remote->unreachable = 1;
+			return;
+		}
+	}
+
+	local = get_local_ref(name);
 	if (!local)
 		return;
 
diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh
index cba26a872d..0c02151747 100755
--- a/t/t5533-push-cas.sh
+++ b/t/t5533-push-cas.sh
@@ -396,4 +396,69 @@ test_expect_success '"--force-if-includes" should allow deletes' '
 	)
 '
 
+test_expect_success '"--force-if-includes" should allow forced update when using differently named branches' '
+	setup_src_dup_dst &&
+	test_when_finished "rm -fr dst src dup" &&
+	(
+		cd src &&
+		git fetch &&
+		git switch -c newbranch origin/main &&
+		git rebase HEAD --onto HEAD^ &&
+		git push --force-if-includes --force-with-lease origin newbranch:main
+	)
+'
+test_expect_success '"--force-if-includes" should allow forced update from HEAD' '
+	setup_src_dup_dst &&
+	test_when_finished "rm -fr dst src dup" &&
+	(
+		cd src &&
+		git fetch &&
+		git switch -c newbranch origin/main &&
+		git rebase HEAD --onto HEAD^ &&
+		git push --force-if-includes --force-with-lease origin HEAD:main
+	)
+'
+
+test_expect_success '"--force-if-includes" should reject forced update from differently named branches when local lacks remote ref' '
+	setup_src_dup_dst &&
+	test_when_finished "rm -fr dst src dup" &&
+	(
+		cd src &&
+		git fetch &&
+		git switch main &&
+		git reset --hard origin/main &&
+		git switch --orphan orphan &&
+		test_commit I &&
+		test_must_fail git push --force-with-lease --force-if-includes origin orphan:main
+	)
+'
+
+test_expect_success '"--force-if-includes" should reject forced update from HEAD when it lacks remote ref' '
+	setup_src_dup_dst &&
+	test_when_finished "rm -fr dst src dup" &&
+	(
+		cd src &&
+		git fetch &&
+		git switch main &&
+		git reset --hard origin/main &&
+		git switch --orphan orphan &&
+		test_commit I &&
+		test_must_fail git push --force-with-lease --force-if-includes origin HEAD:main
+	)
+'
+
+test_expect_success '"--force-if-includes" should reject forced update from detached HEAD' '
+	setup_src_dup_dst &&
+	test_when_finished "rm -fr dst src dup" &&
+	(
+		cd src &&
+		git fetch &&
+		git switch main &&
+		git reset --hard origin/main &&
+		git switch -c newbranch origin/main &&
+		git checkout HEAD^ &&
+		test_must_fail git push --force-if-includes --force-with-lease origin HEAD:main
+	)
+'
+
 test_done
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 2/2] push: fix --force-if-includes detached HEAD advice
  2026-09-04 21:01 [PATCH 0/2] push: fix --force-if-includes consulting wrong ref Tyler Cipriani
  2026-09-04 21:01 ` [PATCH 1/2] push: check pushed ref for --force-if-includes Tyler Cipriani
@ 2026-09-04 21:01 ` Tyler Cipriani
  2026-09-05 18:59 ` [PATCH 0/2] push: fix --force-if-includes consulting wrong ref Ben Knoble
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 22+ messages in thread
From: Tyler Cipriani @ 2026-09-04 21:01 UTC (permalink / raw)
  To: git
  Cc: Srinidhi Kaushik, Stefan Haller, D . Ben Knoble, Phillip Wood,
	Johannes Schindelin, Tyler Cipriani

When a --force-if-includes push is rejected due to a detached HEAD
state where there is no per-branch reflog to consult, the advice is
misleading:

     ! [rejected] HEAD -> main (remote ref updated since checkout)
    error: failed to push some refs to '<remote>'
    hint: Updates were rejected because the tip of the remote-tracking
    hint: branch has been updated since the last checkout. If you want
    hint: to integrate the remote changes, use 'git pull' before
    hint: pushing again. See the 'Note about fast-forwards' in 'git
    hint: push --help' for details.

But a `git pull` will not fix this rejection. What is required is either

- Specify the expected remote tip with --force-with-lease=<ref>:<expect>
- Ignore the error with --no-force-if-includes

Add ref->unverifiable to differentiate between a detached HEAD rejection
vs. a remote update rejection.

Ensure tests check the rejection message.

Reported-by: D. Ben Knoble <ben.knoble@gmail.com>
Signed-off-by: Tyler Cipriani <tyler@tylercipriani.com>
---
 Documentation/config/advice.adoc |  4 ++++
 advice.c                         |  1 +
 advice.h                         |  1 +
 builtin/push.c                   | 15 +++++++++++++++
 builtin/send-pack.c              |  5 +++++
 remote.c                         |  5 ++++-
 remote.h                         | 10 +++++++---
 send-pack.c                      |  1 +
 t/t5533-push-cas.sh              |  7 +++++--
 transport-helper.c               |  5 +++++
 transport.c                      |  8 ++++++++
 transport.h                      |  1 +
 12 files changed, 57 insertions(+), 6 deletions(-)

diff --git a/Documentation/config/advice.adoc b/Documentation/config/advice.adoc
index 257db58918..a0eff8bbd6 100644
--- a/Documentation/config/advice.adoc
+++ b/Documentation/config/advice.adoc
@@ -90,6 +90,10 @@ all advice messages.
 		Shown when linkgit:git-push[1] rejects a forced update of
 		a branch when its remote-tracking ref has updates that we
 		do not have locally.
+	pushRefUnverifiable::
+		Shown when linkgit:git-push[1] rejects a forced update of
+		a branch when we are unable to verify the remote-tracking
+		ref is available locally.
 	pushUnqualifiedRefname::
 		Shown when linkgit:git-push[1] gives up trying to
 		guess based on the source and destination refs what
diff --git a/advice.c b/advice.c
index 0018501b7b..08842deb66 100644
--- a/advice.c
+++ b/advice.c
@@ -69,6 +69,7 @@ static struct {
 	[ADVICE_PUSH_NON_FF_CURRENT]			= { "pushNonFFCurrent" },
 	[ADVICE_PUSH_NON_FF_MATCHING]			= { "pushNonFFMatching" },
 	[ADVICE_PUSH_REF_NEEDS_UPDATE]			= { "pushRefNeedsUpdate" },
+	[ADVICE_PUSH_REF_UNVERIFIABLE]			= { "pushRefUnverifiable" },
 	[ADVICE_PUSH_UNQUALIFIED_REF_NAME]		= { "pushUnqualifiedRefName" },
 	[ADVICE_PUSH_UPDATE_REJECTED]			= { "pushUpdateRejected" },
 	[ADVICE_PUSH_UPDATE_REJECTED_ALIAS]		= { "pushNonFastForward" }, /* backwards compatibility */
diff --git a/advice.h b/advice.h
index 8def280688..189eadc089 100644
--- a/advice.h
+++ b/advice.h
@@ -36,6 +36,7 @@ enum advice_type {
 	ADVICE_PUSH_NON_FF_CURRENT,
 	ADVICE_PUSH_NON_FF_MATCHING,
 	ADVICE_PUSH_REF_NEEDS_UPDATE,
+	ADVICE_PUSH_REF_UNVERIFIABLE,
 	ADVICE_PUSH_UNQUALIFIED_REF_NAME,
 	ADVICE_PUSH_UPDATE_REJECTED,
 	ADVICE_PUSH_UPDATE_REJECTED_ALIAS,
diff --git a/builtin/push.c b/builtin/push.c
index 6021b71d66..9676c6241f 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -319,6 +319,12 @@ static const char message_advice_ref_needs_update[] =
 	   "remote changes, use 'git pull' before pushing again.\n"
 	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
 
+static const char message_advice_ref_unverifiable[] =
+	N_("Updates were rejected because the tip of the remote-tracking branch\n"
+	   "cannot be checked against a detached HEAD. If you want to push anyway,\n"
+	   "specify the expected value with '--force-with-lease=<ref>:<expect>'\n"
+	   "or use '--no-force-if-includes' to skip this check.");
+
 static void advise_pull_before_push(void)
 {
 	if (!advice_enabled(ADVICE_PUSH_NON_FF_CURRENT) || !advice_enabled(ADVICE_PUSH_UPDATE_REJECTED))
@@ -361,6 +367,13 @@ static void advise_ref_needs_update(void)
 	advise(_(message_advice_ref_needs_update));
 }
 
+static void advise_ref_unverifiable(void)
+{
+	if (!advice_enabled(ADVICE_PUSH_REF_UNVERIFIABLE) || !advice_enabled(ADVICE_PUSH_UPDATE_REJECTED))
+		return;
+	advise(_(message_advice_ref_unverifiable));
+}
+
 static int push_with_options(struct transport *transport, struct refspec *rs,
 			     int flags)
 {
@@ -412,6 +425,8 @@ static int push_with_options(struct transport *transport, struct refspec *rs,
 		advise_ref_needs_force();
 	} else if (reject_reasons & REJECT_REF_NEEDS_UPDATE) {
 		advise_ref_needs_update();
+	} else if (reject_reasons & REJECT_REF_UNVERIFIABLE) {
+		advise_ref_unverifiable();
 	}
 
 	return 1;
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 1412b49bc8..07accb6e6b 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -76,6 +76,11 @@ static void print_helper_status(struct ref *ref)
 			msg = "remote ref updated since checkout";
 			break;
 
+		case REF_STATUS_REJECT_UNVERIFIABLE:
+			res = "error";
+			msg = "remote ref unverifiable";
+			break;
+
 		case REF_STATUS_REJECT_ALREADY_EXISTS:
 			res = "error";
 			msg = "already exists";
diff --git a/remote.c b/remote.c
index 326af76eeb..72bfc4dbc4 100644
--- a/remote.c
+++ b/remote.c
@@ -1701,6 +1701,9 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
 			else if (ref->check_reachable && ref->unreachable)
 				reject_reason =
 					REF_STATUS_REJECT_REMOTE_UPDATED;
+			else if (ref->check_reachable && ref->unverifiable)
+				reject_reason =
+					REF_STATUS_REJECT_UNVERIFIABLE;
 			else
 				/*
 				 * If the ref isn't stale, and is reachable
@@ -2823,7 +2826,7 @@ static void check_if_includes_upstream(struct ref *remote)
 					       "HEAD", 0, NULL, &flag);
 		if (!name || !(flag & REF_ISSYMREF)) {
 			/* detached HEAD: no per-branch reflog to consult */
-			remote->unreachable = 1;
+			remote->unverifiable = 1;
 			return;
 		}
 	}
diff --git a/remote.h b/remote.h
index 54b17e4b02..8e2d56c2c2 100644
--- a/remote.h
+++ b/remote.h
@@ -169,10 +169,13 @@ struct ref {
 		/* Need to check if local reflog reaches the remote tip. */
 		check_reachable:1,
 		/*
-		 * Store the result of the check enabled by "check_reachable";
-		 * implies the local reflog does not reach the remote tip.
+		 * Store the result of the check enabled by "check_reachable".
+		 * "unreachable" implies the local reflog does not reach the remote
+		 * tip. "unverifiable" implies no local branch reflog to check; i.e.,
+		 * detached HEAD.
 		 */
-		unreachable:1;
+		unreachable:1,
+		unverifiable:1;
 
 	enum {
 		REF_NOT_MATCHED = 0, /* initial value */
@@ -203,6 +206,7 @@ struct ref {
 		REF_STATUS_REJECT_STALE,
 		REF_STATUS_REJECT_SHALLOW,
 		REF_STATUS_REJECT_REMOTE_UPDATED,
+		REF_STATUS_REJECT_UNVERIFIABLE,
 		REF_STATUS_UPTODATE,
 		REF_STATUS_REMOTE_REJECT,
 		REF_STATUS_EXPECTING_REPORT,
diff --git a/send-pack.c b/send-pack.c
index 3bb5afc687..6b78470f37 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -322,6 +322,7 @@ static int check_to_send_update(const struct ref *ref, const struct send_pack_ar
 	case REF_STATUS_REJECT_NEEDS_FORCE:
 	case REF_STATUS_REJECT_STALE:
 	case REF_STATUS_REJECT_REMOTE_UPDATED:
+	case REF_STATUS_REJECT_UNVERIFIABLE:
 	case REF_STATUS_REJECT_NODELETE:
 		return CHECK_REF_STATUS_REJECTED;
 	case REF_STATUS_UPTODATE:
diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh
index 0c02151747..fe6af3f41c 100755
--- a/t/t5533-push-cas.sh
+++ b/t/t5533-push-cas.sh
@@ -311,7 +311,8 @@ test_expect_success 'background updates to remote can be mitigated with "--force
 		git switch main &&
 		test_commit J &&
 		git fetch --all &&
-		test_must_fail git push --force-with-lease --force-if-includes --all
+		test_must_fail git push --force-with-lease --force-if-includes --all 2>err &&
+		test_grep "remote ref updated since checkout" err
 	) &&
 	git ls-remote dst refs/heads/main >actual.main &&
 	git ls-remote dst refs/heads/branch >actual.branch &&
@@ -457,7 +458,9 @@ test_expect_success '"--force-if-includes" should reject forced update from deta
 		git reset --hard origin/main &&
 		git switch -c newbranch origin/main &&
 		git checkout HEAD^ &&
-		test_must_fail git push --force-if-includes --force-with-lease origin HEAD:main
+		test_must_fail git push --force-if-includes --force-with-lease origin HEAD:main 2>err &&
+		test_grep "remote ref unverifiable" err &&
+		test_grep "no-force-if-includes" err
 	)
 '
 
diff --git a/transport-helper.c b/transport-helper.c
index 80f90eb7ba..1763570352 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -893,6 +893,10 @@ static int push_update_ref_status(struct strbuf *buf,
 			status = REF_STATUS_REJECT_REMOTE_UPDATED;
 			FREE_AND_NULL(msg);
 		}
+		else if (!strcmp(msg, "remote ref unverifiable")) {
+			status = REF_STATUS_REJECT_UNVERIFIABLE;
+			FREE_AND_NULL(msg);
+		}
 		else if (!strcmp(msg, "forced update")) {
 			forced = 1;
 			FREE_AND_NULL(msg);
@@ -1046,6 +1050,7 @@ static int push_refs_with_push(struct transport *transport,
 		case REF_STATUS_REJECT_STALE:
 		case REF_STATUS_REJECT_ALREADY_EXISTS:
 		case REF_STATUS_REJECT_REMOTE_UPDATED:
+		case REF_STATUS_REJECT_UNVERIFIABLE:
 			if (atomic) {
 				reject_atomic_push(remote_refs, mirror);
 				string_list_clear(&cas_options, 0);
diff --git a/transport.c b/transport.c
index 0f5ec30247..3d60d6de54 100644
--- a/transport.c
+++ b/transport.c
@@ -779,6 +779,11 @@ static int print_one_push_report(struct ref *ref, const char *dest, int count,
 				 "remote ref updated since checkout",
 				 report, porcelain, summary_width);
 		break;
+	case REF_STATUS_REJECT_UNVERIFIABLE:
+		print_ref_status('!', "[rejected]", ref, ref->peer_ref,
+				 "remote ref unverifiable",
+				 report, porcelain, summary_width);
+		break;
 	case REF_STATUS_REJECT_SHALLOW:
 		print_ref_status('!', "[rejected]", ref, ref->peer_ref,
 				 "new shallow roots not allowed",
@@ -893,6 +898,8 @@ void transport_print_push_status(const char *dest, struct ref *refs,
 			*reject_reasons |= REJECT_NEEDS_FORCE;
 		} else if (ref->status == REF_STATUS_REJECT_REMOTE_UPDATED) {
 			*reject_reasons |= REJECT_REF_NEEDS_UPDATE;
+		} else if (ref->status == REF_STATUS_REJECT_UNVERIFIABLE) {
+			*reject_reasons |= REJECT_REF_UNVERIFIABLE;
 		}
 	}
 	free(head);
@@ -1348,6 +1355,7 @@ static int pre_push_hook_feed_stdin(int hook_stdin_fd, void *pp_cb UNUSED, void
 	switch (r->status) {
 	case REF_STATUS_REJECT_NONFASTFORWARD:
 	case REF_STATUS_REJECT_REMOTE_UPDATED:
+	case REF_STATUS_REJECT_UNVERIFIABLE:
 	case REF_STATUS_REJECT_STALE:
 	case REF_STATUS_UPTODATE:
 		return 0; /* skip refs which won't be pushed */
diff --git a/transport.h b/transport.h
index 7e5867cffa..eaa3b616ee 100644
--- a/transport.h
+++ b/transport.h
@@ -256,6 +256,7 @@ void transport_set_verbosity(struct transport *transport, int verbosity,
 #define REJECT_FETCH_FIRST      0x08
 #define REJECT_NEEDS_FORCE      0x10
 #define REJECT_REF_NEEDS_UPDATE 0x20
+#define REJECT_REF_UNVERIFIABLE 0x40
 
 int transport_push(struct repository *repo,
 		   struct transport *connection,
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] push: check pushed ref for --force-if-includes
  2026-09-04 21:01 ` [PATCH 1/2] push: check pushed ref for --force-if-includes Tyler Cipriani
@ 2026-09-05 18:57   ` Ben Knoble
  0 siblings, 0 replies; 22+ messages in thread
From: Ben Knoble @ 2026-09-05 18:57 UTC (permalink / raw)
  To: Tyler Cipriani
  Cc: git, Srinidhi Kaushik, Stefan Haller, Phillip Wood,
	Johannes Schindelin, Tyler Cipriani


> Le 4 sept. 2026 à 17:01, Tyler Cipriani <tyler@tylercipriani.com> a écrit :
> 
> "--force-if-includes" ensures, "tip of the remote-tracking ref is
> reachable from one of the 'reflog' entries of the local branch."
> 
> But check_if_includes_upstream() uses the local per-branch reflog based
> on the destination branch rather than the branch being pushed; using
> ref->name vs. ref->peer_ref->name.
> 
> This can cause confusing rejections or unintended data loss.
> 
> Using a command like:
> 
>  git push --force-if-includes --force-with-lease origin src:main
> 
> False rejections: when src is an up-to-date branch, but main is
> out-of-date or nonexistent, then the includes check will fail telling
> users the remote ref has been updated since the last checkout.
> 
> Data loss: when src is an orphan/out-dated branch, but main is
> up-to-date, then the if-includes check will allow the push, clobbering
> the remote main.

Hm. This case *could* be by design, to rewind and potentially
modify a remote branch, discarding new work I’ve already checked.

But the includes check is about reminding to do such a check.
So failing and requiring me to bypass the check seems ok.

> Find local reflog using ref->peer_ref. When using a refspec like
> HEAD:refs/heads/main, we resolve HEAD to a branch and use that reflog.
> In a detached HEAD state, the reflog cannot tell us if the history
> being pushed includes the tip of the remote, so the push is rejected.

This seems to be what I reported in the mail your cover
letter cites. So, am I reading correctly that this is no change
from current behavior?

…ah, patch 2 addresses that specifically. Which, I now
remember you said in the cover as well. Oops.

It *could* be worth clarifying in the proposed log message that we are only preserving behavior here, but that’s a very small nit.

> Skip deletions:
> 
>  git push --force-if-includes --force-with-lease origin :main
> 
> ref->deletion is set after apply_push_cas (which triggers
> check_if_includes_upstream). The ref->peer_ref name is "(delete)".
> Instead check with is_null_oid to detect and allow deletion.
> 
> Reported-by: Stefan Haller <lists@haller-berlin.de>
> Reported-by: D. Ben Knoble <ben.knoble@gmail.com>
> Signed-off-by: Tyler Cipriani <tyler@tylercipriani.com>
> ---
> remote.c            | 24 ++++++++++++++++-
> t/t5533-push-cas.sh | 65 +++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 88 insertions(+), 1 deletion(-)
> 
> diff --git a/remote.c b/remote.c
> index 00723b385e..326af76eeb 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -2806,7 +2806,29 @@ static int is_reachable_in_reflog(const char *local, const struct ref *remote)
> */
> static void check_if_includes_upstream(struct ref *remote)
> {
> -    struct ref *local = get_local_ref(remote->name);
> +    struct ref *local;
> +    const char *name;
> +    int flag;
> +
> +    if (!remote->peer_ref)
> +        return;
> +
> +    /* A deletion has no local history to check against. */
> +    if (is_null_oid(&remote->peer_ref->new_oid))
> +        return;
> +
> +    name = remote->peer_ref->name;
> +    if (!strcmp(name, "HEAD")) {
> +        name = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
> +                           "HEAD", 0, NULL, &flag);
> +        if (!name || !(flag & REF_ISSYMREF)) {
> +            /* detached HEAD: no per-branch reflog to consult */
> +            remote->unreachable = 1;
> +            return;
> +        }
> +    }
> +
> +    local = get_local_ref(name);
>  if (!local)
>      return;
> 
> diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh
> index cba26a872d..0c02151747 100755
> --- a/t/t5533-push-cas.sh
> +++ b/t/t5533-push-cas.sh
> @@ -396,4 +396,69 @@ test_expect_success '"--force-if-includes" should allow deletes' '
>  )
> '
> 
> +test_expect_success '"--force-if-includes" should allow forced update when using differently named branches' '
> +    setup_src_dup_dst &&
> +    test_when_finished "rm -fr dst src dup" &&
> +    (
> +        cd src &&
> +        git fetch &&
> +        git switch -c newbranch origin/main &&
> +        git rebase HEAD --onto HEAD^ &&
> +        git push --force-if-includes --force-with-lease origin newbranch:main
> +    )
> +'
> +test_expect_success '"--force-if-includes" should allow forced update from HEAD' '
> +    setup_src_dup_dst &&
> +    test_when_finished "rm -fr dst src dup" &&
> +    (
> +        cd src &&
> +        git fetch &&
> +        git switch -c newbranch origin/main &&
> +        git rebase HEAD --onto HEAD^ &&
> +        git push --force-if-includes --force-with-lease origin HEAD:main
> +    )
> +'
> +
> +test_expect_success '"--force-if-includes" should reject forced update from differently named branches when local lacks remote ref' '
> +    setup_src_dup_dst &&
> +    test_when_finished "rm -fr dst src dup" &&
> +    (
> +        cd src &&
> +        git fetch &&
> +        git switch main &&
> +        git reset --hard origin/main &&
> +        git switch --orphan orphan &&
> +        test_commit I &&
> +        test_must_fail git push --force-with-lease --force-if-includes origin orphan:main
> +    )
> +'
> +
> +test_expect_success '"--force-if-includes" should reject forced update from HEAD when it lacks remote ref' '
> +    setup_src_dup_dst &&
> +    test_when_finished "rm -fr dst src dup" &&
> +    (
> +        cd src &&
> +        git fetch &&
> +        git switch main &&
> +        git reset --hard origin/main &&
> +        git switch --orphan orphan &&
> +        test_commit I &&
> +        test_must_fail git push --force-with-lease --force-if-includes origin HEAD:main
> +    )
> +'
> +
> +test_expect_success '"--force-if-includes" should reject forced update from detached HEAD' '
> +    setup_src_dup_dst &&
> +    test_when_finished "rm -fr dst src dup" &&
> +    (
> +        cd src &&
> +        git fetch &&
> +        git switch main &&
> +        git reset --hard origin/main &&
> +        git switch -c newbranch origin/main &&
> +        git checkout HEAD^ &&
> +        test_must_fail git push --force-if-includes --force-with-lease origin HEAD:main
> +    )
> +'
> +
> test_done
> --
> 2.47.3

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/2] push: fix --force-if-includes consulting wrong ref
  2026-09-04 21:01 [PATCH 0/2] push: fix --force-if-includes consulting wrong ref Tyler Cipriani
  2026-09-04 21:01 ` [PATCH 1/2] push: check pushed ref for --force-if-includes Tyler Cipriani
  2026-09-04 21:01 ` [PATCH 2/2] push: fix --force-if-includes detached HEAD advice Tyler Cipriani
@ 2026-09-05 18:59 ` Ben Knoble
  2026-09-06 20:24   ` Tyler Cipriani
  2026-09-08 22:20 ` [PATCH v2 " Tyler Cipriani
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 22+ messages in thread
From: Ben Knoble @ 2026-09-05 18:59 UTC (permalink / raw)
  To: Tyler Cipriani
  Cc: git, Srinidhi Kaushik, Stefan Haller, Phillip Wood,
	Johannes Schindelin, Tyler Cipriani


> Le 4 sept. 2026 à 17:01, Tyler Cipriani <tyler@tylercipriani.com> a écrit :
> 
> --force-if-includes has been checking the reflog of the local branch named
> after the destination branch regardless of what's being pushed. This can cause
> false rejections or unintended data loss.
> 
> False rejection has been reported twice that I could find:
> 
> - 2023-07-26 - Stefan Haller reported local branch with a different name
>              false rejection[0]
> - 2025-05-08 - D. Ben Knoble reported detached HEAD false rejection[1]

Aha. I’d nearly forgotten that mail, and have since adjusted to
some intuition of when to force-if-includes.

I’d be grateful to not need such potentially-buggy intuition :)

> The same root cause can result in data loss: when a same-name local branch
> contains the remote tip but you --force-if-includes push an unrelated branch,
> clobbering the remote repo. PoCs are in t/t5533-push-cas.sh -- new test cases
> fail against maint, but pass with patches applied.
> 
> Existing tests covered refspecs with different names for --force-with-lease,
> but missed --force-if-includes. New patches cover:
> 
> - allow forced-update using refspec with different-named local branch
> - allow same as above, but with HEAD
> - reject force-update using refspec with different-named local branch lacking
> branch tip
> - reject same as above using HEAD
> - reject detached HEAD
> 
> Open question: the detached HEAD case. I opted to reject, since it seems like
> it might be surprising to allow in the case where you were just on a branch
> without the the tip of a remote ref, removed the last commit with git checkout
> HEAD^ and pushed with --force-if-includes and it allowed a destructive push.
> I made a separate patch showing different advice for that case (since a
> git pull won't help).
> 
> Based on maint since this is a bugfix. Happy to split patches any way
> that's helpful.
> 
> [0]: <https://lore.kernel.org/git/f51c73ed-eb03-83ca-fb31-d3e2645c9a63@haller-berlin.de>
> [1]: <https://lore.kernel.org/git/CALnO6CCk0SgwObQRnpd5Pt_DvCKF8dBmyVHivU6Nr_O-GusGLA@mail.gmail.com>
> 
> Tyler Cipriani (2):
> push: check pushed ref for --force-if-includes
> push: fix --force-if-includes detached HEAD advice

Thanks for the advice changes! One small nit on the first
patch you can ignore if you choose.

At first I hoped we might be able to stop rejecting detached
HEAD pushes, but some further thought begs the question:
what reflog would we use?
HEAD’s is too broad :)

So this may be all we can do for now.

At least I can replace my intuition with reading the error message again. 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/2] push: fix --force-if-includes consulting wrong ref
  2026-09-05 18:59 ` [PATCH 0/2] push: fix --force-if-includes consulting wrong ref Ben Knoble
@ 2026-09-06 20:24   ` Tyler Cipriani
  0 siblings, 0 replies; 22+ messages in thread
From: Tyler Cipriani @ 2026-09-06 20:24 UTC (permalink / raw)
  To: Ben Knoble
  Cc: git, Srinidhi Kaushik, Stefan Haller, Phillip Wood,
	Johannes Schindelin

On Sat, Sep 5, 2026 at 12:59 PM Ben Knoble <ben.knoble@gmail.com> wrote:
> Thanks for the advice changes! One small nit on the first
> patch you can ignore if you choose.

Good call on updating the log message for PATCH 1/2. I'll note that
detached HEAD
is already rejected in v2.

> At first I hoped we might be able to stop rejecting detached
> HEAD pushes, but some further thought begs the question:
> what reflog would we use?
> HEAD’s is too broad :)
>
> So this may be all we can do for now.

It looks like that's the conclusion they reached on the original patchset, too,
based on my re-reading of the thread[0]. HEAD's reflog is too broad for the
--force-if-includes check (with the acknowledged downside being that
--force-if-includes isn't useful for the detached HEAD case.)

[0]: <https://lore.kernel.org/git/xmqqsgbdk69b.fsf@gitster.c.googlers.com/>

> At least I can replace my intuition with reading the error message again.

:)

Thank you for the review!

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v2 0/2] push: fix --force-if-includes consulting wrong ref
  2026-09-04 21:01 [PATCH 0/2] push: fix --force-if-includes consulting wrong ref Tyler Cipriani
                   ` (2 preceding siblings ...)
  2026-09-05 18:59 ` [PATCH 0/2] push: fix --force-if-includes consulting wrong ref Ben Knoble
@ 2026-09-08 22:20 ` Tyler Cipriani
  2026-09-09 11:59   ` D. Ben Knoble
  2026-09-08 22:20 ` [PATCH v2 1/2] push: check pushed ref for --force-if-includes Tyler Cipriani
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 22+ messages in thread
From: Tyler Cipriani @ 2026-09-08 22:20 UTC (permalink / raw)
  To: git
  Cc: Srinidhi Kaushik, Stefan Haller, D . Ben Knoble, Phillip Wood,
	Johannes Schindelin, Tyler Cipriani

Changes since v1:

- Clarify in log message 1/2 that --force-if-includes will reject a
  detached HEAD today (when the same-named local branch lacks the remote
  tip). And note that this change makes it explicit to always reject
  the detached HEAD case.

--force-if-includes has been checking the reflog of the local branch named
after the destination branch regardless of what's being pushed. This can cause
false rejections or unintended data loss.

False rejection has been reported twice that I could find:

- 2023-07-26 - Stefan Haller reported local branch with a different name
               false rejection[0]
- 2025-05-08 - D. Ben Knoble reported detached HEAD false rejection[1]

The same root cause can result in data loss: when a same-name local branch
contains the remote tip but you --force-if-includes push an unrelated branch,
clobbering the remote repo. PoCs are in t/t5533-push-cas.sh -- new test cases
fail against maint, but pass with patches applied.

Existing tests covered refspecs with different names for --force-with-lease,
but missed --force-if-includes. New patches cover:

- allow forced-update using refspec with different-named local branch
- allow same as above, but with HEAD
- reject force-update using refspec with different-named local branch lacking
  branch tip
- reject same as above using HEAD
- reject detached HEAD

Resolved question: the detached HEAD case; HEAD's reflog was considered
and rejected as too broad for purpose in the original review. cf. [2]

[0]: <https://lore.kernel.org/git/f51c73ed-eb03-83ca-fb31-d3e2645c9a63@haller-berlin.de>
[1]: <https://lore.kernel.org/git/CALnO6CCk0SgwObQRnpd5Pt_DvCKF8dBmyVHivU6Nr_O-GusGLA@mail.gmail.com>
[2]: <https://lore.kernel.org/git/CAHLx=O=tVhtiZpaRP9TpfiBfOMS2xPe3c3=mC3VNEdBrLOioFg@mail.gmail.com>

Tyler Cipriani (2):
  push: check pushed ref for --force-if-includes
  push: fix --force-if-includes detached HEAD advice

 Documentation/config/advice.adoc |  4 ++
 advice.c                         |  1 +
 advice.h                         |  1 +
 builtin/push.c                   | 15 +++++++
 builtin/send-pack.c              |  5 +++
 remote.c                         | 27 +++++++++++-
 remote.h                         | 10 +++--
 send-pack.c                      |  1 +
 t/t5533-push-cas.sh              | 70 +++++++++++++++++++++++++++++++-
 transport-helper.c               |  5 +++
 transport.c                      |  8 ++++
 transport.h                      |  1 +
 12 files changed, 143 insertions(+), 5 deletions(-)

Range-diff against v1:
1:  5e866b883e ! 1:  da27c421ed push: check pushed ref for --force-if-includes
    @@ Commit message
         the remote main.
     
         Find local reflog using ref->peer_ref. When using a refspec like
    -    HEAD:refs/heads/main, we resolve HEAD to a branch and use that reflog.
    -    In a detached HEAD state, the reflog cannot tell us if the history
    -    being pushed includes the tip of the remote, so the push is rejected.
    +    HEAD:refs/heads/main, we resolve HEAD. If HEAD is a branch, use that
    +    branch's reflog.
    +
    +    But if HEAD does not resolve to a branch (i.e. a detached HEAD), then we
    +    reject the push. HEAD's reflog is too broad to tell us if the history
    +    being pushed includes the tip of the remote. Rejecting a detached HEAD
    +    already happens today (if the same-named local branch lacks the remote
    +    tip); now the detached HEAD state is explicitly rejected.
     
         Skip deletions:
     
2:  4ae40db7fe = 2:  e07d16d53e push: fix --force-if-includes detached HEAD advice

base-commit: e9019fcafe0040228b8631c30f97ae1adb61bcdc
-- 
2.47.3


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v2 1/2] push: check pushed ref for --force-if-includes
  2026-09-04 21:01 [PATCH 0/2] push: fix --force-if-includes consulting wrong ref Tyler Cipriani
                   ` (3 preceding siblings ...)
  2026-09-08 22:20 ` [PATCH v2 " Tyler Cipriani
@ 2026-09-08 22:20 ` Tyler Cipriani
  2026-09-10 18:43   ` Junio C Hamano
  2026-09-08 22:20 ` [PATCH v2 2/2] push: fix --force-if-includes detached HEAD advice Tyler Cipriani
  2026-09-10 23:05 ` [PATCH v3 0/2] push: fix --force-if-includes consulting wrong ref Tyler Cipriani
  6 siblings, 1 reply; 22+ messages in thread
From: Tyler Cipriani @ 2026-09-08 22:20 UTC (permalink / raw)
  To: git
  Cc: Srinidhi Kaushik, Stefan Haller, D . Ben Knoble, Phillip Wood,
	Johannes Schindelin, Tyler Cipriani

"--force-if-includes" ensures, "tip of the remote-tracking ref is
reachable from one of the 'reflog' entries of the local branch."

But check_if_includes_upstream() uses the local per-branch reflog based
on the destination branch rather than the branch being pushed; using
ref->name vs. ref->peer_ref->name.

This can cause confusing rejections or unintended data loss.

Using a command like:

    git push --force-if-includes --force-with-lease origin src:main

False rejections: when src is an up-to-date branch, but main is
out-of-date or nonexistent, then the includes check will fail telling
users the remote ref has been updated since the last checkout.

Data loss: when src is an orphan/out-dated branch, but main is
up-to-date, then the if-includes check will allow the push, clobbering
the remote main.

Find local reflog using ref->peer_ref. When using a refspec like
HEAD:refs/heads/main, we resolve HEAD. If HEAD is a branch, use that
branch's reflog.

But if HEAD does not resolve to a branch (i.e. a detached HEAD), then we
reject the push. HEAD's reflog is too broad to tell us if the history
being pushed includes the tip of the remote. Rejecting a detached HEAD
already happens today (if the same-named local branch lacks the remote
tip); now the detached HEAD state is explicitly rejected.

Skip deletions:

    git push --force-if-includes --force-with-lease origin :main

ref->deletion is set after apply_push_cas (which triggers
check_if_includes_upstream). The ref->peer_ref name is "(delete)".
Instead check with is_null_oid to detect and allow deletion.

Reported-by: Stefan Haller <lists@haller-berlin.de>
Reported-by: D. Ben Knoble <ben.knoble@gmail.com>
Signed-off-by: Tyler Cipriani <tyler@tylercipriani.com>
---
 remote.c            | 24 ++++++++++++++++-
 t/t5533-push-cas.sh | 65 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 88 insertions(+), 1 deletion(-)

diff --git a/remote.c b/remote.c
index 00723b385e..326af76eeb 100644
--- a/remote.c
+++ b/remote.c
@@ -2806,7 +2806,29 @@ static int is_reachable_in_reflog(const char *local, const struct ref *remote)
  */
 static void check_if_includes_upstream(struct ref *remote)
 {
-	struct ref *local = get_local_ref(remote->name);
+	struct ref *local;
+	const char *name;
+	int flag;
+
+	if (!remote->peer_ref)
+		return;
+
+	/* A deletion has no local history to check against. */
+	if (is_null_oid(&remote->peer_ref->new_oid))
+		return;
+
+	name = remote->peer_ref->name;
+	if (!strcmp(name, "HEAD")) {
+		name = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
+					       "HEAD", 0, NULL, &flag);
+		if (!name || !(flag & REF_ISSYMREF)) {
+			/* detached HEAD: no per-branch reflog to consult */
+			remote->unreachable = 1;
+			return;
+		}
+	}
+
+	local = get_local_ref(name);
 	if (!local)
 		return;
 
diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh
index cba26a872d..0c02151747 100755
--- a/t/t5533-push-cas.sh
+++ b/t/t5533-push-cas.sh
@@ -396,4 +396,69 @@ test_expect_success '"--force-if-includes" should allow deletes' '
 	)
 '
 
+test_expect_success '"--force-if-includes" should allow forced update when using differently named branches' '
+	setup_src_dup_dst &&
+	test_when_finished "rm -fr dst src dup" &&
+	(
+		cd src &&
+		git fetch &&
+		git switch -c newbranch origin/main &&
+		git rebase HEAD --onto HEAD^ &&
+		git push --force-if-includes --force-with-lease origin newbranch:main
+	)
+'
+test_expect_success '"--force-if-includes" should allow forced update from HEAD' '
+	setup_src_dup_dst &&
+	test_when_finished "rm -fr dst src dup" &&
+	(
+		cd src &&
+		git fetch &&
+		git switch -c newbranch origin/main &&
+		git rebase HEAD --onto HEAD^ &&
+		git push --force-if-includes --force-with-lease origin HEAD:main
+	)
+'
+
+test_expect_success '"--force-if-includes" should reject forced update from differently named branches when local lacks remote ref' '
+	setup_src_dup_dst &&
+	test_when_finished "rm -fr dst src dup" &&
+	(
+		cd src &&
+		git fetch &&
+		git switch main &&
+		git reset --hard origin/main &&
+		git switch --orphan orphan &&
+		test_commit I &&
+		test_must_fail git push --force-with-lease --force-if-includes origin orphan:main
+	)
+'
+
+test_expect_success '"--force-if-includes" should reject forced update from HEAD when it lacks remote ref' '
+	setup_src_dup_dst &&
+	test_when_finished "rm -fr dst src dup" &&
+	(
+		cd src &&
+		git fetch &&
+		git switch main &&
+		git reset --hard origin/main &&
+		git switch --orphan orphan &&
+		test_commit I &&
+		test_must_fail git push --force-with-lease --force-if-includes origin HEAD:main
+	)
+'
+
+test_expect_success '"--force-if-includes" should reject forced update from detached HEAD' '
+	setup_src_dup_dst &&
+	test_when_finished "rm -fr dst src dup" &&
+	(
+		cd src &&
+		git fetch &&
+		git switch main &&
+		git reset --hard origin/main &&
+		git switch -c newbranch origin/main &&
+		git checkout HEAD^ &&
+		test_must_fail git push --force-if-includes --force-with-lease origin HEAD:main
+	)
+'
+
 test_done
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 2/2] push: fix --force-if-includes detached HEAD advice
  2026-09-04 21:01 [PATCH 0/2] push: fix --force-if-includes consulting wrong ref Tyler Cipriani
                   ` (4 preceding siblings ...)
  2026-09-08 22:20 ` [PATCH v2 1/2] push: check pushed ref for --force-if-includes Tyler Cipriani
@ 2026-09-08 22:20 ` Tyler Cipriani
  2026-09-10 23:05 ` [PATCH v3 0/2] push: fix --force-if-includes consulting wrong ref Tyler Cipriani
  6 siblings, 0 replies; 22+ messages in thread
From: Tyler Cipriani @ 2026-09-08 22:20 UTC (permalink / raw)
  To: git
  Cc: Srinidhi Kaushik, Stefan Haller, D . Ben Knoble, Phillip Wood,
	Johannes Schindelin, Tyler Cipriani

When a --force-if-includes push is rejected due to a detached HEAD
state where there is no per-branch reflog to consult, the advice is
misleading:

     ! [rejected] HEAD -> main (remote ref updated since checkout)
    error: failed to push some refs to '<remote>'
    hint: Updates were rejected because the tip of the remote-tracking
    hint: branch has been updated since the last checkout. If you want
    hint: to integrate the remote changes, use 'git pull' before
    hint: pushing again. See the 'Note about fast-forwards' in 'git
    hint: push --help' for details.

But a `git pull` will not fix this rejection. What is required is either

- Specify the expected remote tip with --force-with-lease=<ref>:<expect>
- Ignore the error with --no-force-if-includes

Add ref->unverifiable to differentiate between a detached HEAD rejection
vs. a remote update rejection.

Ensure tests check the rejection message.

Reported-by: D. Ben Knoble <ben.knoble@gmail.com>
Signed-off-by: Tyler Cipriani <tyler@tylercipriani.com>
---
 Documentation/config/advice.adoc |  4 ++++
 advice.c                         |  1 +
 advice.h                         |  1 +
 builtin/push.c                   | 15 +++++++++++++++
 builtin/send-pack.c              |  5 +++++
 remote.c                         |  5 ++++-
 remote.h                         | 10 +++++++---
 send-pack.c                      |  1 +
 t/t5533-push-cas.sh              |  7 +++++--
 transport-helper.c               |  5 +++++
 transport.c                      |  8 ++++++++
 transport.h                      |  1 +
 12 files changed, 57 insertions(+), 6 deletions(-)

diff --git a/Documentation/config/advice.adoc b/Documentation/config/advice.adoc
index 257db58918..a0eff8bbd6 100644
--- a/Documentation/config/advice.adoc
+++ b/Documentation/config/advice.adoc
@@ -90,6 +90,10 @@ all advice messages.
 		Shown when linkgit:git-push[1] rejects a forced update of
 		a branch when its remote-tracking ref has updates that we
 		do not have locally.
+	pushRefUnverifiable::
+		Shown when linkgit:git-push[1] rejects a forced update of
+		a branch when we are unable to verify the remote-tracking
+		ref is available locally.
 	pushUnqualifiedRefname::
 		Shown when linkgit:git-push[1] gives up trying to
 		guess based on the source and destination refs what
diff --git a/advice.c b/advice.c
index 0018501b7b..08842deb66 100644
--- a/advice.c
+++ b/advice.c
@@ -69,6 +69,7 @@ static struct {
 	[ADVICE_PUSH_NON_FF_CURRENT]			= { "pushNonFFCurrent" },
 	[ADVICE_PUSH_NON_FF_MATCHING]			= { "pushNonFFMatching" },
 	[ADVICE_PUSH_REF_NEEDS_UPDATE]			= { "pushRefNeedsUpdate" },
+	[ADVICE_PUSH_REF_UNVERIFIABLE]			= { "pushRefUnverifiable" },
 	[ADVICE_PUSH_UNQUALIFIED_REF_NAME]		= { "pushUnqualifiedRefName" },
 	[ADVICE_PUSH_UPDATE_REJECTED]			= { "pushUpdateRejected" },
 	[ADVICE_PUSH_UPDATE_REJECTED_ALIAS]		= { "pushNonFastForward" }, /* backwards compatibility */
diff --git a/advice.h b/advice.h
index 8def280688..189eadc089 100644
--- a/advice.h
+++ b/advice.h
@@ -36,6 +36,7 @@ enum advice_type {
 	ADVICE_PUSH_NON_FF_CURRENT,
 	ADVICE_PUSH_NON_FF_MATCHING,
 	ADVICE_PUSH_REF_NEEDS_UPDATE,
+	ADVICE_PUSH_REF_UNVERIFIABLE,
 	ADVICE_PUSH_UNQUALIFIED_REF_NAME,
 	ADVICE_PUSH_UPDATE_REJECTED,
 	ADVICE_PUSH_UPDATE_REJECTED_ALIAS,
diff --git a/builtin/push.c b/builtin/push.c
index 6021b71d66..9676c6241f 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -319,6 +319,12 @@ static const char message_advice_ref_needs_update[] =
 	   "remote changes, use 'git pull' before pushing again.\n"
 	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
 
+static const char message_advice_ref_unverifiable[] =
+	N_("Updates were rejected because the tip of the remote-tracking branch\n"
+	   "cannot be checked against a detached HEAD. If you want to push anyway,\n"
+	   "specify the expected value with '--force-with-lease=<ref>:<expect>'\n"
+	   "or use '--no-force-if-includes' to skip this check.");
+
 static void advise_pull_before_push(void)
 {
 	if (!advice_enabled(ADVICE_PUSH_NON_FF_CURRENT) || !advice_enabled(ADVICE_PUSH_UPDATE_REJECTED))
@@ -361,6 +367,13 @@ static void advise_ref_needs_update(void)
 	advise(_(message_advice_ref_needs_update));
 }
 
+static void advise_ref_unverifiable(void)
+{
+	if (!advice_enabled(ADVICE_PUSH_REF_UNVERIFIABLE) || !advice_enabled(ADVICE_PUSH_UPDATE_REJECTED))
+		return;
+	advise(_(message_advice_ref_unverifiable));
+}
+
 static int push_with_options(struct transport *transport, struct refspec *rs,
 			     int flags)
 {
@@ -412,6 +425,8 @@ static int push_with_options(struct transport *transport, struct refspec *rs,
 		advise_ref_needs_force();
 	} else if (reject_reasons & REJECT_REF_NEEDS_UPDATE) {
 		advise_ref_needs_update();
+	} else if (reject_reasons & REJECT_REF_UNVERIFIABLE) {
+		advise_ref_unverifiable();
 	}
 
 	return 1;
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 1412b49bc8..07accb6e6b 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -76,6 +76,11 @@ static void print_helper_status(struct ref *ref)
 			msg = "remote ref updated since checkout";
 			break;
 
+		case REF_STATUS_REJECT_UNVERIFIABLE:
+			res = "error";
+			msg = "remote ref unverifiable";
+			break;
+
 		case REF_STATUS_REJECT_ALREADY_EXISTS:
 			res = "error";
 			msg = "already exists";
diff --git a/remote.c b/remote.c
index 326af76eeb..72bfc4dbc4 100644
--- a/remote.c
+++ b/remote.c
@@ -1701,6 +1701,9 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
 			else if (ref->check_reachable && ref->unreachable)
 				reject_reason =
 					REF_STATUS_REJECT_REMOTE_UPDATED;
+			else if (ref->check_reachable && ref->unverifiable)
+				reject_reason =
+					REF_STATUS_REJECT_UNVERIFIABLE;
 			else
 				/*
 				 * If the ref isn't stale, and is reachable
@@ -2823,7 +2826,7 @@ static void check_if_includes_upstream(struct ref *remote)
 					       "HEAD", 0, NULL, &flag);
 		if (!name || !(flag & REF_ISSYMREF)) {
 			/* detached HEAD: no per-branch reflog to consult */
-			remote->unreachable = 1;
+			remote->unverifiable = 1;
 			return;
 		}
 	}
diff --git a/remote.h b/remote.h
index 54b17e4b02..8e2d56c2c2 100644
--- a/remote.h
+++ b/remote.h
@@ -169,10 +169,13 @@ struct ref {
 		/* Need to check if local reflog reaches the remote tip. */
 		check_reachable:1,
 		/*
-		 * Store the result of the check enabled by "check_reachable";
-		 * implies the local reflog does not reach the remote tip.
+		 * Store the result of the check enabled by "check_reachable".
+		 * "unreachable" implies the local reflog does not reach the remote
+		 * tip. "unverifiable" implies no local branch reflog to check; i.e.,
+		 * detached HEAD.
 		 */
-		unreachable:1;
+		unreachable:1,
+		unverifiable:1;
 
 	enum {
 		REF_NOT_MATCHED = 0, /* initial value */
@@ -203,6 +206,7 @@ struct ref {
 		REF_STATUS_REJECT_STALE,
 		REF_STATUS_REJECT_SHALLOW,
 		REF_STATUS_REJECT_REMOTE_UPDATED,
+		REF_STATUS_REJECT_UNVERIFIABLE,
 		REF_STATUS_UPTODATE,
 		REF_STATUS_REMOTE_REJECT,
 		REF_STATUS_EXPECTING_REPORT,
diff --git a/send-pack.c b/send-pack.c
index 3bb5afc687..6b78470f37 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -322,6 +322,7 @@ static int check_to_send_update(const struct ref *ref, const struct send_pack_ar
 	case REF_STATUS_REJECT_NEEDS_FORCE:
 	case REF_STATUS_REJECT_STALE:
 	case REF_STATUS_REJECT_REMOTE_UPDATED:
+	case REF_STATUS_REJECT_UNVERIFIABLE:
 	case REF_STATUS_REJECT_NODELETE:
 		return CHECK_REF_STATUS_REJECTED;
 	case REF_STATUS_UPTODATE:
diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh
index 0c02151747..fe6af3f41c 100755
--- a/t/t5533-push-cas.sh
+++ b/t/t5533-push-cas.sh
@@ -311,7 +311,8 @@ test_expect_success 'background updates to remote can be mitigated with "--force
 		git switch main &&
 		test_commit J &&
 		git fetch --all &&
-		test_must_fail git push --force-with-lease --force-if-includes --all
+		test_must_fail git push --force-with-lease --force-if-includes --all 2>err &&
+		test_grep "remote ref updated since checkout" err
 	) &&
 	git ls-remote dst refs/heads/main >actual.main &&
 	git ls-remote dst refs/heads/branch >actual.branch &&
@@ -457,7 +458,9 @@ test_expect_success '"--force-if-includes" should reject forced update from deta
 		git reset --hard origin/main &&
 		git switch -c newbranch origin/main &&
 		git checkout HEAD^ &&
-		test_must_fail git push --force-if-includes --force-with-lease origin HEAD:main
+		test_must_fail git push --force-if-includes --force-with-lease origin HEAD:main 2>err &&
+		test_grep "remote ref unverifiable" err &&
+		test_grep "no-force-if-includes" err
 	)
 '
 
diff --git a/transport-helper.c b/transport-helper.c
index 80f90eb7ba..1763570352 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -893,6 +893,10 @@ static int push_update_ref_status(struct strbuf *buf,
 			status = REF_STATUS_REJECT_REMOTE_UPDATED;
 			FREE_AND_NULL(msg);
 		}
+		else if (!strcmp(msg, "remote ref unverifiable")) {
+			status = REF_STATUS_REJECT_UNVERIFIABLE;
+			FREE_AND_NULL(msg);
+		}
 		else if (!strcmp(msg, "forced update")) {
 			forced = 1;
 			FREE_AND_NULL(msg);
@@ -1046,6 +1050,7 @@ static int push_refs_with_push(struct transport *transport,
 		case REF_STATUS_REJECT_STALE:
 		case REF_STATUS_REJECT_ALREADY_EXISTS:
 		case REF_STATUS_REJECT_REMOTE_UPDATED:
+		case REF_STATUS_REJECT_UNVERIFIABLE:
 			if (atomic) {
 				reject_atomic_push(remote_refs, mirror);
 				string_list_clear(&cas_options, 0);
diff --git a/transport.c b/transport.c
index 0f5ec30247..3d60d6de54 100644
--- a/transport.c
+++ b/transport.c
@@ -779,6 +779,11 @@ static int print_one_push_report(struct ref *ref, const char *dest, int count,
 				 "remote ref updated since checkout",
 				 report, porcelain, summary_width);
 		break;
+	case REF_STATUS_REJECT_UNVERIFIABLE:
+		print_ref_status('!', "[rejected]", ref, ref->peer_ref,
+				 "remote ref unverifiable",
+				 report, porcelain, summary_width);
+		break;
 	case REF_STATUS_REJECT_SHALLOW:
 		print_ref_status('!', "[rejected]", ref, ref->peer_ref,
 				 "new shallow roots not allowed",
@@ -893,6 +898,8 @@ void transport_print_push_status(const char *dest, struct ref *refs,
 			*reject_reasons |= REJECT_NEEDS_FORCE;
 		} else if (ref->status == REF_STATUS_REJECT_REMOTE_UPDATED) {
 			*reject_reasons |= REJECT_REF_NEEDS_UPDATE;
+		} else if (ref->status == REF_STATUS_REJECT_UNVERIFIABLE) {
+			*reject_reasons |= REJECT_REF_UNVERIFIABLE;
 		}
 	}
 	free(head);
@@ -1348,6 +1355,7 @@ static int pre_push_hook_feed_stdin(int hook_stdin_fd, void *pp_cb UNUSED, void
 	switch (r->status) {
 	case REF_STATUS_REJECT_NONFASTFORWARD:
 	case REF_STATUS_REJECT_REMOTE_UPDATED:
+	case REF_STATUS_REJECT_UNVERIFIABLE:
 	case REF_STATUS_REJECT_STALE:
 	case REF_STATUS_UPTODATE:
 		return 0; /* skip refs which won't be pushed */
diff --git a/transport.h b/transport.h
index 7e5867cffa..eaa3b616ee 100644
--- a/transport.h
+++ b/transport.h
@@ -256,6 +256,7 @@ void transport_set_verbosity(struct transport *transport, int verbosity,
 #define REJECT_FETCH_FIRST      0x08
 #define REJECT_NEEDS_FORCE      0x10
 #define REJECT_REF_NEEDS_UPDATE 0x20
+#define REJECT_REF_UNVERIFIABLE 0x40
 
 int transport_push(struct repository *repo,
 		   struct transport *connection,
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 0/2] push: fix --force-if-includes consulting wrong ref
  2026-09-08 22:20 ` [PATCH v2 " Tyler Cipriani
@ 2026-09-09 11:59   ` D. Ben Knoble
  0 siblings, 0 replies; 22+ messages in thread
From: D. Ben Knoble @ 2026-09-09 11:59 UTC (permalink / raw)
  To: Tyler Cipriani
  Cc: git, Srinidhi Kaushik, Stefan Haller, Phillip Wood,
	Johannes Schindelin

On Tue, Sep 8, 2026 at 6:21 PM Tyler Cipriani <tyler@tylercipriani.com> wrote:
>
> Changes since v1:
>
> - Clarify in log message 1/2 that --force-if-includes will reject a
>   detached HEAD today (when the same-named local branch lacks the remote
>   tip). And note that this change makes it explicit to always reject
>   the detached HEAD case.

Thanks!

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 1/2] push: check pushed ref for --force-if-includes
  2026-09-08 22:20 ` [PATCH v2 1/2] push: check pushed ref for --force-if-includes Tyler Cipriani
@ 2026-09-10 18:43   ` Junio C Hamano
  2026-09-10 22:08     ` Tyler Cipriani
  0 siblings, 1 reply; 22+ messages in thread
From: Junio C Hamano @ 2026-09-10 18:43 UTC (permalink / raw)
  To: Tyler Cipriani
  Cc: git, Srinidhi Kaushik, Stefan Haller, D . Ben Knoble,
	Phillip Wood, Johannes Schindelin

Tyler Cipriani <tyler@tylercipriani.com> writes:

> Message-ID: <20260908222056.1150748-2-tyler@tylercipriani.com>
> References: <20260904210122.431757-1-tyler@tylercipriani.com>

This is incorrectly threaded.  It is not made as a reply to the
cover letter of v2; it is a reply to the cover letter of the initial
iteration, and breaks automation.

The same problem exists for [v2 2/2] as well.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 1/2] push: check pushed ref for --force-if-includes
  2026-09-10 18:43   ` Junio C Hamano
@ 2026-09-10 22:08     ` Tyler Cipriani
  0 siblings, 0 replies; 22+ messages in thread
From: Tyler Cipriani @ 2026-09-10 22:08 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Srinidhi Kaushik, Stefan Haller, D . Ben Knoble,
	Phillip Wood, Johannes Schindelin

On Thu, Sep 10, 2026 at 12:43 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Tyler Cipriani <tyler@tylercipriani.com> writes:
>
> > Message-ID: <20260908222056.1150748-2-tyler@tylercipriani.com>
> > References: <20260904210122.431757-1-tyler@tylercipriani.com>
>
> This is incorrectly threaded.  It is not made as a reply to the
> cover letter of v2; it is a reply to the cover letter of the initial
> iteration, and breaks automation.
>
> The same problem exists for [v2 2/2] as well.

Sorry for that. I had --in-reply-to on format-patch vs. send-email.
I'll send a v3 with correct shallow threading.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v3 0/2] push: fix --force-if-includes consulting wrong ref
  2026-09-04 21:01 [PATCH 0/2] push: fix --force-if-includes consulting wrong ref Tyler Cipriani
                   ` (5 preceding siblings ...)
  2026-09-08 22:20 ` [PATCH v2 2/2] push: fix --force-if-includes detached HEAD advice Tyler Cipriani
@ 2026-09-10 23:05 ` Tyler Cipriani
  2026-09-10 23:05   ` [PATCH v3 1/2] push: check pushed ref for --force-if-includes Tyler Cipriani
  2026-09-10 23:05   ` [PATCH v3 2/2] push: fix --force-if-includes detached HEAD advice Tyler Cipriani
  6 siblings, 2 replies; 22+ messages in thread
From: Tyler Cipriani @ 2026-09-10 23:05 UTC (permalink / raw)
  To: git
  Cc: Srinidhi Kaushik, Stefan Haller, D . Ben Knoble, Phillip Wood,
	Johannes Schindelin, Tyler Cipriani

Changes since v2:

- Correct patch threading of 1/2 and 2/2 to reply to cover letter of
  current patchset vs. cover letter of the initial iteration.

Changes since v1:

- Clarify in log message 1/2 that --force-if-includes will reject a
  detached HEAD today (when the same-named local branch lacks the remote
  tip). And note that this change makes it explicit to always reject
  the detached HEAD case.

--force-if-includes has been checking the reflog of the local branch named
after the destination branch regardless of what's being pushed. This can cause
false rejections or unintended data loss.

False rejection has been reported twice that I could find:

- 2023-07-26 - Stefan Haller reported local branch with a different name
               false rejection[0]
- 2025-05-08 - D. Ben Knoble reported detached HEAD false rejection[1]

The same root cause can result in data loss: when a same-name local branch
contains the remote tip but you --force-if-includes push an unrelated branch,
clobbering the remote repo. PoCs are in t/t5533-push-cas.sh -- new test cases
fail against maint, but pass with patches applied.

Existing tests covered refspecs with different names for --force-with-lease,
but missed --force-if-includes. New patches cover:

- allow forced-update using refspec with different-named local branch
- allow same as above, but with HEAD
- reject force-update using refspec with different-named local branch lacking
  branch tip
- reject same as above using HEAD
- reject detached HEAD

Resolved question: the detached HEAD case; HEAD's reflog was considered
and rejected as too broad for purpose in the original review. cf. [2]

[0]: <https://lore.kernel.org/git/f51c73ed-eb03-83ca-fb31-d3e2645c9a63@haller-berlin.de>
[1]: <https://lore.kernel.org/git/CALnO6CCk0SgwObQRnpd5Pt_DvCKF8dBmyVHivU6Nr_O-GusGLA@mail.gmail.com>
[2]: <https://lore.kernel.org/git/CAHLx=O=tVhtiZpaRP9TpfiBfOMS2xPe3c3=mC3VNEdBrLOioFg@mail.gmail.com>

Tyler Cipriani (2):
  push: check pushed ref for --force-if-includes
  push: fix --force-if-includes detached HEAD advice

 Documentation/config/advice.adoc |  4 ++
 advice.c                         |  1 +
 advice.h                         |  1 +
 builtin/push.c                   | 15 +++++++
 builtin/send-pack.c              |  5 +++
 remote.c                         | 27 +++++++++++-
 remote.h                         | 10 +++--
 send-pack.c                      |  1 +
 t/t5533-push-cas.sh              | 70 +++++++++++++++++++++++++++++++-
 transport-helper.c               |  5 +++
 transport.c                      |  8 ++++
 transport.h                      |  1 +
 12 files changed, 143 insertions(+), 5 deletions(-)

Range-diff against v2:
1:  da27c421ed = 1:  da27c421ed push: check pushed ref for --force-if-includes
2:  e07d16d53e = 2:  e07d16d53e push: fix --force-if-includes detached HEAD advice
-- 
2.47.3


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v3 1/2] push: check pushed ref for --force-if-includes
  2026-09-10 23:05 ` [PATCH v3 0/2] push: fix --force-if-includes consulting wrong ref Tyler Cipriani
@ 2026-09-10 23:05   ` Tyler Cipriani
  2026-09-11  6:55     ` Patrick Steinhardt
  2026-09-11 15:31     ` Junio C Hamano
  2026-09-10 23:05   ` [PATCH v3 2/2] push: fix --force-if-includes detached HEAD advice Tyler Cipriani
  1 sibling, 2 replies; 22+ messages in thread
From: Tyler Cipriani @ 2026-09-10 23:05 UTC (permalink / raw)
  To: git
  Cc: Srinidhi Kaushik, Stefan Haller, D . Ben Knoble, Phillip Wood,
	Johannes Schindelin, Tyler Cipriani

"--force-if-includes" ensures, "tip of the remote-tracking ref is
reachable from one of the 'reflog' entries of the local branch."

But check_if_includes_upstream() uses the local per-branch reflog based
on the destination branch rather than the branch being pushed; using
ref->name vs. ref->peer_ref->name.

This can cause confusing rejections or unintended data loss.

Using a command like:

    git push --force-if-includes --force-with-lease origin src:main

False rejections: when src is an up-to-date branch, but main is
out-of-date or nonexistent, then the includes check will fail telling
users the remote ref has been updated since the last checkout.

Data loss: when src is an orphan/out-dated branch, but main is
up-to-date, then the if-includes check will allow the push, clobbering
the remote main.

Find local reflog using ref->peer_ref. When using a refspec like
HEAD:refs/heads/main, we resolve HEAD. If HEAD is a branch, use that
branch's reflog.

But if HEAD does not resolve to a branch (i.e. a detached HEAD), then we
reject the push. HEAD's reflog is too broad to tell us if the history
being pushed includes the tip of the remote. Rejecting a detached HEAD
already happens today (if the same-named local branch lacks the remote
tip); now the detached HEAD state is explicitly rejected.

Skip deletions:

    git push --force-if-includes --force-with-lease origin :main

ref->deletion is set after apply_push_cas (which triggers
check_if_includes_upstream). The ref->peer_ref name is "(delete)".
Instead check with is_null_oid to detect and allow deletion.

Reported-by: Stefan Haller <lists@haller-berlin.de>
Reported-by: D. Ben Knoble <ben.knoble@gmail.com>
Signed-off-by: Tyler Cipriani <tyler@tylercipriani.com>
---
 remote.c            | 24 ++++++++++++++++-
 t/t5533-push-cas.sh | 65 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 88 insertions(+), 1 deletion(-)

diff --git a/remote.c b/remote.c
index 00723b385e..326af76eeb 100644
--- a/remote.c
+++ b/remote.c
@@ -2806,7 +2806,29 @@ static int is_reachable_in_reflog(const char *local, const struct ref *remote)
  */
 static void check_if_includes_upstream(struct ref *remote)
 {
-	struct ref *local = get_local_ref(remote->name);
+	struct ref *local;
+	const char *name;
+	int flag;
+
+	if (!remote->peer_ref)
+		return;
+
+	/* A deletion has no local history to check against. */
+	if (is_null_oid(&remote->peer_ref->new_oid))
+		return;
+
+	name = remote->peer_ref->name;
+	if (!strcmp(name, "HEAD")) {
+		name = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
+					       "HEAD", 0, NULL, &flag);
+		if (!name || !(flag & REF_ISSYMREF)) {
+			/* detached HEAD: no per-branch reflog to consult */
+			remote->unreachable = 1;
+			return;
+		}
+	}
+
+	local = get_local_ref(name);
 	if (!local)
 		return;
 
diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh
index cba26a872d..0c02151747 100755
--- a/t/t5533-push-cas.sh
+++ b/t/t5533-push-cas.sh
@@ -396,4 +396,69 @@ test_expect_success '"--force-if-includes" should allow deletes' '
 	)
 '
 
+test_expect_success '"--force-if-includes" should allow forced update when using differently named branches' '
+	setup_src_dup_dst &&
+	test_when_finished "rm -fr dst src dup" &&
+	(
+		cd src &&
+		git fetch &&
+		git switch -c newbranch origin/main &&
+		git rebase HEAD --onto HEAD^ &&
+		git push --force-if-includes --force-with-lease origin newbranch:main
+	)
+'
+test_expect_success '"--force-if-includes" should allow forced update from HEAD' '
+	setup_src_dup_dst &&
+	test_when_finished "rm -fr dst src dup" &&
+	(
+		cd src &&
+		git fetch &&
+		git switch -c newbranch origin/main &&
+		git rebase HEAD --onto HEAD^ &&
+		git push --force-if-includes --force-with-lease origin HEAD:main
+	)
+'
+
+test_expect_success '"--force-if-includes" should reject forced update from differently named branches when local lacks remote ref' '
+	setup_src_dup_dst &&
+	test_when_finished "rm -fr dst src dup" &&
+	(
+		cd src &&
+		git fetch &&
+		git switch main &&
+		git reset --hard origin/main &&
+		git switch --orphan orphan &&
+		test_commit I &&
+		test_must_fail git push --force-with-lease --force-if-includes origin orphan:main
+	)
+'
+
+test_expect_success '"--force-if-includes" should reject forced update from HEAD when it lacks remote ref' '
+	setup_src_dup_dst &&
+	test_when_finished "rm -fr dst src dup" &&
+	(
+		cd src &&
+		git fetch &&
+		git switch main &&
+		git reset --hard origin/main &&
+		git switch --orphan orphan &&
+		test_commit I &&
+		test_must_fail git push --force-with-lease --force-if-includes origin HEAD:main
+	)
+'
+
+test_expect_success '"--force-if-includes" should reject forced update from detached HEAD' '
+	setup_src_dup_dst &&
+	test_when_finished "rm -fr dst src dup" &&
+	(
+		cd src &&
+		git fetch &&
+		git switch main &&
+		git reset --hard origin/main &&
+		git switch -c newbranch origin/main &&
+		git checkout HEAD^ &&
+		test_must_fail git push --force-if-includes --force-with-lease origin HEAD:main
+	)
+'
+
 test_done
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 2/2] push: fix --force-if-includes detached HEAD advice
  2026-09-10 23:05 ` [PATCH v3 0/2] push: fix --force-if-includes consulting wrong ref Tyler Cipriani
  2026-09-10 23:05   ` [PATCH v3 1/2] push: check pushed ref for --force-if-includes Tyler Cipriani
@ 2026-09-10 23:05   ` Tyler Cipriani
  2026-09-11  6:55     ` Patrick Steinhardt
  2026-09-11 15:40     ` Junio C Hamano
  1 sibling, 2 replies; 22+ messages in thread
From: Tyler Cipriani @ 2026-09-10 23:05 UTC (permalink / raw)
  To: git
  Cc: Srinidhi Kaushik, Stefan Haller, D . Ben Knoble, Phillip Wood,
	Johannes Schindelin, Tyler Cipriani

When a --force-if-includes push is rejected due to a detached HEAD
state where there is no per-branch reflog to consult, the advice is
misleading:

     ! [rejected] HEAD -> main (remote ref updated since checkout)
    error: failed to push some refs to '<remote>'
    hint: Updates were rejected because the tip of the remote-tracking
    hint: branch has been updated since the last checkout. If you want
    hint: to integrate the remote changes, use 'git pull' before
    hint: pushing again. See the 'Note about fast-forwards' in 'git
    hint: push --help' for details.

But a `git pull` will not fix this rejection. What is required is either

- Specify the expected remote tip with --force-with-lease=<ref>:<expect>
- Ignore the error with --no-force-if-includes

Add ref->unverifiable to differentiate between a detached HEAD rejection
vs. a remote update rejection.

Ensure tests check the rejection message.

Reported-by: D. Ben Knoble <ben.knoble@gmail.com>
Signed-off-by: Tyler Cipriani <tyler@tylercipriani.com>
---
 Documentation/config/advice.adoc |  4 ++++
 advice.c                         |  1 +
 advice.h                         |  1 +
 builtin/push.c                   | 15 +++++++++++++++
 builtin/send-pack.c              |  5 +++++
 remote.c                         |  5 ++++-
 remote.h                         | 10 +++++++---
 send-pack.c                      |  1 +
 t/t5533-push-cas.sh              |  7 +++++--
 transport-helper.c               |  5 +++++
 transport.c                      |  8 ++++++++
 transport.h                      |  1 +
 12 files changed, 57 insertions(+), 6 deletions(-)

diff --git a/Documentation/config/advice.adoc b/Documentation/config/advice.adoc
index 257db58918..a0eff8bbd6 100644
--- a/Documentation/config/advice.adoc
+++ b/Documentation/config/advice.adoc
@@ -90,6 +90,10 @@ all advice messages.
 		Shown when linkgit:git-push[1] rejects a forced update of
 		a branch when its remote-tracking ref has updates that we
 		do not have locally.
+	pushRefUnverifiable::
+		Shown when linkgit:git-push[1] rejects a forced update of
+		a branch when we are unable to verify the remote-tracking
+		ref is available locally.
 	pushUnqualifiedRefname::
 		Shown when linkgit:git-push[1] gives up trying to
 		guess based on the source and destination refs what
diff --git a/advice.c b/advice.c
index 0018501b7b..08842deb66 100644
--- a/advice.c
+++ b/advice.c
@@ -69,6 +69,7 @@ static struct {
 	[ADVICE_PUSH_NON_FF_CURRENT]			= { "pushNonFFCurrent" },
 	[ADVICE_PUSH_NON_FF_MATCHING]			= { "pushNonFFMatching" },
 	[ADVICE_PUSH_REF_NEEDS_UPDATE]			= { "pushRefNeedsUpdate" },
+	[ADVICE_PUSH_REF_UNVERIFIABLE]			= { "pushRefUnverifiable" },
 	[ADVICE_PUSH_UNQUALIFIED_REF_NAME]		= { "pushUnqualifiedRefName" },
 	[ADVICE_PUSH_UPDATE_REJECTED]			= { "pushUpdateRejected" },
 	[ADVICE_PUSH_UPDATE_REJECTED_ALIAS]		= { "pushNonFastForward" }, /* backwards compatibility */
diff --git a/advice.h b/advice.h
index 8def280688..189eadc089 100644
--- a/advice.h
+++ b/advice.h
@@ -36,6 +36,7 @@ enum advice_type {
 	ADVICE_PUSH_NON_FF_CURRENT,
 	ADVICE_PUSH_NON_FF_MATCHING,
 	ADVICE_PUSH_REF_NEEDS_UPDATE,
+	ADVICE_PUSH_REF_UNVERIFIABLE,
 	ADVICE_PUSH_UNQUALIFIED_REF_NAME,
 	ADVICE_PUSH_UPDATE_REJECTED,
 	ADVICE_PUSH_UPDATE_REJECTED_ALIAS,
diff --git a/builtin/push.c b/builtin/push.c
index 6021b71d66..9676c6241f 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -319,6 +319,12 @@ static const char message_advice_ref_needs_update[] =
 	   "remote changes, use 'git pull' before pushing again.\n"
 	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
 
+static const char message_advice_ref_unverifiable[] =
+	N_("Updates were rejected because the tip of the remote-tracking branch\n"
+	   "cannot be checked against a detached HEAD. If you want to push anyway,\n"
+	   "specify the expected value with '--force-with-lease=<ref>:<expect>'\n"
+	   "or use '--no-force-if-includes' to skip this check.");
+
 static void advise_pull_before_push(void)
 {
 	if (!advice_enabled(ADVICE_PUSH_NON_FF_CURRENT) || !advice_enabled(ADVICE_PUSH_UPDATE_REJECTED))
@@ -361,6 +367,13 @@ static void advise_ref_needs_update(void)
 	advise(_(message_advice_ref_needs_update));
 }
 
+static void advise_ref_unverifiable(void)
+{
+	if (!advice_enabled(ADVICE_PUSH_REF_UNVERIFIABLE) || !advice_enabled(ADVICE_PUSH_UPDATE_REJECTED))
+		return;
+	advise(_(message_advice_ref_unverifiable));
+}
+
 static int push_with_options(struct transport *transport, struct refspec *rs,
 			     int flags)
 {
@@ -412,6 +425,8 @@ static int push_with_options(struct transport *transport, struct refspec *rs,
 		advise_ref_needs_force();
 	} else if (reject_reasons & REJECT_REF_NEEDS_UPDATE) {
 		advise_ref_needs_update();
+	} else if (reject_reasons & REJECT_REF_UNVERIFIABLE) {
+		advise_ref_unverifiable();
 	}
 
 	return 1;
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 1412b49bc8..07accb6e6b 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -76,6 +76,11 @@ static void print_helper_status(struct ref *ref)
 			msg = "remote ref updated since checkout";
 			break;
 
+		case REF_STATUS_REJECT_UNVERIFIABLE:
+			res = "error";
+			msg = "remote ref unverifiable";
+			break;
+
 		case REF_STATUS_REJECT_ALREADY_EXISTS:
 			res = "error";
 			msg = "already exists";
diff --git a/remote.c b/remote.c
index 326af76eeb..72bfc4dbc4 100644
--- a/remote.c
+++ b/remote.c
@@ -1701,6 +1701,9 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
 			else if (ref->check_reachable && ref->unreachable)
 				reject_reason =
 					REF_STATUS_REJECT_REMOTE_UPDATED;
+			else if (ref->check_reachable && ref->unverifiable)
+				reject_reason =
+					REF_STATUS_REJECT_UNVERIFIABLE;
 			else
 				/*
 				 * If the ref isn't stale, and is reachable
@@ -2823,7 +2826,7 @@ static void check_if_includes_upstream(struct ref *remote)
 					       "HEAD", 0, NULL, &flag);
 		if (!name || !(flag & REF_ISSYMREF)) {
 			/* detached HEAD: no per-branch reflog to consult */
-			remote->unreachable = 1;
+			remote->unverifiable = 1;
 			return;
 		}
 	}
diff --git a/remote.h b/remote.h
index 54b17e4b02..8e2d56c2c2 100644
--- a/remote.h
+++ b/remote.h
@@ -169,10 +169,13 @@ struct ref {
 		/* Need to check if local reflog reaches the remote tip. */
 		check_reachable:1,
 		/*
-		 * Store the result of the check enabled by "check_reachable";
-		 * implies the local reflog does not reach the remote tip.
+		 * Store the result of the check enabled by "check_reachable".
+		 * "unreachable" implies the local reflog does not reach the remote
+		 * tip. "unverifiable" implies no local branch reflog to check; i.e.,
+		 * detached HEAD.
 		 */
-		unreachable:1;
+		unreachable:1,
+		unverifiable:1;
 
 	enum {
 		REF_NOT_MATCHED = 0, /* initial value */
@@ -203,6 +206,7 @@ struct ref {
 		REF_STATUS_REJECT_STALE,
 		REF_STATUS_REJECT_SHALLOW,
 		REF_STATUS_REJECT_REMOTE_UPDATED,
+		REF_STATUS_REJECT_UNVERIFIABLE,
 		REF_STATUS_UPTODATE,
 		REF_STATUS_REMOTE_REJECT,
 		REF_STATUS_EXPECTING_REPORT,
diff --git a/send-pack.c b/send-pack.c
index 3bb5afc687..6b78470f37 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -322,6 +322,7 @@ static int check_to_send_update(const struct ref *ref, const struct send_pack_ar
 	case REF_STATUS_REJECT_NEEDS_FORCE:
 	case REF_STATUS_REJECT_STALE:
 	case REF_STATUS_REJECT_REMOTE_UPDATED:
+	case REF_STATUS_REJECT_UNVERIFIABLE:
 	case REF_STATUS_REJECT_NODELETE:
 		return CHECK_REF_STATUS_REJECTED;
 	case REF_STATUS_UPTODATE:
diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh
index 0c02151747..fe6af3f41c 100755
--- a/t/t5533-push-cas.sh
+++ b/t/t5533-push-cas.sh
@@ -311,7 +311,8 @@ test_expect_success 'background updates to remote can be mitigated with "--force
 		git switch main &&
 		test_commit J &&
 		git fetch --all &&
-		test_must_fail git push --force-with-lease --force-if-includes --all
+		test_must_fail git push --force-with-lease --force-if-includes --all 2>err &&
+		test_grep "remote ref updated since checkout" err
 	) &&
 	git ls-remote dst refs/heads/main >actual.main &&
 	git ls-remote dst refs/heads/branch >actual.branch &&
@@ -457,7 +458,9 @@ test_expect_success '"--force-if-includes" should reject forced update from deta
 		git reset --hard origin/main &&
 		git switch -c newbranch origin/main &&
 		git checkout HEAD^ &&
-		test_must_fail git push --force-if-includes --force-with-lease origin HEAD:main
+		test_must_fail git push --force-if-includes --force-with-lease origin HEAD:main 2>err &&
+		test_grep "remote ref unverifiable" err &&
+		test_grep "no-force-if-includes" err
 	)
 '
 
diff --git a/transport-helper.c b/transport-helper.c
index 80f90eb7ba..1763570352 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -893,6 +893,10 @@ static int push_update_ref_status(struct strbuf *buf,
 			status = REF_STATUS_REJECT_REMOTE_UPDATED;
 			FREE_AND_NULL(msg);
 		}
+		else if (!strcmp(msg, "remote ref unverifiable")) {
+			status = REF_STATUS_REJECT_UNVERIFIABLE;
+			FREE_AND_NULL(msg);
+		}
 		else if (!strcmp(msg, "forced update")) {
 			forced = 1;
 			FREE_AND_NULL(msg);
@@ -1046,6 +1050,7 @@ static int push_refs_with_push(struct transport *transport,
 		case REF_STATUS_REJECT_STALE:
 		case REF_STATUS_REJECT_ALREADY_EXISTS:
 		case REF_STATUS_REJECT_REMOTE_UPDATED:
+		case REF_STATUS_REJECT_UNVERIFIABLE:
 			if (atomic) {
 				reject_atomic_push(remote_refs, mirror);
 				string_list_clear(&cas_options, 0);
diff --git a/transport.c b/transport.c
index 0f5ec30247..3d60d6de54 100644
--- a/transport.c
+++ b/transport.c
@@ -779,6 +779,11 @@ static int print_one_push_report(struct ref *ref, const char *dest, int count,
 				 "remote ref updated since checkout",
 				 report, porcelain, summary_width);
 		break;
+	case REF_STATUS_REJECT_UNVERIFIABLE:
+		print_ref_status('!', "[rejected]", ref, ref->peer_ref,
+				 "remote ref unverifiable",
+				 report, porcelain, summary_width);
+		break;
 	case REF_STATUS_REJECT_SHALLOW:
 		print_ref_status('!', "[rejected]", ref, ref->peer_ref,
 				 "new shallow roots not allowed",
@@ -893,6 +898,8 @@ void transport_print_push_status(const char *dest, struct ref *refs,
 			*reject_reasons |= REJECT_NEEDS_FORCE;
 		} else if (ref->status == REF_STATUS_REJECT_REMOTE_UPDATED) {
 			*reject_reasons |= REJECT_REF_NEEDS_UPDATE;
+		} else if (ref->status == REF_STATUS_REJECT_UNVERIFIABLE) {
+			*reject_reasons |= REJECT_REF_UNVERIFIABLE;
 		}
 	}
 	free(head);
@@ -1348,6 +1355,7 @@ static int pre_push_hook_feed_stdin(int hook_stdin_fd, void *pp_cb UNUSED, void
 	switch (r->status) {
 	case REF_STATUS_REJECT_NONFASTFORWARD:
 	case REF_STATUS_REJECT_REMOTE_UPDATED:
+	case REF_STATUS_REJECT_UNVERIFIABLE:
 	case REF_STATUS_REJECT_STALE:
 	case REF_STATUS_UPTODATE:
 		return 0; /* skip refs which won't be pushed */
diff --git a/transport.h b/transport.h
index 7e5867cffa..eaa3b616ee 100644
--- a/transport.h
+++ b/transport.h
@@ -256,6 +256,7 @@ void transport_set_verbosity(struct transport *transport, int verbosity,
 #define REJECT_FETCH_FIRST      0x08
 #define REJECT_NEEDS_FORCE      0x10
 #define REJECT_REF_NEEDS_UPDATE 0x20
+#define REJECT_REF_UNVERIFIABLE 0x40
 
 int transport_push(struct repository *repo,
 		   struct transport *connection,
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 1/2] push: check pushed ref for --force-if-includes
  2026-09-10 23:05   ` [PATCH v3 1/2] push: check pushed ref for --force-if-includes Tyler Cipriani
@ 2026-09-11  6:55     ` Patrick Steinhardt
  2026-09-11 22:58       ` Tyler Cipriani
  2026-09-11 15:31     ` Junio C Hamano
  1 sibling, 1 reply; 22+ messages in thread
From: Patrick Steinhardt @ 2026-09-11  6:55 UTC (permalink / raw)
  To: Tyler Cipriani
  Cc: git, Srinidhi Kaushik, Stefan Haller, D . Ben Knoble,
	Phillip Wood, Johannes Schindelin

On Thu, Sep 10, 2026 at 05:05:05PM -0600, Tyler Cipriani wrote:
> "--force-if-includes" ensures, "tip of the remote-tracking ref is
> reachable from one of the 'reflog' entries of the local branch."
> 
> But check_if_includes_upstream() uses the local per-branch reflog based
> on the destination branch rather than the branch being pushed; using
> ref->name vs. ref->peer_ref->name.

So... in a `git push origin foo:bar` we look up the reflog for "bar" and
not "foo"?

> This can cause confusing rejections or unintended data loss.
> 
> Using a command like:
> 
>     git push --force-if-includes --force-with-lease origin src:main
> 
> False rejections: when src is an up-to-date branch, but main is
> out-of-date or nonexistent, then the includes check will fail telling
> users the remote ref has been updated since the last checkout.

Hm. "up-to-date branch" in relation to what? You mean if we had commits
A, B and C, with C being the most recent commit, then "src" points to C
and "main" points to B?

> Data loss: when src is an orphan/out-dated branch, but main is
> up-to-date, then the if-includes check will allow the push, clobbering
> the remote main.

Right, here "src" would point to B and "main" would point to C.

> Find local reflog using ref->peer_ref. When using a refspec like
> HEAD:refs/heads/main, we resolve HEAD. If HEAD is a branch, use that
> branch's reflog.
> 
> But if HEAD does not resolve to a branch (i.e. a detached HEAD), then we
> reject the push. HEAD's reflog is too broad to tell us if the history
> being pushed includes the tip of the remote. Rejecting a detached HEAD
> already happens today (if the same-named local branch lacks the remote
> tip); now the detached HEAD state is explicitly rejected.

Makes sense.

> Skip deletions:
> 
>     git push --force-if-includes --force-with-lease origin :main
> 
> ref->deletion is set after apply_push_cas (which triggers
> check_if_includes_upstream). The ref->peer_ref name is "(delete)".
> Instead check with is_null_oid to detect and allow deletion.

This part feels a bit off to me. Deletions are the most risky operation
that we can do, so why would we want to just blindly allow them? There
may be good reasons for this, but if so those should be documented as
part of the commit message. It would probably even be sufficient to say
"it has worked this way before, and we don't want to break that case".

> diff --git a/remote.c b/remote.c
> index 00723b385e..326af76eeb 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -2806,7 +2806,29 @@ static int is_reachable_in_reflog(const char *local, const struct ref *remote)
>   */
>  static void check_if_includes_upstream(struct ref *remote)
>  {
> -	struct ref *local = get_local_ref(remote->name);
> +	struct ref *local;
> +	const char *name;
> +	int flag;
> +
> +	if (!remote->peer_ref)
> +		return;
> +
> +	/* A deletion has no local history to check against. */
> +	if (is_null_oid(&remote->peer_ref->new_oid))
> +		return;
> +
> +	name = remote->peer_ref->name;
> +	if (!strcmp(name, "HEAD")) {
> +		name = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
> +					       "HEAD", 0, NULL, &flag);

Shouldn't we pass `RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE` here?
Otherwise, the function will return "HEAD" even if it could not be
resolved, and we don't want to recursively resolve symrefs, either.

Also, is it sufficient to single out "HEAD" here? It could for example
be that the user passes "HEAD~", an object ID or really any other
revision, and these should probably not be considered reachable, either,
right?

Maybe we should instead verify whether this names a local reference and,
if so, resolve potential symrefs to their target.

> diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh
> index cba26a872d..0c02151747 100755
> --- a/t/t5533-push-cas.sh
> +++ b/t/t5533-push-cas.sh
> @@ -396,4 +396,69 @@ test_expect_success '"--force-if-includes" should allow deletes' '
>  	)
>  '
>  
> +test_expect_success '"--force-if-includes" should allow forced update when using differently named branches' '
> +	setup_src_dup_dst &&
> +	test_when_finished "rm -fr dst src dup" &&
> +	(
> +		cd src &&
> +		git fetch &&
> +		git switch -c newbranch origin/main &&
> +		git rebase HEAD --onto HEAD^ &&
> +		git push --force-if-includes --force-with-lease origin newbranch:main
> +	)
> +'

Nit: missing empty line between these two tests.

Patrick

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 2/2] push: fix --force-if-includes detached HEAD advice
  2026-09-10 23:05   ` [PATCH v3 2/2] push: fix --force-if-includes detached HEAD advice Tyler Cipriani
@ 2026-09-11  6:55     ` Patrick Steinhardt
  2026-09-11 16:03       ` Junio C Hamano
  2026-09-11 15:40     ` Junio C Hamano
  1 sibling, 1 reply; 22+ messages in thread
From: Patrick Steinhardt @ 2026-09-11  6:55 UTC (permalink / raw)
  To: Tyler Cipriani
  Cc: git, Srinidhi Kaushik, Stefan Haller, D . Ben Knoble,
	Phillip Wood, Johannes Schindelin

On Thu, Sep 10, 2026 at 05:05:06PM -0600, Tyler Cipriani wrote:
> diff --git a/Documentation/config/advice.adoc b/Documentation/config/advice.adoc
> index 257db58918..a0eff8bbd6 100644
> --- a/Documentation/config/advice.adoc
> +++ b/Documentation/config/advice.adoc
> @@ -90,6 +90,10 @@ all advice messages.
>  		Shown when linkgit:git-push[1] rejects a forced update of
>  		a branch when its remote-tracking ref has updates that we
>  		do not have locally.
> +	pushRefUnverifiable::
> +		Shown when linkgit:git-push[1] rejects a forced update of
> +		a branch when we are unable to verify the remote-tracking
> +		ref is available locally.

We don't really care about the ref being available, but rather about it
being integrated, right? So maybe s/available/integrated/.

Patrick

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 1/2] push: check pushed ref for --force-if-includes
  2026-09-10 23:05   ` [PATCH v3 1/2] push: check pushed ref for --force-if-includes Tyler Cipriani
  2026-09-11  6:55     ` Patrick Steinhardt
@ 2026-09-11 15:31     ` Junio C Hamano
  2026-09-11 23:47       ` Tyler Cipriani
  1 sibling, 1 reply; 22+ messages in thread
From: Junio C Hamano @ 2026-09-11 15:31 UTC (permalink / raw)
  To: Tyler Cipriani
  Cc: git, Srinidhi Kaushik, Stefan Haller, D . Ben Knoble,
	Phillip Wood, Johannes Schindelin

Tyler Cipriani <tyler@tylercipriani.com> writes:

>  static void check_if_includes_upstream(struct ref *remote)
>  {
> -	struct ref *local = get_local_ref(remote->name);
> +	struct ref *local;
> +	const char *name;
> +	int flag;
> +
> +	if (!remote->peer_ref)
> +		return;

This function signals its displeasure by setting remote->unreachble
to true, so any early return means it is OK to force the push, right?

What is the significance of remote not having peer_ref?  Is it a
usage error (i.e., push is not updating anything over there, and it
makes me wonder what the command line to do so looks like)?  Is it a
programming error (i.e., if we are pushing to update no remote ref,
this function should never be called)?  If the latter, I wonder if
BUG() is more appropriate.

> +	/* A deletion has no local history to check against. */
> +	if (is_null_oid(&remote->peer_ref->new_oid))
> +		return;

The comment for this condition is clear.  If we are pushing to
delete, checking if our side once used to build on top of theirs
does not guarantee us anything, so we accept the loss of history.

> +	name = remote->peer_ref->name;
> +	if (!strcmp(name, "HEAD")) {
> +		name = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
> +					       "HEAD", 0, NULL, &flag);
> +		if (!name || !(flag & REF_ISSYMREF)) {
> +			/* detached HEAD: no per-branch reflog to consult */
> +			remote->unreachable = 1;
> +			return;
> +		}
> +	}
> +
> +	local = get_local_ref(name);
>  	if (!local)
>  		return;

The same question here.

Are any of these silent "punt" returns tested below?  It does not
seem to add a new test about pushing-to-delete.

Thanks.

> diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh
> index cba26a872d..0c02151747 100755
> --- a/t/t5533-push-cas.sh
> +++ b/t/t5533-push-cas.sh
> @@ -396,4 +396,69 @@ test_expect_success '"--force-if-includes" should allow deletes' '
>  	)
>  '
>  
> +test_expect_success '"--force-if-includes" should allow forced update when using differently named branches' '
> +	setup_src_dup_dst &&
> +	test_when_finished "rm -fr dst src dup" &&
> +	(
> +		cd src &&
> +		git fetch &&
> +		git switch -c newbranch origin/main &&
> +		git rebase HEAD --onto HEAD^ &&
> +		git push --force-if-includes --force-with-lease origin newbranch:main
> +	)
> +'
> +test_expect_success '"--force-if-includes" should allow forced update from HEAD' '
> +	setup_src_dup_dst &&
> +	test_when_finished "rm -fr dst src dup" &&
> +	(
> +		cd src &&
> +		git fetch &&
> +		git switch -c newbranch origin/main &&
> +		git rebase HEAD --onto HEAD^ &&
> +		git push --force-if-includes --force-with-lease origin HEAD:main
> +	)
> +'
> +
> +test_expect_success '"--force-if-includes" should reject forced update from differently named branches when local lacks remote ref' '
> +	setup_src_dup_dst &&
> +	test_when_finished "rm -fr dst src dup" &&
> +	(
> +		cd src &&
> +		git fetch &&
> +		git switch main &&
> +		git reset --hard origin/main &&
> +		git switch --orphan orphan &&
> +		test_commit I &&
> +		test_must_fail git push --force-with-lease --force-if-includes origin orphan:main
> +	)
> +'
> +
> +test_expect_success '"--force-if-includes" should reject forced update from HEAD when it lacks remote ref' '
> +	setup_src_dup_dst &&
> +	test_when_finished "rm -fr dst src dup" &&
> +	(
> +		cd src &&
> +		git fetch &&
> +		git switch main &&
> +		git reset --hard origin/main &&
> +		git switch --orphan orphan &&
> +		test_commit I &&
> +		test_must_fail git push --force-with-lease --force-if-includes origin HEAD:main
> +	)
> +'
> +
> +test_expect_success '"--force-if-includes" should reject forced update from detached HEAD' '
> +	setup_src_dup_dst &&
> +	test_when_finished "rm -fr dst src dup" &&
> +	(
> +		cd src &&
> +		git fetch &&
> +		git switch main &&
> +		git reset --hard origin/main &&
> +		git switch -c newbranch origin/main &&
> +		git checkout HEAD^ &&
> +		test_must_fail git push --force-if-includes --force-with-lease origin HEAD:main
> +	)
> +'
> +
>  test_done

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 2/2] push: fix --force-if-includes detached HEAD advice
  2026-09-10 23:05   ` [PATCH v3 2/2] push: fix --force-if-includes detached HEAD advice Tyler Cipriani
  2026-09-11  6:55     ` Patrick Steinhardt
@ 2026-09-11 15:40     ` Junio C Hamano
  1 sibling, 0 replies; 22+ messages in thread
From: Junio C Hamano @ 2026-09-11 15:40 UTC (permalink / raw)
  To: Tyler Cipriani
  Cc: git, Srinidhi Kaushik, Stefan Haller, D . Ben Knoble,
	Phillip Wood, Johannes Schindelin

Tyler Cipriani <tyler@tylercipriani.com> writes:

> When a --force-if-includes push is rejected due to a detached HEAD
> state where there is no per-branch reflog to consult, the advice is
> misleading:
>
>      ! [rejected] HEAD -> main (remote ref updated since checkout)
>     error: failed to push some refs to '<remote>'
>     hint: Updates were rejected because the tip of the remote-tracking
>     hint: branch has been updated since the last checkout. If you want
>     hint: to integrate the remote changes, use 'git pull' before
>     hint: pushing again. See the 'Note about fast-forwards' in 'git
>     hint: push --help' for details.
>
> But a `git pull` will not fix this rejection. What is required is either
>
> - Specify the expected remote tip with --force-with-lease=<ref>:<expect>
> - Ignore the error with --no-force-if-includes
>
> Add ref->unverifiable to differentiate between a detached HEAD rejection
> vs. a remote update rejection.

Makes sense.

> diff --git a/builtin/push.c b/builtin/push.c
> index 6021b71d66..9676c6241f 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -319,6 +319,12 @@ static const char message_advice_ref_needs_update[] =
>  	   "remote changes, use 'git pull' before pushing again.\n"
>  	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
>  
> +static const char message_advice_ref_unverifiable[] =
> +	N_("Updates were rejected because the tip of the remote-tracking branch\n"
> +	   "cannot be checked against a detached HEAD. If you want to push anyway,\n"
> +	   "specify the expected value with '--force-with-lease=<ref>:<expect>'\n"
> +	   "or use '--no-force-if-includes' to skip this check.");

Good.

> +static void advise_ref_unverifiable(void)
> +{
> +	if (!advice_enabled(ADVICE_PUSH_REF_UNVERIFIABLE) || !advice_enabled(ADVICE_PUSH_UPDATE_REJECTED))
> +		return;

Line that is over +100 column wide?

> +	advise(_(message_advice_ref_unverifiable));
> +}

This is a tangent, but on a separate thread we were talking about
consolidating a sequence

    if (advice_enabled(ADVICE_FOO))
	advise(_(message for FOO));

into

    advise_if_enabled(ADVICE_FOO, _(message for FOO));

This is an example of usage that falls outside of the pattern (not a
bad thing; just what those who advocate more use of advise_if_enabled()
need to be aware of).

> diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh
> index 0c02151747..fe6af3f41c 100755
> --- a/t/t5533-push-cas.sh
> +++ b/t/t5533-push-cas.sh
> @@ -311,7 +311,8 @@ test_expect_success 'background updates to remote can be mitigated with "--force
>  		git switch main &&
>  		test_commit J &&
>  		git fetch --all &&
> -		test_must_fail git push --force-with-lease --force-if-includes --all
> +		test_must_fail git push --force-with-lease --force-if-includes --all 2>err &&
> +		test_grep "remote ref updated since checkout" err
>  	) &&
>  	git ls-remote dst refs/heads/main >actual.main &&
>  	git ls-remote dst refs/heads/branch >actual.branch &&
> @@ -457,7 +458,9 @@ test_expect_success '"--force-if-includes" should reject forced update from deta
>  		git reset --hard origin/main &&
>  		git switch -c newbranch origin/main &&
>  		git checkout HEAD^ &&
> -		test_must_fail git push --force-if-includes --force-with-lease origin HEAD:main
> +		test_must_fail git push --force-if-includes --force-with-lease origin HEAD:main 2>err &&
> +		test_grep "remote ref unverifiable" err &&
> +		test_grep "no-force-if-includes" err
>  	)
>  '

Great.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 2/2] push: fix --force-if-includes detached HEAD advice
  2026-09-11  6:55     ` Patrick Steinhardt
@ 2026-09-11 16:03       ` Junio C Hamano
  0 siblings, 0 replies; 22+ messages in thread
From: Junio C Hamano @ 2026-09-11 16:03 UTC (permalink / raw)
  To: Patrick Steinhardt
  Cc: Tyler Cipriani, git, Srinidhi Kaushik, Stefan Haller,
	D . Ben Knoble, Phillip Wood, Johannes Schindelin

Patrick Steinhardt <ps@pks.im> writes:

> On Thu, Sep 10, 2026 at 05:05:06PM -0600, Tyler Cipriani wrote:
>> diff --git a/Documentation/config/advice.adoc b/Documentation/config/advice.adoc
>> index 257db58918..a0eff8bbd6 100644
>> --- a/Documentation/config/advice.adoc
>> +++ b/Documentation/config/advice.adoc
>> @@ -90,6 +90,10 @@ all advice messages.
>>  		Shown when linkgit:git-push[1] rejects a forced update of
>>  		a branch when its remote-tracking ref has updates that we
>>  		do not have locally.
>> +	pushRefUnverifiable::
>> +		Shown when linkgit:git-push[1] rejects a forced update of
>> +		a branch when we are unable to verify the remote-tracking
>> +		ref is available locally.
>
> We don't really care about the ref being available, but rather about it
> being integrated, right? So maybe s/available/integrated/.

Ah, I missed that one.  "available locally" is not of interest.  We
cannot tell if we integrated it is what matters.

Thanks.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 1/2] push: check pushed ref for --force-if-includes
  2026-09-11  6:55     ` Patrick Steinhardt
@ 2026-09-11 22:58       ` Tyler Cipriani
  0 siblings, 0 replies; 22+ messages in thread
From: Tyler Cipriani @ 2026-09-11 22:58 UTC (permalink / raw)
  To: ps
  Cc: git, Srinidhi Kaushik, Stefan Haller, D . Ben Knoble,
	Phillip Wood, Johannes Schindelin

On Fri, Sep 11, 2026 at 12:55 AM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Thu, Sep 10, 2026 at 05:05:05PM -0600, Tyler Cipriani wrote:
> > "--force-if-includes" ensures, "tip of the remote-tracking ref is
> > reachable from one of the 'reflog' entries of the local branch."
> >
> > But check_if_includes_upstream() uses the local per-branch reflog based
> > on the destination branch rather than the branch being pushed; using
> > ref->name vs. ref->peer_ref->name.
>
> So... in a `git push origin foo:bar` we look up the reflog for "bar" and
> not "foo"?

Exactly.

> > This can cause confusing rejections or unintended data loss.
> >
> > Using a command like:
> >
> >     git push --force-if-includes --force-with-lease origin src:main
> >
> > False rejections: when src is an up-to-date branch, but main is
> > out-of-date or nonexistent, then the includes check will fail telling
> > users the remote ref has been updated since the last checkout.
>
> Hm. "up-to-date branch" in relation to what? You mean if we had commits
> A, B and C, with C being the most recent commit, then "src" points to C
> and "main" points to B?

You got it. It should read something like: "False rejections: when src
is up-to-date with the tip of the remote ref, but..." etc.

I can clarify in a v4.

> > Data loss: when src is an orphan/out-dated branch, but main is
> > up-to-date, then the if-includes check will allow the push, clobbering
> > the remote main.
>
> Right, here "src" would point to B and "main" would point to C.
>
> > Find local reflog using ref->peer_ref. When using a refspec like
> > HEAD:refs/heads/main, we resolve HEAD. If HEAD is a branch, use that
> > branch's reflog.
> >
> > But if HEAD does not resolve to a branch (i.e. a detached HEAD), then we
> > reject the push. HEAD's reflog is too broad to tell us if the history
> > being pushed includes the tip of the remote. Rejecting a detached HEAD
> > already happens today (if the same-named local branch lacks the remote
> > tip); now the detached HEAD state is explicitly rejected.
>
> Makes sense.
>
> > Skip deletions:
> >
> >     git push --force-if-includes --force-with-lease origin :main
> >
> > ref->deletion is set after apply_push_cas (which triggers
> > check_if_includes_upstream). The ref->peer_ref name is "(delete)".
> > Instead check with is_null_oid to detect and allow deletion.
>
> This part feels a bit off to me. Deletions are the most risky operation
> that we can do, so why would we want to just blindly allow them? There
> may be good reasons for this, but if so those should be documented as
> part of the commit message. It would probably even be sufficient to say
> "it has worked this way before, and we don't want to break that case".

For deletions, there's no history on our side to check. Also, there
was an existing test case that ensured deletions were allowed. I took
that as intent and opted to keep that behavior. I'll clarify in the
commit.

> > diff --git a/remote.c b/remote.c
> > index 00723b385e..326af76eeb 100644
> > --- a/remote.c
> > +++ b/remote.c
> > @@ -2806,7 +2806,29 @@ static int is_reachable_in_reflog(const char *local, const struct ref *remote)
> >   */
> >  static void check_if_includes_upstream(struct ref *remote)
> >  {
> > -     struct ref *local = get_local_ref(remote->name);
> > +     struct ref *local;
> > +     const char *name;
> > +     int flag;
> > +
> > +     if (!remote->peer_ref)
> > +             return;
> > +
> > +     /* A deletion has no local history to check against. */
> > +     if (is_null_oid(&remote->peer_ref->new_oid))
> > +             return;
> > +
> > +     name = remote->peer_ref->name;
> > +     if (!strcmp(name, "HEAD")) {
> > +             name = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
> > +                                            "HEAD", 0, NULL, &flag);
>
> Shouldn't we pass `RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE` here?
> Otherwise, the function will return "HEAD" even if it could not be
> resolved, and we don't want to recursively resolve symrefs, either.

RESOLVE_REF_READING: agreed. Will add.
RESOLVE_REF_NO_RECURSE: For the current (v3) state that only looks at
"HEAD" that makes sense. But I'd expect --force-if-includes to
resolve, e.g., STABLE -> HEAD -> refs/heads/main -- that is, to
recurse through multiple symlinks. Otherwise, we'd reject a push we
could verify.

> Also, is it sufficient to single out "HEAD" here? It could for example
> be that the user passes "HEAD~", an object ID or really any other
> revision, and these should probably not be considered reachable, either,
> right?

Oooh, great catch! Folks could put in tags or specific oids, none of
which have a reflog to check. These are rejected in v3, but only by
happenstance since they lack a reflog (and with bad advice about
running "git pull").

> Maybe we should instead verify whether this names a local reference and,
> if so, resolve potential symrefs to their target.

Yes, that makes sense. Resolve symrefs to the target branch, then
check the branch's reflog.

I'll try that in v4.

This comment left me spiraling for a bit about tags. Like: you can
push tags and a tag and a branch might point to the same commit. BUT
tags don't have reflogs, so I think it makes sense to reject those as
unverifiable here, too.

> > diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh
> > index cba26a872d..0c02151747 100755
> > --- a/t/t5533-push-cas.sh
> > +++ b/t/t5533-push-cas.sh
> > @@ -396,4 +396,69 @@ test_expect_success '"--force-if-includes" should allow deletes' '
> >       )
> >  '
> >
> > +test_expect_success '"--force-if-includes" should allow forced update when using differently named branches' '
> > +     setup_src_dup_dst &&
> > +     test_when_finished "rm -fr dst src dup" &&
> > +     (
> > +             cd src &&
> > +             git fetch &&
> > +             git switch -c newbranch origin/main &&
> > +             git rebase HEAD --onto HEAD^ &&
> > +             git push --force-if-includes --force-with-lease origin newbranch:main
> > +     )
> > +'
>
> Nit: missing empty line between these two tests.

Ack.

> Patrick

Thanks for the review!

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 1/2] push: check pushed ref for --force-if-includes
  2026-09-11 15:31     ` Junio C Hamano
@ 2026-09-11 23:47       ` Tyler Cipriani
  0 siblings, 0 replies; 22+ messages in thread
From: Tyler Cipriani @ 2026-09-11 23:47 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Srinidhi Kaushik, Stefan Haller, D . Ben Knoble,
	Phillip Wood, Johannes Schindelin

On Fri, Sep 11, 2026 at 9:31 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Tyler Cipriani <tyler@tylercipriani.com> writes:
>
> >  static void check_if_includes_upstream(struct ref *remote)
> >  {
> > -     struct ref *local = get_local_ref(remote->name);
> > +     struct ref *local;
> > +     const char *name;
> > +     int flag;
> > +
> > +     if (!remote->peer_ref)
> > +             return;
>
> This function signals its displeasure by setting remote->unreachble
> to true, so any early return means it is OK to force the push, right?

That's true, for each ref that will be pushed. But this return does
not imply it's OK to force push; refs with no peer_ref are not part of
the push. The caller (apply_push_cas) walks every ref in remote_refs,
then this function gets called for each ref that has check_reachable,
regardless of whether it will later be pushed.

We could move this check to apply_push_cas to winnow what
check_if_includes_upstream is responsible for checking and make every
bare return mean "OK to force"; i.e., change apply_push_cas from:

if (ref->check_reachable)
    check_if_includes_upstream(ref);

to:

if (ref->peer_ref && ref->check_reachable)
    check_if_includes_upstream(ref);

And drop this return (and probably add a comment). I like that better.

> What is the significance of remote not having peer_ref?  Is it a
> usage error (i.e., push is not updating anything over there, and it
> makes me wonder what the command line to do so looks like)?  Is it a
> programming error (i.e., if we are pushing to update no remote ref,
> this function should never be called)?  If the latter, I wonder if
> BUG() is more appropriate.

This is an ordinary path vs. BUG(). For the command:

git --force-with-lease --force-if-includes origin main

apply_push_cas checks all advertised refs. If there's no peer_ref,
then remote.c's set_ref_status_for_push skips the ref before even
checking ref->unreachable. When I ran the coverage report, this guard
was hit regularly.

> > +     /* A deletion has no local history to check against. */
> > +     if (is_null_oid(&remote->peer_ref->new_oid))
> > +             return;
>
> The comment for this condition is clear.  If we are pushing to
> delete, checking if our side once used to build on top of theirs
> does not guarantee us anything, so we accept the loss of history.

Agreed.

> > +     name = remote->peer_ref->name;
> > +     if (!strcmp(name, "HEAD")) {
> > +             name = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
> > +                                            "HEAD", 0, NULL, &flag);
> > +             if (!name || !(flag & REF_ISSYMREF)) {
> > +                     /* detached HEAD: no per-branch reflog to consult */
> > +                     remote->unreachable = 1;
> > +                     return;
> > +             }
> > +     }
> > +
> > +     local = get_local_ref(name);
> >       if (!local)
> >               return;
>
> The same question here.

get_local_ref should not return null. And when I ran the coverage
report, this guard never ran. I'd be happy to remove it in v4.

> Are any of these silent "punt" returns tested below?  It does not
> seem to add a new test about pushing-to-delete.

There is an existing test for push-to-delete that this patch set kept.

> Thanks.

Thanks for the review!

Patrick suggested generalizing away from checking "HEAD" and I think
that's the right call. I'll try that, plus adding your feedback (plus
some additional detail in comments) in a v4.

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2026-09-11 23:47 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-04 21:01 [PATCH 0/2] push: fix --force-if-includes consulting wrong ref Tyler Cipriani
2026-09-04 21:01 ` [PATCH 1/2] push: check pushed ref for --force-if-includes Tyler Cipriani
2026-09-05 18:57   ` Ben Knoble
2026-09-04 21:01 ` [PATCH 2/2] push: fix --force-if-includes detached HEAD advice Tyler Cipriani
2026-09-05 18:59 ` [PATCH 0/2] push: fix --force-if-includes consulting wrong ref Ben Knoble
2026-09-06 20:24   ` Tyler Cipriani
2026-09-08 22:20 ` [PATCH v2 " Tyler Cipriani
2026-09-09 11:59   ` D. Ben Knoble
2026-09-08 22:20 ` [PATCH v2 1/2] push: check pushed ref for --force-if-includes Tyler Cipriani
2026-09-10 18:43   ` Junio C Hamano
2026-09-10 22:08     ` Tyler Cipriani
2026-09-08 22:20 ` [PATCH v2 2/2] push: fix --force-if-includes detached HEAD advice Tyler Cipriani
2026-09-10 23:05 ` [PATCH v3 0/2] push: fix --force-if-includes consulting wrong ref Tyler Cipriani
2026-09-10 23:05   ` [PATCH v3 1/2] push: check pushed ref for --force-if-includes Tyler Cipriani
2026-09-11  6:55     ` Patrick Steinhardt
2026-09-11 22:58       ` Tyler Cipriani
2026-09-11 15:31     ` Junio C Hamano
2026-09-11 23:47       ` Tyler Cipriani
2026-09-10 23:05   ` [PATCH v3 2/2] push: fix --force-if-includes detached HEAD advice Tyler Cipriani
2026-09-11  6:55     ` Patrick Steinhardt
2026-09-11 16:03       ` Junio C Hamano
2026-09-11 15:40     ` Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox