From a6eec6687c34b631309586d030e28c87596c0566 Mon Sep 17 00:00:00 2001 From: Jochen Hunz Date: Tue, 15 Sep 2026 22:39:07 +0200 Subject: [PATCH 1/4] Only follow a parent the server renumbered, never a moved head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A push that carries more than one local revision into a branch head that has moved silently drops the other side's work. The push succeeds, no conflict is reported, and the overwritten commit is still in the history, so nothing looks wrong afterwards. `collect_fragments_and_push` bent a revision's parent pointer onto whatever the previous iteration left in `current_latest`, keeping its tree: if !current_latest.is_zero() && state.parent_self() != current_latest { state.set_parent_self(current_latest); } That is right for the case the TODO describes: the server accepts the revision as it stands and only rewrites its revision number, so the same content is stored under a new signature and the next revision has to follow it. It is wrong after a server-side fast-forward merge. `current_latest` is then a revision the server built, carrying the other side's work, and re-pointing at it makes this revision's stale tree look like a direct descendant. The server takes the ordinary fast-forward path — no merge, no three-way diff, no conflict check — and the branch becomes that tree. Everything the other side added is simply absent from it. Track the rename instead: remember the pushed revision as (signature in local history, signature on the server) and follow the parent only when it is exactly that revision, and only when the two names differ. Clear it whenever the head moved to a merge. Any other mismatch is now left alone, which sends the revision as it stands and lets the server three-way merge it against a base it already holds — the revision's own parent, which reached the server as the previous merge's second parent. So a chain of N local revisions pushes correctly, one server merge each, instead of the last one winning outright. The guard `!current_latest.is_zero()` is why this was never seen with a single-revision push: the block cannot fire on the first revision of a push, only from the second on. Reproduction added at contrib/repro-push-reparent-drops-work.sh — CLI and loreserver only, no other dependencies. Against 0.9.0 it reports RESULT: LOST — A's file is gone from the branch, with no error reported and with this change RESULT: OK — A's file survived B's push `cargo test -p lore-revision` is green (700+ tests, no failures). Signed-off-by: Jochen Hunz --- contrib/repro-push-reparent-drops-work.sh | 100 ++++++++++++++++++++++ lore-revision/src/branch/push.rs | 59 ++++++++++--- 2 files changed, 149 insertions(+), 10 deletions(-) create mode 100755 contrib/repro-push-reparent-drops-work.sh diff --git a/contrib/repro-push-reparent-drops-work.sh b/contrib/repro-push-reparent-drops-work.sh new file mode 100755 index 00000000..1297498d --- /dev/null +++ b/contrib/repro-push-reparent-drops-work.sh @@ -0,0 +1,100 @@ +#!/usr/bin/env bash +# +# Reproduction: a push carrying more than one local revision into a branch head +# that has moved silently drops the other side's work. +# +# LORE=/path/to/lore LORE_SERVER=lore://127.0.0.1:41337 ./repro-push-reparent-drops-work.sh +# +# Needs a reachable loreserver with no auth and a lore CLI. Creates its own +# repository on that server and three working copies under a temp directory. +# +# What it does: +# A and B both clone the same head. +# A commits one file and pushes. -> the head moves +# B, still on the old head, commits TWO files and +# pushes once with --fast-forward-merge. -> both land +# A third clone then shows what the server really holds. +# +# Expected (correct): the third clone has A's file and both of B's. +# Observed (before the fix): A's file is gone, while A's commit is still in +# the history, so nothing looks wrong to anyone reading it. +# +# Why: in lore-revision/src/branch/push.rs the client bends a revision's +# parent pointer onto whatever the previous push returned, keeping its tree: +# +# if !current_latest.is_zero() && state.parent_self() != current_latest { +# state.set_parent_self(current_latest); +# } +# +# That is right when the server merely renumbered the revision we just sent +# (same content under a new signature). After a server-side fast-forward merge +# `current_latest` is a different revision carrying somebody else's work, and +# re-pointing at it makes B's stale tree look like a direct descendant. The +# server then takes the ordinary fast-forward path — no merge, no three-way +# diff, no conflict check — and the branch becomes B's tree. +# +# The guard `!current_latest.is_zero()` is why a single-revision push is safe: +# the block cannot fire for the first revision of a push, only from the second. + +set -euo pipefail + +LORE=${LORE:-lore} +: "${LORE_SERVER:?set LORE_SERVER, e.g. lore://127.0.0.1:41337}" + +WORK=$(mktemp -d) +trap 'rm -rf "$WORK"' EXIT +REPO="${LORE_SERVER%/}/reparent-repro-$$-$(date +%s)" + +lore_in() { local wc=$1; shift; "$LORE" --repository "$wc" --no-pager "$@" + local wc=$1 file=$2 content=$3 msg=$4 + printf '%s\n' "$content" > "$wc/$file" + lore_in "$wc" stage "$wc/$file" >/dev/null + lore_in "$wc" commit "$msg" >/dev/null +} + +echo "repository: $REPO" +echo + +# ── seed ──────────────────────────────────────────────────────────────────── +"$LORE" --no-pager repository create "$REPO" --repository "$WORK/seed" /dev/null +add_commit "$WORK/seed" keep.txt seed "seed" +lore_in "$WORK/seed" push >/dev/null + +# ── two clones off the same head ──────────────────────────────────────────── +"$LORE" --no-pager clone "$REPO" "$WORK/a" /dev/null +"$LORE" --no-pager clone "$REPO" "$WORK/b" /dev/null + +# ── A publishes, moving the head ──────────────────────────────────────────── +add_commit "$WORK/a" from-a.txt "A only" "A publishes" +lore_in "$WORK/a" push >/dev/null +echo "A pushed from-a.txt" + +# ── B, still on the old head, commits TWICE and pushes once ───────────────── +add_commit "$WORK/b" from-b1.txt "B one" "B one" +add_commit "$WORK/b" from-b2.txt "B two" "B two" +lore_in "$WORK/b" push --fast-forward-merge >/dev/null +echo "B pushed two revisions with --fast-forward-merge" +echo + +# ── what does the server actually hold? ───────────────────────────────────── +"$LORE" --no-pager clone "$REPO" "$WORK/c" /dev/null +echo "files in a fresh clone:" +find "$WORK/c" -maxdepth 1 -mindepth 1 -exec basename {} \; | sort | sed 's/^/ /' +echo +echo "history (A's commit is still in it either way):" +lore_in "$WORK/c" history 6 | grep -E '^(Revision|Signature| )' | sed 's/^/ /' +echo + +rc=0 +for f in from-b1.txt from-b2.txt; do + [ -f "$WORK/c/$f" ] || { echo "UNEXPECTED: B's own $f is missing too"; rc=1; } +done +if [ -f "$WORK/c/from-a.txt" ]; then + echo "RESULT: OK — A's file survived B's push" +else + echo "RESULT: LOST — A's file is gone from the branch, with no error reported" + rc=1 +fi +exit $rc diff --git a/lore-revision/src/branch/push.rs b/lore-revision/src/branch/push.rs index 4ed08d9f..0f838a0f 100644 --- a/lore-revision/src/branch/push.rs +++ b/lore-revision/src/branch/push.rs @@ -1021,8 +1021,23 @@ async fn collect_fragments_and_push( let mut current_latest = Hash::default(); let mut fast_forward_merged = false; - for current_revision in full_local_history.iter().rev() { - let mut current_revision = *current_revision; + // The revision pushed on the previous iteration, as (the signature it has + // in local history, the signature it ended up with on the server). Set + // only when the server accepted that revision as it stood and merely + // renumbered it, so the two names denote the same content and the same + // lineage. That is the one situation in which the next revision's parent + // pointer may be moved. + // + // Deliberately NOT `current_latest`: that also holds a revision the head + // moved to for entirely different reasons — a server-side fast-forward + // merge, which is a new revision carrying somebody else's work. Pointing + // the next revision at that one keeps this revision's tree while claiming + // to descend from theirs, so the server sees an ordinary fast-forward and + // the push silently drops everything the other side added. Nothing + // reports it: the push succeeds and their commit is still in the history. + let mut renumbered_previous: Option<(Hash, Hash)> = None; + for original_revision in full_local_history.iter().rev() { + let mut current_revision = *original_revision; let state = State::deserialize(repository.clone(), current_revision) .await @@ -1030,22 +1045,35 @@ async fn collect_fragments_and_push( push_revision_links(&repository, token, &options, &state, branch).await?; - if !current_latest.is_zero() && state.parent_self() != current_latest { - // Rebase on new latest revision - // TODO(mjansson): This only handles revision number rewrite for now, implement proper - // automatic rebase if the push resulted in a clean rebase - // ... - + if let Some((local, stored)) = renumbered_previous + && local != stored + && state.parent_self() == local + { + // The parent this revision descends from was stored under a + // different signature (the server rewrites the revision number + // and re-serializes). Same content, same lineage, new name — so + // follow it. Any other mismatch is left alone: pushing the + // revision as it stands lets the server three-way merge it + // against a base it holds, which is what keeps both sides. + // + // TODO(mjansson): this still only follows a revision the server + // renamed — a real automatic rebase, applying this revision's + // delta onto a head that moved for other reasons, is not + // implemented. The result today is one server merge revision + // per pushed revision rather than a linear history. Note that + // a rebase cannot be done on the server alone: the merge is + // what keeps the pushed revision reachable there, and the next + // revision in the same push needs it as its diff base. event::LoreEvent::BranchPushRevisionUpdateBegin( LoreBranchPushRevisionUpdateBeginEventData { revision: state.revision(), old_parent: state.parent_self(), - new_parent: current_latest, + new_parent: stored, }, ) .send(); - state.set_parent_self(current_latest); + state.set_parent_self(stored); current_revision = state .serialize(repository.clone(), token) .await @@ -1157,6 +1185,12 @@ async fn collect_fragments_and_push( remote_latest = response.revision; current_latest = response.revision; + // The head moved to a merge the server built, not to a + // renumbered copy of what we sent. The revision we pushed is + // on the server unchanged (as the merge's second parent), so + // the next revision's parent is already resolvable there and + // must keep pointing at it. + renumbered_previous = None; event::LoreEvent::BranchPushRevisionPushEnd( LoreBranchPushRevisionPushEndEventData { @@ -1201,8 +1235,13 @@ async fn collect_fragments_and_push( remote_latest = response.revision; current_latest = response.revision; + // The server took this revision as it stood — it only rewrote the + // revision number, which changes the signature. Record both names + // so the next revision can follow its parent to the stored one. + renumbered_previous = Some((*original_revision, response.revision)); } else { current_latest = current_revision; + renumbered_previous = Some((*original_revision, current_revision)); } let current_number = State::deserialize(repository.clone(), current_latest) From 720a792602e0568edad7969e8e8316d56402d1b4 Mon Sep 17 00:00:00 2001 From: Jochen Hunz Date: Tue, 15 Sep 2026 23:54:27 +0200 Subject: [PATCH 2/4] Let a push ask the server to rebase instead of merge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Integrating a push whose parent is no longer the branch head already applies the incoming revision's changes onto the head — a three-way diff against the revision's own parent, applied to the head's state. The only thing that makes the result a merge rather than a rebase is that it also records the pushed revision as `parent_other`. Add `rebase` to BranchPushRequest to ask for the other shape. The diff and the conflict handling are unchanged; the result keeps only the head as its parent, so the branch stays linear, and it is stamped `rebased-on-push` instead of `merged-by`/`fast-forward-merge` so a reader can tell the two apart rather than seeing a merge with no second parent. The two are mutually exclusive rather than ordered by precedence: they ask for different histories, and quietly applying one would leave the caller unable to tell which it got. Rejected in three places, because each has callers the others do not see — clap (`conflicts_with`), `branch::push` for embedders, and the v1 handler for anything on the wire. `rebase` deliberately does NOT imply `fast_forward_merge`. A server that predates the field ignores it, sees a push with no integration opt-in, and soft-rejects. A client that asked to rebase must never be answered with a merge it did not ask for — so the client also turns that refusal into a message naming the likely cause, since a server that understood the field would have rebased or reported conflicts instead. What a rebase gives up is stated in try_integrate_onto_head: merging keeps the pushed revision reachable as a second parent, which is also what lets the next revision of the same push use it as a diff base. Rebasing drops it, so it survives only as long as the store holds it — long enough for the rest of the push, not beyond. Verified with contrib/repro-push-reparent-drops-work.sh, which grew a MODE=rebase arm: new server, --rebase RESULT: OK, history is linear (revisions 1-4, no merge revision) new server, --fast-forward-merge RESULT: OK, merge shape unchanged 0.9.0 server, --rebase RESULT: REFUSED, A's file untouched both flags error: the argument '--rebase' cannot be used with '--fast-forward-merge' `cargo test -p lore-revision` green, fmt and clippy clean. Stacked on the parent-pointer fix: without it a multi-revision push arrives with its parents already bent onto the head, so revisions 2..N never reach this path at all and there would be nothing to rebase. Signed-off-by: Jochen Hunz --- contrib/repro-push-reparent-drops-work.sh | 31 +++++- lore-capi/lore.h | 3 + lore-client/src/cli/commands/branch.rs | 7 ++ .../proto/lore/revision/v1/revision.proto | 14 +++ lore-proto/src/grpc/lore.revision.v1.rs | 15 +++ lore-revision/src/branch/merge.rs | 4 +- lore-revision/src/branch/push.rs | 50 +++++++++- lore-revision/src/metadata.rs | 5 + lore-revision/src/revision/restore.rs | 2 +- lore-server/src/grpc/handlers/branch_push.rs | 99 +++++++++++++------ .../src/grpc/revision/v1/branch_push.rs | 13 +++ lore-transport/src/grpc/mod.rs | 3 +- lore-transport/src/grpc/revision_client.rs | 2 + lore-transport/src/traits.rs | 4 + lore/src/branch.rs | 4 + 15 files changed, 215 insertions(+), 41 deletions(-) diff --git a/contrib/repro-push-reparent-drops-work.sh b/contrib/repro-push-reparent-drops-work.sh index 1297498d..c14b3de6 100755 --- a/contrib/repro-push-reparent-drops-work.sh +++ b/contrib/repro-push-reparent-drops-work.sh @@ -40,6 +40,10 @@ set -euo pipefail LORE=${LORE:-lore} : "${LORE_SERVER:?set LORE_SERVER, e.g. lore://127.0.0.1:41337}" +# merge - integrate with --fast-forward-merge (the shape that ships today) +# rebase - integrate with --rebase (linear history) +MODE=${MODE:-merge} +case "$MODE" in merge|rebase) ;; *) echo "MODE must be merge or rebase"; exit 2 ;; esac WORK=$(mktemp -d) trap 'rm -rf "$WORK"' EXIT @@ -74,8 +78,22 @@ echo "A pushed from-a.txt" # ── B, still on the old head, commits TWICE and pushes once ───────────────── add_commit "$WORK/b" from-b1.txt "B one" "B one" add_commit "$WORK/b" from-b2.txt "B two" "B two" -lore_in "$WORK/b" push --fast-forward-merge >/dev/null -echo "B pushed two revisions with --fast-forward-merge" +if [ "$MODE" = rebase ]; then + if ! lore_in "$WORK/b" push --rebase; then + echo + echo "RESULT: REFUSED — the server did not integrate the push." + echo " On a server that predates --rebase this is the expected answer:" + echo " it ignores the unknown field, sees no integration opt-in and" + echo " soft-rejects, rather than silently merging instead." + "$LORE" --no-pager clone "$REPO" "$WORK/c" /dev/null + [ -f "$WORK/c/from-a.txt" ] && echo " A's file is untouched on the server." || echo " UNEXPECTED: A's file is gone anyway." + exit 3 + fi + echo "B pushed two revisions with --rebase" +else + lore_in "$WORK/b" push --fast-forward-merge >/dev/null + echo "B pushed two revisions with --fast-forward-merge" +fi echo # ── what does the server actually hold? ───────────────────────────────────── @@ -91,6 +109,15 @@ rc=0 for f in from-b1.txt from-b2.txt; do [ -f "$WORK/c/$f" ] || { echo "UNEXPECTED: B's own $f is missing too"; rc=1; } done +if [ "$MODE" = rebase ]; then + if lore_in "$WORK/c" history 6 | grep -q '^Merge '; then + echo "RESULT: NOT LINEAR — a rebase must not leave a merge revision" + rc=1 + else + echo "(history is linear: no merge revision)" + fi +fi + if [ -f "$WORK/c/from-a.txt" ]; then echo "RESULT: OK — A's file survived B's push" else diff --git a/lore-capi/lore.h b/lore-capi/lore.h index cee485c6..a33a60c4 100644 --- a/lore-capi/lore.h +++ b/lore-capi/lore.h @@ -4291,6 +4291,9 @@ typedef struct lore_branch_push_args_t { struct lore_string_t branch; // Allow the server to fast-forward merge if the target branch head has moved uint8_t fast_forward_merge; + // Allow the server to rebase onto the target branch head if it has moved, + // keeping the branch linear. Mutually exclusive with `fast_forward_merge`. + uint8_t rebase; } lore_branch_push_args_t; // Arguments for retrieving branch metadata (one key or all). diff --git a/lore-client/src/cli/commands/branch.rs b/lore-client/src/cli/commands/branch.rs index 267a95db..85456908 100644 --- a/lore-client/src/cli/commands/branch.rs +++ b/lore-client/src/cli/commands/branch.rs @@ -88,6 +88,12 @@ pub struct BranchPushArgs { /// Allow the server to fast-forward merge if the target branch head has moved #[clap(long)] fast_forward_merge: bool, + + /// Allow the server to rebase onto the target branch head if it has moved, + /// keeping the branch linear instead of recording a merge. A server that + /// does not support this refuses the push rather than merging instead. + #[clap(long, conflicts_with = "fast_forward_merge")] + rebase: bool, } #[derive(Args)] @@ -818,6 +824,7 @@ pub fn handle_branch_push(globals: LoreGlobalArgs, args: &BranchPushArgs) -> u8 let push_args = LoreBranchPushArgs { branch: args.name.clone().into(), fast_forward_merge: args.fast_forward_merge.into(), + rebase: args.rebase.into(), }; let debug = progress_debug(); diff --git a/lore-proto/proto/lore/revision/v1/revision.proto b/lore-proto/proto/lore/revision/v1/revision.proto index 689b6e78..58f7d7df 100644 --- a/lore-proto/proto/lore/revision/v1/revision.proto +++ b/lore-proto/proto/lore/revision/v1/revision.proto @@ -130,6 +130,20 @@ message BranchPushRequest { // the server attempts a fast-forward merge. Mutually superseded by // `force`. bool fast_forward_merge = 4; + // When true and the new tip does not descend from the current tip, + // the server rebases it: the same three-way diff `fast_forward_merge` + // performs, but the result records only the current tip as its parent, + // so the branch stays linear and the pushed revision is not retained + // as a second parent. Mutually superseded by `force`; setting it + // together with `fast_forward_merge` is INVALID_ARGUMENT, because the + // two ask for different histories and silently picking one would hide + // which was applied. + // + // A server that predates this field ignores it and sees a push with + // no integration opt-in, so it soft-rejects rather than merging. That + // is deliberate: a client asking to rebase must never be answered + // with a merge it did not ask for. + bool rebase = 5; } // Response describing the resulting branch tip after a push. diff --git a/lore-proto/src/grpc/lore.revision.v1.rs b/lore-proto/src/grpc/lore.revision.v1.rs index 9eafeb6f..72f7dbfc 100644 --- a/lore-proto/src/grpc/lore.revision.v1.rs +++ b/lore-proto/src/grpc/lore.revision.v1.rs @@ -204,6 +204,21 @@ pub struct BranchPushRequest { /// `force`. #[prost(bool, tag = "4")] pub fast_forward_merge: bool, + /// When true and the new tip does not descend from the current tip, + /// the server rebases it: the same three-way diff `fast_forward_merge` + /// performs, but the result records only the current tip as its parent, + /// so the branch stays linear and the pushed revision is not retained + /// as a second parent. Mutually superseded by `force`; setting it + /// together with `fast_forward_merge` is INVALID_ARGUMENT, because the + /// two ask for different histories and silently picking one would hide + /// which was applied. + /// + /// A server that predates this field ignores it and sees a push with + /// no integration opt-in, so it soft-rejects rather than merging. That + /// is deliberate: a client asking to rebase must never be answered + /// with a merge it did not ask for. + #[prost(bool, tag = "5")] + pub rebase: bool, } impl ::prost::Name for BranchPushRequest { const NAME: &'static str = "BranchPushRequest"; diff --git a/lore-revision/src/branch/merge.rs b/lore-revision/src/branch/merge.rs index 704822db..ee295fa6 100644 --- a/lore-revision/src/branch/merge.rs +++ b/lore-revision/src/branch/merge.rs @@ -4225,7 +4225,7 @@ async fn merge_into_link( .forward::("pushing fragments")?; let response = revision_protocol - .branch_push(target_branch, signature, false, false) + .branch_push(target_branch, signature, false, false, false) .await .forward::("pushing branch")?; @@ -4616,7 +4616,7 @@ pub async fn merge_into( .send(); let response = revision_protocol - .branch_push(branch, signature, false, false) + .branch_push(branch, signature, false, false, false) .await .forward::("pushing branch")?; diff --git a/lore-revision/src/branch/push.rs b/lore-revision/src/branch/push.rs index 0f838a0f..21cac440 100644 --- a/lore-revision/src/branch/push.rs +++ b/lore-revision/src/branch/push.rs @@ -288,6 +288,12 @@ pub struct PushOptions { pub branch: Option, /// Allow the server to fast-forward merge if the target branch head has moved pub fast_forward_merge: bool, + /// Allow the server to rebase onto the branch head if it has moved. Same + /// three-way diff as `fast_forward_merge`, but the result keeps only the + /// head as its parent, so the branch stays linear. Setting both is an + /// error: they ask for different histories, and quietly preferring one + /// would leave the caller unable to tell which it got. + pub rebase: bool, } impl EventError for PushError { @@ -480,6 +486,16 @@ pub async fn push( token: &RepositoryWriteToken, options: PushOptions, ) -> Result<(), PushError> { + // Rejected here rather than resolved by precedence: the two ask the server + // for different histories, and picking one silently would leave the caller + // unable to tell which it got. The CLI refuses the combination too, but + // embedders do not go through it. + if options.fast_forward_merge && options.rebase { + return Err(PushError::internal( + "fast_forward_merge and rebase are mutually exclusive", + )); + } + let _stats_report = PushStatsReport::start(); let branch; @@ -906,12 +922,13 @@ async fn collect_fragments_and_push( return Ok(()); } - // If the branch diverged, early out (unless fast-forward merge is enabled, - // in which case let the server attempt to resolve the divergence) + // If the branch diverged, early out (unless the caller asked the server to + // resolve the divergence — by merging or by rebasing) let force = execution_context().globals().force(); if !current_branch_remote_history.is_empty() && !force && !options.fast_forward_merge + && !options.rebase && !repository.is_link() { lore_debug!( @@ -1116,7 +1133,13 @@ async fn collect_fragments_and_push( if !dry_run && remote_latest != current_revision { let push_result = revision_protocol - .branch_push(branch, current_revision, force, options.fast_forward_merge) + .branch_push( + branch, + current_revision, + force, + options.fast_forward_merge, + options.rebase, + ) .await; // If the server returns NotFound, the branch was deleted on the server. @@ -1159,11 +1182,13 @@ async fn collect_fragments_and_push( current_revision, force, options.fast_forward_merge, + options.rebase, ) .await, + options.rebase, )? } - result => forward_branch_push(result)?, + result => forward_branch_push(result, options.rebase)?, }; if response.fast_forward_merged { // Server performed a fast-forward merge — push succeeded with a new revision. @@ -1302,7 +1327,10 @@ async fn collect_fragments_and_push( /// does not hold, and the address is the peer's answer rather than anything the attempt decides, /// so both report it the same way. #[track_caller] -fn forward_branch_push(result: Result) -> Result { +fn forward_branch_push( + result: Result, + rebase_requested: bool, +) -> Result { match result { Err(ProtocolError::AddressNotFound(missing)) => { let address = Address::from(&missing.address[..]); @@ -1310,6 +1338,18 @@ fn forward_branch_push(result: Result) -> Result Err(err).forward::(concat!( + "pushing branch to remote: the server did not perform the rebase. ", + "If it does not support rebasing on push, nothing was pushed and ", + "nothing was merged — synchronize, or allow a fast-forward merge ", + "to let the server merge instead", + )), result => result.forward::("pushing branch to remote"), } } diff --git a/lore-revision/src/metadata.rs b/lore-revision/src/metadata.rs index e0a11acd..55ae0ab7 100644 --- a/lore-revision/src/metadata.rs +++ b/lore-revision/src/metadata.rs @@ -127,6 +127,11 @@ pub const REVERTED_FROM: &str = "reverted-from"; pub const CHANGE_REQUEST: &str = "change-request"; /// Indicates the revision was created by a fast-forward merge ([`MetadataType::Numeric`]) pub const FAST_FORWARD_MERGE: &str = "fast-forward-merge"; +/// Indicates the revision was rebased onto the branch head when it was pushed +/// ([`MetadataType::Numeric`]). Set instead of [`FAST_FORWARD_MERGE`], never +/// alongside it: the two describe the same integration resolved into different +/// histories, and a reader has to be able to tell them apart. +pub const REBASED_ON_PUSH: &str = "rebased-on-push"; /// Keys describing the operation that creates a revision rather than the work /// it records, written by that operation. diff --git a/lore-revision/src/revision/restore.rs b/lore-revision/src/revision/restore.rs index 91d0600a..7ca00a01 100644 --- a/lore-revision/src/revision/restore.rs +++ b/lore-revision/src/revision/restore.rs @@ -563,7 +563,7 @@ pub async fn restore( .send(); let response = revision_protocol - .branch_push(current_branch, signature, false, false) + .branch_push(current_branch, signature, false, false, false) .await .forward::("pushing branch head pointer")?; diff --git a/lore-server/src/grpc/handlers/branch_push.rs b/lore-server/src/grpc/handlers/branch_push.rs index 0e3e9bf7..a09c7264 100644 --- a/lore-server/src/grpc/handlers/branch_push.rs +++ b/lore-server/src/grpc/handlers/branch_push.rs @@ -160,6 +160,9 @@ pub async fn handler( bypass_protection, force, fast_forward_merge, + // The deprecated request has no rebase field, so this path + // always integrates by merging. + false, history_step_size, acceleration, ) @@ -290,6 +293,7 @@ pub async fn push( bypass_protection: bool, force: bool, fast_forward_merge: bool, + rebase: bool, history_step_size: u64, acceleration: crate::grpc::server::RevisionListAcceleration, ) -> Result { @@ -365,7 +369,7 @@ pub async fn push( return Err(Status::not_found("Branch not found")); } - if !fast_forward_merge { + if !fast_forward_merge && !rebase { return Ok(PushResult { success: false, fast_forward_merged: false, @@ -374,14 +378,17 @@ pub async fn push( }); } - // Fast-forward merge: the incoming revision's parent_self no longer matches - // the branch head. Attempt to create a new merge revision with - // parent_self=current_head and parent_other=incoming_revision. - return try_fast_forward_merge( + // The incoming revision's parent_self no longer matches the branch + // head. Integrate it with a three-way diff against its own parent: + // with parent_self=current_head, and parent_other=incoming_revision + // unless the caller asked for a rebase, which keeps the branch + // linear by recording only the head as the parent. + return try_integrate_onto_head( repository.clone(), branch, state.clone(), current_head, + rebase, history_step_size, acceleration, ) @@ -463,24 +470,32 @@ pub async fn push( }) } -/// Attempts a server-side fast-forward merge when the target branch head has moved -/// since the client created the merge revision. +/// Integrates a push whose parent is no longer the branch head, by applying the +/// incoming revision's changes onto the head. +/// +/// Uses a three-way diff between the incoming revision's own parent, the +/// incoming revision, and the current head, and applies the non-conflicting +/// changes to the head's state. If conflicts are detected, returns failure so +/// the client can resolve locally. /// -/// Creates a new merge revision with: -/// - `parent_self` = current branch head (target branch) -/// - `parent_other` = the incoming merge revision +/// The result records: +/// - `parent_self` = current branch head (target branch), always +/// - `parent_other` = the incoming revision, unless `rebase` /// -/// Uses a three-way diff between the original merge base, the incoming revision, -/// and the current head. If conflicts are detected, returns failure so the client -/// can resolve locally. +/// That single difference is what separates the two shapes. Merging keeps the +/// pushed revision reachable as a second parent, which is also what lets the +/// next revision of the same push use it as a diff base. Rebasing keeps the +/// branch linear and drops it, so the pushed revision survives only as long as +/// the store holds it — long enough for the rest of this push, not beyond. /// /// Retries via CAS loop if the branch head moves again during processing. #[instrument(level = "debug", skip_all)] -async fn try_fast_forward_merge( +async fn try_integrate_onto_head( repository: Arc, branch: BranchId, incoming_state: Arc, mut current_head: Hash, + rebase: bool, history_step_size: u64, acceleration: crate::grpc::server::RevisionListAcceleration, ) -> Result { @@ -577,9 +592,13 @@ async fn try_fast_forward_merge( )) })?; - // Set parents: self=current head (target branch), other=incoming merge revision + // Set parents: self=current head (target branch), other=incoming + // revision — the latter only when merging. A rebase records no second + // parent, which is what keeps the branch linear. state_current.set_parent_self(current_head); - state_current.set_parent_other(incoming_revision); + if !rebase { + state_current.set_parent_other(incoming_revision); + } // Compute revision number from both parents let parent_state = State::deserialize(repository.clone(), current_head) @@ -589,13 +608,23 @@ async fn try_fast_forward_merge( Status::internal(format!("Failed to load current head state: {err}")) })?; + // A rebased revision has one parent, so its number follows the head + // alone — the same arithmetic the ordinary push path uses when there + // is no second parent. let revision_number = next_revision_number( parent_state.revision_number(), - incoming_state.revision_number(), + if rebase { + 0 + } else { + incoming_state.revision_number() + }, ); state_current.set_revision_number(revision_number); - // Copy metadata from the incoming revision and set merged-by to "server" + // Copy metadata from the incoming revision. A merge is stamped as + // merged-by/fast-forward-merge; a rebase carries no merger — nothing + // was merged — and is marked as rebased instead, so a reader can tell + // the two apart rather than seeing a merge that has no second parent. let incoming_metadata_hash = incoming_state.metadata_hash(); if !incoming_metadata_hash.is_zero() { let mut metadata = lore_revision::metadata::Metadata::deserialize( @@ -611,20 +640,30 @@ async fn try_fast_forward_merge( metadata .set_branch(branch) .warn_map_err(|_| Status::internal("Failed to set branch in metadata"))?; - // Preserve the existing merged-by field if set, otherwise fall back to "server" - if metadata - .get_string(lore_revision::metadata::MERGED_BY) - .is_err() - { + if rebase { metadata - .set_string(lore_revision::metadata::MERGED_BY, "server") - .warn_map_err(|_| Status::internal("Failed to set merged-by in metadata"))?; + .set_u64(lore_revision::metadata::REBASED_ON_PUSH, 1) + .warn_map_err(|_| { + Status::internal("Failed to set rebased-on-push in metadata") + })?; + } else { + // Preserve the existing merged-by field if set, otherwise fall back to "server" + if metadata + .get_string(lore_revision::metadata::MERGED_BY) + .is_err() + { + metadata + .set_string(lore_revision::metadata::MERGED_BY, "server") + .warn_map_err(|_| { + Status::internal("Failed to set merged-by in metadata") + })?; + } + metadata + .set_u64(lore_revision::metadata::FAST_FORWARD_MERGE, 1) + .warn_map_err(|_| { + Status::internal("Failed to set fast-forward-merge in metadata") + })?; } - metadata - .set_u64(lore_revision::metadata::FAST_FORWARD_MERGE, 1) - .warn_map_err(|_| { - Status::internal("Failed to set fast-forward-merge in metadata") - })?; let metadata_hash = metadata .serialize(repository.clone()) diff --git a/lore-server/src/grpc/revision/v1/branch_push.rs b/lore-server/src/grpc/revision/v1/branch_push.rs index bb89718b..2f4496e4 100644 --- a/lore-server/src/grpc/revision/v1/branch_push.rs +++ b/lore-server/src/grpc/revision/v1/branch_push.rs @@ -88,6 +88,7 @@ pub async fn handler( let revision = Hash::from(req.revision_signature); let force = req.force; let fast_forward_merge = req.fast_forward_merge; + let rebase = req.rebase; if revision.is_zero() { info!("Invalid branch push request, revision_signature is zero"); @@ -96,12 +97,23 @@ pub async fn handler( )); } + // The two ask for different histories. Integrating one way anyway would + // leave the client unable to tell which it got, so refuse the request + // rather than resolve it by precedence. + if fast_forward_merge && rebase { + info!("Invalid branch push request, fast_forward_merge and rebase both set"); + return Err(Status::invalid_argument( + "fast_forward_merge and rebase are mutually exclusive", + )); + } + debug!( {REVISION} = %revision, bypass_protection, {BRANCH_ID} = %branch_id, force, fast_forward_merge, + rebase, "Handling branch push request", ); @@ -147,6 +159,7 @@ pub async fn handler( bypass_protection, force, fast_forward_merge, + rebase, history_step_size, acceleration, ) diff --git a/lore-transport/src/grpc/mod.rs b/lore-transport/src/grpc/mod.rs index 566971d2..8a50bebd 100644 --- a/lore-transport/src/grpc/mod.rs +++ b/lore-transport/src/grpc/mod.rs @@ -1444,6 +1444,7 @@ impl Revision for GRPCRevision { latest: Hash, force: bool, fast_forward_merge: bool, + rebase: bool, ) -> Result { with_reconnect( &self.connection, @@ -1451,7 +1452,7 @@ impl Revision for GRPCRevision { self.client .read() .await - .branch_push(branch, latest, force, fast_forward_merge) + .branch_push(branch, latest, force, fast_forward_merge, rebase) .await }, |reconnect_id| self.reconnect(reconnect_id), diff --git a/lore-transport/src/grpc/revision_client.rs b/lore-transport/src/grpc/revision_client.rs index 786e957c..5a1b9e78 100644 --- a/lore-transport/src/grpc/revision_client.rs +++ b/lore-transport/src/grpc/revision_client.rs @@ -227,6 +227,7 @@ impl RevisionService { revision: Hash, force: bool, fast_forward_merge: bool, + rebase: bool, ) -> Result { lore_debug!("Pushing branch: {} at {}", branch, revision); let _ = RequestScopedCounter::new(self.request_inflight.clone()); @@ -238,6 +239,7 @@ impl RevisionService { revision_signature: revision.into(), force, fast_forward_merge, + rebase, }; let mut client = self.client.clone(); diff --git a/lore-transport/src/traits.rs b/lore-transport/src/traits.rs index 37cbd504..1d5c9448 100644 --- a/lore-transport/src/traits.rs +++ b/lore-transport/src/traits.rs @@ -274,12 +274,16 @@ pub trait Revision: Send + Sync { /// Push a new LATEST pointer for branch. Returns the (new) current LATEST pointer for the branch, /// if this is different from the given LATEST pointer the operation failed due to the /// LATEST pointer having moved. + /// `rebase` integrates by rebasing onto the head instead of merging onto + /// it, and is mutually exclusive with `fast_forward_merge` — callers are + /// expected to have rejected the combination before reaching here. async fn branch_push( &self, branch: BranchId, latest: Hash, force: bool, fast_forward_merge: bool, + rebase: bool, ) -> Result; /// List all branches diff --git a/lore/src/branch.rs b/lore/src/branch.rs index a9a407d6..ca3f51de 100644 --- a/lore/src/branch.rs +++ b/lore/src/branch.rs @@ -860,6 +860,9 @@ pub struct LoreBranchPushArgs { pub branch: LoreString, /// Allow the server to fast-forward merge if the target branch head has moved pub fast_forward_merge: u8, + /// Allow the server to rebase onto the target branch head if it has moved, + /// keeping the branch linear. Mutually exclusive with `fast_forward_merge`. + pub rebase: u8, } /// Pushes the current or specified branch and its revisions to the remote. @@ -929,6 +932,7 @@ async fn push_impl( let options = PushOptions { branch: args.branch.into(), fast_forward_merge: args.fast_forward_merge != 0, + rebase: args.rebase != 0, }; // Push is never local From 9c9e596cf3ac86e1253f5fb98b2863eb975a3e2e Mon Sep 17 00:00:00 2001 From: Jochen Hunz Date: Wed, 16 Sep 2026 08:42:51 +0200 Subject: [PATCH 3/4] Update every push() call site and the proto round-trip guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The server's push() gained the rebase parameter and BranchPushRequest gained the field, which the workspace's test targets pick up: 43 test call sites of branch_push::push pass it as false (unchanged merge behaviour), and lore-proto's exhaustive-destructuring guard names it. That guard is doing its job — a new field on a v1 message should not land without someone looking at it. Also adds the test that belongs with the validation: a request setting both fast_forward_merge and rebase is refused with InvalidArgument. Found by CI: I had only built the binaries locally, never cargo check --workspace --all-targets, so none of this showed up. Signed-off-by: Jochen Hunz --- lore-proto/tests/v1_revision.rs | 1 + lore-server/src/cache/revision.rs | 2 + .../forwarded_revision/v1/branch_delete.rs | 1 + .../grpc/forwarded_revision/v1/branch_get.rs | 1 + .../src/grpc/handlers/branch_delete.rs | 1 + lore-server/src/grpc/handlers/branch_diff.rs | 1 + lore-server/src/grpc/handlers/branch_push.rs | 7 +++ .../src/grpc/handlers/branch_revision_list.rs | 7 +++ .../src/grpc/handlers/revision_describe.rs | 4 ++ .../src/grpc/handlers/revision_tree.rs | 4 ++ .../src/grpc/revision/v1/branch_delete.rs | 1 + .../src/grpc/revision/v1/branch_get.rs | 1 + .../src/grpc/revision/v1/branch_list.rs | 1 + .../grpc/revision/v1/branch_metadata_get.rs | 1 + .../grpc/revision/v1/branch_metadata_set.rs | 1 + .../src/grpc/revision/v1/branch_push.rs | 51 +++++++++++++++++++ .../src/grpc/revision/v1/revision_list.rs | 2 + .../src/grpc/thinclient/v1/revision_diff.rs | 1 + .../src/grpc/thinclient/v1/revision_info.rs | 3 ++ .../src/grpc/thinclient/v1/revision_tree.rs | 4 ++ 20 files changed, 95 insertions(+) diff --git a/lore-proto/tests/v1_revision.rs b/lore-proto/tests/v1_revision.rs index e195eab3..7dc08961 100644 --- a/lore-proto/tests/v1_revision.rs +++ b/lore-proto/tests/v1_revision.rs @@ -76,6 +76,7 @@ fn v1_revision_field_shapes() { revision_signature: _, force: _, fast_forward_merge: _, + rebase: _, } = BranchPushRequest::default(); let BranchPushResponse { revision_signature: _, diff --git a/lore-server/src/cache/revision.rs b/lore-server/src/cache/revision.rs index 97bd0590..7fa778e9 100644 --- a/lore-server/src/cache/revision.rs +++ b/lore-server/src/cache/revision.rs @@ -1353,6 +1353,7 @@ mod tests { true, true, false, + false, STEP_ONE_HUNDRED, BOTH, ) @@ -1386,6 +1387,7 @@ mod tests { true, true, false, + false, STEP_ONE_HUNDRED, BOTH, ) diff --git a/lore-server/src/grpc/forwarded_revision/v1/branch_delete.rs b/lore-server/src/grpc/forwarded_revision/v1/branch_delete.rs index 78404512..aec39f74 100644 --- a/lore-server/src/grpc/forwarded_revision/v1/branch_delete.rs +++ b/lore-server/src/grpc/forwarded_revision/v1/branch_delete.rs @@ -112,6 +112,7 @@ mod test { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/forwarded_revision/v1/branch_get.rs b/lore-server/src/grpc/forwarded_revision/v1/branch_get.rs index 300fc744..8ceae2f8 100644 --- a/lore-server/src/grpc/forwarded_revision/v1/branch_get.rs +++ b/lore-server/src/grpc/forwarded_revision/v1/branch_get.rs @@ -89,6 +89,7 @@ mod test { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/handlers/branch_delete.rs b/lore-server/src/grpc/handlers/branch_delete.rs index cc9aeca9..8c055023 100644 --- a/lore-server/src/grpc/handlers/branch_delete.rs +++ b/lore-server/src/grpc/handlers/branch_delete.rs @@ -171,6 +171,7 @@ mod test { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/handlers/branch_diff.rs b/lore-server/src/grpc/handlers/branch_diff.rs index a518bf06..6190100a 100644 --- a/lore-server/src/grpc/handlers/branch_diff.rs +++ b/lore-server/src/grpc/handlers/branch_diff.rs @@ -269,6 +269,7 @@ mod test { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/handlers/branch_push.rs b/lore-server/src/grpc/handlers/branch_push.rs index a09c7264..262bf19c 100644 --- a/lore-server/src/grpc/handlers/branch_push.rs +++ b/lore-server/src/grpc/handlers/branch_push.rs @@ -1105,6 +1105,7 @@ mod tests { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, RevisionListAcceleration::default(), ) @@ -1149,6 +1150,7 @@ mod tests { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, RevisionListAcceleration::default(), ) @@ -1305,6 +1307,7 @@ mod tests { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, RevisionListAcceleration::default(), ) @@ -1356,6 +1359,7 @@ mod tests { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, RevisionListAcceleration::default(), ) @@ -1419,6 +1423,7 @@ mod tests { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, RevisionListAcceleration::default(), ) @@ -1483,6 +1488,7 @@ mod tests { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, RevisionListAcceleration::default(), ) @@ -1555,6 +1561,7 @@ mod tests { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/handlers/branch_revision_list.rs b/lore-server/src/grpc/handlers/branch_revision_list.rs index 29cce27f..710c4934 100644 --- a/lore-server/src/grpc/handlers/branch_revision_list.rs +++ b/lore-server/src/grpc/handlers/branch_revision_list.rs @@ -152,6 +152,7 @@ mod tests { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -174,6 +175,7 @@ mod tests { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -196,6 +198,7 @@ mod tests { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -218,6 +221,7 @@ mod tests { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -455,6 +459,7 @@ mod tests { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -476,6 +481,7 @@ mod tests { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -497,6 +503,7 @@ mod tests { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/handlers/revision_describe.rs b/lore-server/src/grpc/handlers/revision_describe.rs index 96e215b8..49a4220c 100644 --- a/lore-server/src/grpc/handlers/revision_describe.rs +++ b/lore-server/src/grpc/handlers/revision_describe.rs @@ -152,6 +152,7 @@ mod tests { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -174,6 +175,7 @@ mod tests { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -196,6 +198,7 @@ mod tests { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -267,6 +270,7 @@ mod tests { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/handlers/revision_tree.rs b/lore-server/src/grpc/handlers/revision_tree.rs index 9d553d8e..3dccd8a4 100644 --- a/lore-server/src/grpc/handlers/revision_tree.rs +++ b/lore-server/src/grpc/handlers/revision_tree.rs @@ -175,6 +175,7 @@ mod tests { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -249,6 +250,7 @@ mod tests { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -340,6 +342,7 @@ mod tests { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -456,6 +459,7 @@ mod tests { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/revision/v1/branch_delete.rs b/lore-server/src/grpc/revision/v1/branch_delete.rs index 28a278cd..c3b81b42 100644 --- a/lore-server/src/grpc/revision/v1/branch_delete.rs +++ b/lore-server/src/grpc/revision/v1/branch_delete.rs @@ -268,6 +268,7 @@ mod test { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/revision/v1/branch_get.rs b/lore-server/src/grpc/revision/v1/branch_get.rs index 3ee342c6..38bc85e1 100644 --- a/lore-server/src/grpc/revision/v1/branch_get.rs +++ b/lore-server/src/grpc/revision/v1/branch_get.rs @@ -230,6 +230,7 @@ mod test { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/revision/v1/branch_list.rs b/lore-server/src/grpc/revision/v1/branch_list.rs index 02d0dca0..31f9c0c0 100644 --- a/lore-server/src/grpc/revision/v1/branch_list.rs +++ b/lore-server/src/grpc/revision/v1/branch_list.rs @@ -375,6 +375,7 @@ pub mod test { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/revision/v1/branch_metadata_get.rs b/lore-server/src/grpc/revision/v1/branch_metadata_get.rs index 854c93b8..2536880e 100644 --- a/lore-server/src/grpc/revision/v1/branch_metadata_get.rs +++ b/lore-server/src/grpc/revision/v1/branch_metadata_get.rs @@ -202,6 +202,7 @@ mod test { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/revision/v1/branch_metadata_set.rs b/lore-server/src/grpc/revision/v1/branch_metadata_set.rs index 1f60bbad..67e58362 100644 --- a/lore-server/src/grpc/revision/v1/branch_metadata_set.rs +++ b/lore-server/src/grpc/revision/v1/branch_metadata_set.rs @@ -521,6 +521,7 @@ mod test { true, true, false, + false, lore_revision::branch::DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/revision/v1/branch_push.rs b/lore-server/src/grpc/revision/v1/branch_push.rs index 2f4496e4..56df35e6 100644 --- a/lore-server/src/grpc/revision/v1/branch_push.rs +++ b/lore-server/src/grpc/revision/v1/branch_push.rs @@ -433,12 +433,24 @@ mod test { revision: Hash, force: bool, fast_forward_merge: bool, + ) -> Request { + make_request_with(repository, branch, revision, force, fast_forward_merge, false) + } + + fn make_request_with( + repository: RepositoryId, + branch: BranchId, + revision: Hash, + force: bool, + fast_forward_merge: bool, + rebase: bool, ) -> Request { let mut request = Request::new(BranchPushRequest { id: branch.into(), revision_signature: revision.into(), force, fast_forward_merge, + rebase, }); request.metadata_mut().insert_bin( REPOSITORY_ID_KEY, @@ -507,6 +519,45 @@ mod test { .await; } + /// The two ask for different histories, so the request is refused rather + /// than resolved by precedence — a client must never be left guessing + /// which of the two it got. + #[tokio::test] + async fn push_with_both_merge_and_rebase_returns_invalid_argument() { + let repository = random::(); + let (immutable_store, mutable_store, execution) = + test_store_create().await.expect("Failed to create stores"); + + let notification_sender = Arc::new(MockNotificationSender::new()); + let instrument_provider = TestInstrumentProvider {}; + + Box::pin(LORE_CONTEXT.scope(execution.clone(), async move { + let repository_context = Arc::new(RepositoryContext::new_server_context( + immutable_store.clone(), + mutable_store.clone(), + repository, + )); + let main = create_root_branch(&repository_context, "main").await; + let revision = build_revision(&repository_context, Hash::default(), 1).await; + + let hook_dispatcher = HookDispatcher::empty(); + let err = handler( + make_request_with(repository, main, revision, false, true, true), + immutable_store.clone(), + mutable_store.clone(), + notification_sender.clone(), + &hook_dispatcher, + DEFAULT_HISTORY_STEP_SIZE, + crate::grpc::server::RevisionListAcceleration::default(), + &instrument_provider, + ) + .await + .expect_err("asking for a merge and a rebase at once should fail"); + assert_eq!(err.code(), tonic::Code::InvalidArgument); + })) + .await; + } + #[tokio::test] async fn push_zero_revision_returns_invalid_argument() { let repository = random::(); diff --git a/lore-server/src/grpc/revision/v1/revision_list.rs b/lore-server/src/grpc/revision/v1/revision_list.rs index 015c7e81..5f8ff17d 100644 --- a/lore-server/src/grpc/revision/v1/revision_list.rs +++ b/lore-server/src/grpc/revision/v1/revision_list.rs @@ -1167,6 +1167,7 @@ mod test { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -1240,6 +1241,7 @@ mod test { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/thinclient/v1/revision_diff.rs b/lore-server/src/grpc/thinclient/v1/revision_diff.rs index a447505f..f071d0c8 100644 --- a/lore-server/src/grpc/thinclient/v1/revision_diff.rs +++ b/lore-server/src/grpc/thinclient/v1/revision_diff.rs @@ -860,6 +860,7 @@ mod test { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/thinclient/v1/revision_info.rs b/lore-server/src/grpc/thinclient/v1/revision_info.rs index 16380a94..d238de85 100644 --- a/lore-server/src/grpc/thinclient/v1/revision_info.rs +++ b/lore-server/src/grpc/thinclient/v1/revision_info.rs @@ -361,6 +361,7 @@ mod test { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -552,6 +553,7 @@ mod test { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -645,6 +647,7 @@ mod test { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/thinclient/v1/revision_tree.rs b/lore-server/src/grpc/thinclient/v1/revision_tree.rs index e34c91df..8a6ebdc9 100644 --- a/lore-server/src/grpc/thinclient/v1/revision_tree.rs +++ b/lore-server/src/grpc/thinclient/v1/revision_tree.rs @@ -291,6 +291,7 @@ mod test { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -379,6 +380,7 @@ mod test { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -482,6 +484,7 @@ mod test { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -1491,6 +1494,7 @@ mod test { true, true, false, + false, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) From 8f99d78a4fbfc7e92b835bcdf18a90ace7d871ab Mon Sep 17 00:00:00 2001 From: Jochen Hunz Date: Wed, 16 Sep 2026 09:10:32 +0200 Subject: [PATCH 4/4] Take one Integration decision rather than two exclusive bools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit clippy's fn_params_excessive_bools fired on push() once rebase made it a fourth bool, and it is right to: fast_forward_merge and rebase are mutually exclusive, so they were never two independent choices. Replace the pair with an Integration enum — Refuse, Merge, Rebase — which makes the invalid combination unrepresentable past the wire boundary instead of something a runtime guard has to catch. The v1 handler now maps the two request bits onto it and is the single place that rejects both-set; the guard inside push() is gone because the type no longer allows the state. The deprecated handler maps its one bit to Merge or Refuse. Also fixes the nightly rustfmt diff CI reported: the hook runs cargo +nightly fmt, and the repo's rustfmt.toml uses nightly-only options, so a stable-toolchain format is not the same check. Signed-off-by: Jochen Hunz --- lore-server/src/cache/revision.rs | 6 +- .../forwarded_revision/v1/branch_delete.rs | 3 +- .../grpc/forwarded_revision/v1/branch_get.rs | 3 +- .../src/grpc/handlers/branch_delete.rs | 3 +- lore-server/src/grpc/handlers/branch_diff.rs | 3 +- lore-server/src/grpc/handlers/branch_push.rs | 57 ++++++++++++------- .../src/grpc/handlers/branch_revision_list.rs | 21 +++---- .../src/grpc/handlers/revision_describe.rs | 12 ++-- .../src/grpc/handlers/revision_tree.rs | 12 ++-- .../src/grpc/revision/v1/branch_delete.rs | 3 +- .../src/grpc/revision/v1/branch_get.rs | 3 +- .../src/grpc/revision/v1/branch_list.rs | 3 +- .../grpc/revision/v1/branch_metadata_get.rs | 3 +- .../grpc/revision/v1/branch_metadata_set.rs | 3 +- .../src/grpc/revision/v1/branch_push.rs | 38 +++++++++---- .../src/grpc/revision/v1/revision_list.rs | 6 +- .../src/grpc/thinclient/v1/revision_diff.rs | 3 +- .../src/grpc/thinclient/v1/revision_info.rs | 9 +-- .../src/grpc/thinclient/v1/revision_tree.rs | 12 ++-- 19 files changed, 98 insertions(+), 105 deletions(-) diff --git a/lore-server/src/cache/revision.rs b/lore-server/src/cache/revision.rs index 7fa778e9..5ca4ea36 100644 --- a/lore-server/src/cache/revision.rs +++ b/lore-server/src/cache/revision.rs @@ -1352,8 +1352,7 @@ mod tests { serialized, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, STEP_ONE_HUNDRED, BOTH, ) @@ -1386,8 +1385,7 @@ mod tests { serialized, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, STEP_ONE_HUNDRED, BOTH, ) diff --git a/lore-server/src/grpc/forwarded_revision/v1/branch_delete.rs b/lore-server/src/grpc/forwarded_revision/v1/branch_delete.rs index aec39f74..4dfaa3ee 100644 --- a/lore-server/src/grpc/forwarded_revision/v1/branch_delete.rs +++ b/lore-server/src/grpc/forwarded_revision/v1/branch_delete.rs @@ -111,8 +111,7 @@ mod test { state_hash, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/forwarded_revision/v1/branch_get.rs b/lore-server/src/grpc/forwarded_revision/v1/branch_get.rs index 8ceae2f8..fc9d76ac 100644 --- a/lore-server/src/grpc/forwarded_revision/v1/branch_get.rs +++ b/lore-server/src/grpc/forwarded_revision/v1/branch_get.rs @@ -88,8 +88,7 @@ mod test { state_hash, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/handlers/branch_delete.rs b/lore-server/src/grpc/handlers/branch_delete.rs index 8c055023..4e7aa0f2 100644 --- a/lore-server/src/grpc/handlers/branch_delete.rs +++ b/lore-server/src/grpc/handlers/branch_delete.rs @@ -170,8 +170,7 @@ mod test { state_hash, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/handlers/branch_diff.rs b/lore-server/src/grpc/handlers/branch_diff.rs index 6190100a..ec2dc0cc 100644 --- a/lore-server/src/grpc/handlers/branch_diff.rs +++ b/lore-server/src/grpc/handlers/branch_diff.rs @@ -268,8 +268,7 @@ mod test { state_hash, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/handlers/branch_push.rs b/lore-server/src/grpc/handlers/branch_push.rs index 262bf19c..fe2d936c 100644 --- a/lore-server/src/grpc/handlers/branch_push.rs +++ b/lore-server/src/grpc/handlers/branch_push.rs @@ -159,10 +159,13 @@ pub async fn handler( revision, bypass_protection, force, - fast_forward_merge, // The deprecated request has no rebase field, so this path - // always integrates by merging. - false, + // can only ever merge. + if fast_forward_merge { + Integration::Merge + } else { + Integration::Refuse + }, history_step_size, acceleration, ) @@ -284,6 +287,26 @@ pub struct PushResult { pub revision_number: u64, } +/// What the server may do with a push whose parent is no longer the branch +/// head. +/// +/// One value rather than a pair of flags because the two ways of integrating +/// are mutually exclusive — they produce different histories, and a request +/// asking for both is refused at the wire boundary. Expressing that here makes +/// the invalid combination unrepresentable rather than something every caller +/// has to be trusted not to construct. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Integration { + /// Refuse the push and let the client resolve the divergence itself. + Refuse, + /// Merge onto the head, recording the pushed revision as the second + /// parent, which keeps it reachable on the branch. + Merge, + /// Rebase onto the head, recording only the head as the parent, which + /// keeps the branch linear. + Rebase, +} + #[allow(clippy::too_many_arguments)] #[instrument(level = "debug", skip_all, fields(branch))] pub async fn push( @@ -292,8 +315,7 @@ pub async fn push( latest: Hash, bypass_protection: bool, force: bool, - fast_forward_merge: bool, - rebase: bool, + integration: Integration, history_step_size: u64, acceleration: crate::grpc::server::RevisionListAcceleration, ) -> Result { @@ -369,7 +391,7 @@ pub async fn push( return Err(Status::not_found("Branch not found")); } - if !fast_forward_merge && !rebase { + if integration == Integration::Refuse { return Ok(PushResult { success: false, fast_forward_merged: false, @@ -388,7 +410,7 @@ pub async fn push( branch, state.clone(), current_head, - rebase, + integration == Integration::Rebase, history_step_size, acceleration, ) @@ -1104,8 +1126,7 @@ mod tests { state.revision(), true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, RevisionListAcceleration::default(), ) @@ -1149,8 +1170,7 @@ mod tests { state.revision(), true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, RevisionListAcceleration::default(), ) @@ -1306,8 +1326,7 @@ mod tests { nonexistent_revision, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, RevisionListAcceleration::default(), ) @@ -1358,8 +1377,7 @@ mod tests { state.revision(), true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, RevisionListAcceleration::default(), ) @@ -1422,8 +1440,7 @@ mod tests { state.revision(), true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, RevisionListAcceleration::default(), ) @@ -1487,8 +1504,7 @@ mod tests { merge.revision(), true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, RevisionListAcceleration::default(), ) @@ -1560,8 +1576,7 @@ mod tests { merge.revision(), true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/handlers/branch_revision_list.rs b/lore-server/src/grpc/handlers/branch_revision_list.rs index 710c4934..4bb4025b 100644 --- a/lore-server/src/grpc/handlers/branch_revision_list.rs +++ b/lore-server/src/grpc/handlers/branch_revision_list.rs @@ -151,8 +151,7 @@ mod tests { first_hash, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -174,8 +173,7 @@ mod tests { second_hash, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -197,8 +195,7 @@ mod tests { third_hash, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -220,8 +217,7 @@ mod tests { fourth_hash, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -458,8 +454,7 @@ mod tests { first_hash, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -480,8 +475,7 @@ mod tests { second_hash, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -502,8 +496,7 @@ mod tests { third_hash, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/handlers/revision_describe.rs b/lore-server/src/grpc/handlers/revision_describe.rs index 49a4220c..c0223ead 100644 --- a/lore-server/src/grpc/handlers/revision_describe.rs +++ b/lore-server/src/grpc/handlers/revision_describe.rs @@ -151,8 +151,7 @@ mod tests { first_hash, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -174,8 +173,7 @@ mod tests { second_hash, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -197,8 +195,7 @@ mod tests { third_hash, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -269,8 +266,7 @@ mod tests { merge_hash, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/handlers/revision_tree.rs b/lore-server/src/grpc/handlers/revision_tree.rs index 3dccd8a4..33f9cc96 100644 --- a/lore-server/src/grpc/handlers/revision_tree.rs +++ b/lore-server/src/grpc/handlers/revision_tree.rs @@ -174,8 +174,7 @@ mod tests { revision_hash, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -249,8 +248,7 @@ mod tests { revision_hash, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -341,8 +339,7 @@ mod tests { revision_hash, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -458,8 +455,7 @@ mod tests { revision_hash, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/revision/v1/branch_delete.rs b/lore-server/src/grpc/revision/v1/branch_delete.rs index c3b81b42..444d582d 100644 --- a/lore-server/src/grpc/revision/v1/branch_delete.rs +++ b/lore-server/src/grpc/revision/v1/branch_delete.rs @@ -267,8 +267,7 @@ mod test { state_hash, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/revision/v1/branch_get.rs b/lore-server/src/grpc/revision/v1/branch_get.rs index 38bc85e1..b9832734 100644 --- a/lore-server/src/grpc/revision/v1/branch_get.rs +++ b/lore-server/src/grpc/revision/v1/branch_get.rs @@ -229,8 +229,7 @@ mod test { state_hash, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/revision/v1/branch_list.rs b/lore-server/src/grpc/revision/v1/branch_list.rs index 31f9c0c0..08ef1c28 100644 --- a/lore-server/src/grpc/revision/v1/branch_list.rs +++ b/lore-server/src/grpc/revision/v1/branch_list.rs @@ -374,8 +374,7 @@ pub mod test { state_hash, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/revision/v1/branch_metadata_get.rs b/lore-server/src/grpc/revision/v1/branch_metadata_get.rs index 2536880e..81f80bff 100644 --- a/lore-server/src/grpc/revision/v1/branch_metadata_get.rs +++ b/lore-server/src/grpc/revision/v1/branch_metadata_get.rs @@ -201,8 +201,7 @@ mod test { state_hash, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/revision/v1/branch_metadata_set.rs b/lore-server/src/grpc/revision/v1/branch_metadata_set.rs index 67e58362..e0c6be4a 100644 --- a/lore-server/src/grpc/revision/v1/branch_metadata_set.rs +++ b/lore-server/src/grpc/revision/v1/branch_metadata_set.rs @@ -520,8 +520,7 @@ mod test { state_hash, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, lore_revision::branch::DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/revision/v1/branch_push.rs b/lore-server/src/grpc/revision/v1/branch_push.rs index 56df35e6..79d30dd1 100644 --- a/lore-server/src/grpc/revision/v1/branch_push.rs +++ b/lore-server/src/grpc/revision/v1/branch_push.rs @@ -31,6 +31,7 @@ use crate::grpc::extract_correlation_id; use crate::grpc::get_authorization; use crate::grpc::get_repository; use crate::grpc::get_user_id; +use crate::grpc::handlers::branch_push::Integration; use crate::grpc::handlers::branch_push::PushResult; use crate::grpc::handlers::branch_push::dispatch_response_message; use crate::grpc::handlers::branch_push::extract_client_ip; @@ -97,15 +98,22 @@ pub async fn handler( )); } - // The two ask for different histories. Integrating one way anyway would - // leave the client unable to tell which it got, so refuse the request - // rather than resolve it by precedence. - if fast_forward_merge && rebase { - info!("Invalid branch push request, fast_forward_merge and rebase both set"); - return Err(Status::invalid_argument( - "fast_forward_merge and rebase are mutually exclusive", - )); - } + // The wire carries two independent bits; the server takes one decision. + // Both set is refused rather than resolved by precedence: they ask for + // different histories, and applying one anyway would leave the client + // unable to tell which it got. Past this point the invalid combination + // cannot be expressed. + let integration = match (fast_forward_merge, rebase) { + (true, true) => { + info!("Invalid branch push request, fast_forward_merge and rebase both set"); + return Err(Status::invalid_argument( + "fast_forward_merge and rebase are mutually exclusive", + )); + } + (true, false) => Integration::Merge, + (false, true) => Integration::Rebase, + (false, false) => Integration::Refuse, + }; debug!( {REVISION} = %revision, @@ -158,8 +166,7 @@ pub async fn handler( revision, bypass_protection, force, - fast_forward_merge, - rebase, + integration, history_step_size, acceleration, ) @@ -434,7 +441,14 @@ mod test { force: bool, fast_forward_merge: bool, ) -> Request { - make_request_with(repository, branch, revision, force, fast_forward_merge, false) + make_request_with( + repository, + branch, + revision, + force, + fast_forward_merge, + false, + ) } fn make_request_with( diff --git a/lore-server/src/grpc/revision/v1/revision_list.rs b/lore-server/src/grpc/revision/v1/revision_list.rs index 5f8ff17d..a613f492 100644 --- a/lore-server/src/grpc/revision/v1/revision_list.rs +++ b/lore-server/src/grpc/revision/v1/revision_list.rs @@ -1166,8 +1166,7 @@ mod test { serialized, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -1240,8 +1239,7 @@ mod test { serialized, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/thinclient/v1/revision_diff.rs b/lore-server/src/grpc/thinclient/v1/revision_diff.rs index f071d0c8..66b78b37 100644 --- a/lore-server/src/grpc/thinclient/v1/revision_diff.rs +++ b/lore-server/src/grpc/thinclient/v1/revision_diff.rs @@ -859,8 +859,7 @@ mod test { serialized, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/thinclient/v1/revision_info.rs b/lore-server/src/grpc/thinclient/v1/revision_info.rs index d238de85..5bee75fb 100644 --- a/lore-server/src/grpc/thinclient/v1/revision_info.rs +++ b/lore-server/src/grpc/thinclient/v1/revision_info.rs @@ -360,8 +360,7 @@ mod test { serialized, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -552,8 +551,7 @@ mod test { serialized, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -646,8 +644,7 @@ mod test { serialized, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/thinclient/v1/revision_tree.rs b/lore-server/src/grpc/thinclient/v1/revision_tree.rs index 8a6ebdc9..ed60a796 100644 --- a/lore-server/src/grpc/thinclient/v1/revision_tree.rs +++ b/lore-server/src/grpc/thinclient/v1/revision_tree.rs @@ -290,8 +290,7 @@ mod test { serialized, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -379,8 +378,7 @@ mod test { serialized, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -483,8 +481,7 @@ mod test { serialized, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -1493,8 +1490,7 @@ mod test { serialized, true, true, - false, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), )