From cf2829447362f6903bc07ebdffc29f0aa3445014 Mon Sep 17 00:00:00 2001 From: Fabrice Bellamy Date: Wed, 10 Dec 2025 22:06:08 +0100 Subject: [PATCH] remote-helper: Account for first push from delegate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- CHANGELOG.md | 10 ++++++++++ crates/radicle-remote-helper/src/push.rs | 19 ++++++++++++++----- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b633ce0..166838f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/crates/radicle-remote-helper/src/push.rs b/crates/radicle-remote-helper/src/push.rs index ec6b25b4..61c1f901 100644 --- a/crates/radicle-remote-helper/src/push.rs +++ b/crates/radicle-remote-helper/src/push.rs @@ -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)?; } } }