Conversation
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 <j.hunz@anchorpoint.app>
jochenhz
force-pushed
the
fix/push-reparent-only-on-rewrite
branch
from
September 16, 2026 06:43
300cce1 to
a6eec66
Compare
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 <j.hunz@anchorpoint.app>
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 <j.hunz@anchorpoint.app>
jochenhz
force-pushed
the
feat/push-server-rebase
branch
from
September 16, 2026 06:44
54041dd to
9c9e596
Compare
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 <j.hunz@anchorpoint.app>
jochenhz
force-pushed
the
fix/push-reparent-only-on-rewrite
branch
2 times, most recently
from
September 16, 2026 19:12
abe7c41 to
212b90e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
So the machinery for a rebase is there. This adds a
rebaseflag toBranchPushRequestthat asks for the other shape: same diff, same conflict handling, but the result records only the head as its parent, so the branch stays linear.Design notes
Mutually exclusive, not ordered by precedence. The two 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::pushfor embedders that never touch the CLI, and the v1 handler for anything arriving on the wire.rebasedoes not implyfast_forward_merge. A server that predates the field ignores it, sees a push with no integration opt-in, and soft-rejects. That is the point: a client asking to rebase must never be answered with a merge it did not ask for. The client turns that refusal into a message naming the likely cause, since a server that did understand the field would have rebased or reported conflicts instead — so the refusal is diagnostic.Metadata. A rebase is stamped
rebased-on-pushrather thanmerged-by/fast-forward-merge. Nothing was merged, and a reader should not meet a "merge" with no second parent.What it gives up, 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 that push, not beyond.Verification
contrib/repro-push-reparent-drops-work.sh(from #2) grew aMODE=rebasearm. Run against a server built from this branch and against the released 0.9.0 server:--rebasehistory is linear: no merge revision(revisions 1‑4)--fast-forward-merge--rebaseerror: the argument '--rebase' cannot be used with '--fast-forward-merge'cargo test -p lore-revisiongreen (700+ tests).cargo fmt --checkandcargo clippyclean for the touched crates. Protobuf bindings regenerated with protoc 36.1; the diff is a pure addition.Not covered
contrib/is what I have.rebase: falsefrom their call sites, so their behaviour is unchanged — but the reproduction does not exercise them.model.protorequest has norebasefield; that handler always merges. Intentional, but worth a maintainer's eye.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.