remote-helper: Account for first push from delegate

An edge case was found where a delegate attempted to merge a patch,
but had not pushed to their default branch yet.
This resulted in the `old` not being defined and the check to revert
and/or merge not happening – so the patch continued to appear as open.

Since `old` only matters for the revert and merge check for the
default branch, resolving the `old` side does its best to find a
commit for the checks.
It does this by falling back to getting the canonical head before the
push, and using that as the old.
This commit is contained in:
Fabrice Bellamy 2025-12-10 22:06:08 +01:00 committed by Fintan Halpenny
parent c70d4a29eb
commit cf28294473
2 changed files with 24 additions and 5 deletions

View File

@ -5,6 +5,16 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
## [Unreleased]
## Fixed Bugs
- Previously, a delegate could push to the default branch, for the first time,
while attempting to merge a patch. However, the patch would not be marked as
merged. The Git remote helper will now fallback to the canonical default
branch, if the default branch of the delegate could not be found, and
correctly mark the patch as merged.
## 1.9.1
## Improvements

View File

@ -800,8 +800,18 @@ where
{
let head = *src;
let dst = dst.with_namespace(nid.into());
// It's ok for the destination reference to be unknown, eg. when pushing a new branch.
let old = stored.backend.find_reference(dst.as_str()).ok();
// In some rare cases, pushing to the default branch is a new action for a delegate.
// If this is the case, use the canonical head before the push to determine the old side.
//
// Note that this is only important for reverting and merging, but must happen before the push.
let old = stored
.backend
.find_reference(dst.as_str())
.ok()
.map(|old| old.peel_to_commit().map(|c| c.id().into()))
.transpose()?
.or_else(|| stored.canonical_head().map(|(_, oid)| oid).ok());
push_ref(
src,
@ -820,11 +830,10 @@ where
// If we're pushing to the project's default branch, we want to see if any patches got
// merged or reverted, and if so, update the patch COB.
if &*dst.strip_namespace() == master {
let old = old.peel_to_commit()?.id();
// Only delegates affect the merge state of the COB.
if stored.delegates()?.contains(&nid.into()) {
patch_revert_all(old.into(), head, &stored.backend, &mut patches)?;
patch_merge_all(old.into(), head, working, &mut patches, signer)?;
patch_revert_all(old, head, &stored.backend, &mut patches)?;
patch_merge_all(old, head, working, &mut patches, signer)?;
}
}
}