Let a commit land when it already carries the branch latest

lore-revision/src/commit.rs

@@ -637,7 +637,34 @@ pub async fn commit_impl(
         .await
         .unwrap_or_default();
     if !globals.force() && !branch_latest.is_zero() && branch_latest != current_revision {
-        return Err(BranchAdvanced.into());
+        // The rule this check exists for is "do not drop what another instance
+        // added", and an anchor that equals the latest is only the cheapest way
+        // of satisfying it. A staged state that already has the latest in its
+        // ancestry satisfies it too: committing it publishes that work rather
+        // than replacing it.
+        //
+        // Sync's own merge is exactly that state, and without this it cannot
+        // land: sync stages a merge of the remote target with the local
+        // revision, then commits it — and the commit was refused for the very
+        // divergence the merge resolves, telling the user to sync while they
+        // were inside a sync.
+        //
+        // Reached only when the cheap comparison already failed, so the
+        // ordinary commit path is unchanged.
+        if !incorporates_branch_latest(
+            repository.clone(),
+            current_branch,
+            staged_revision,
+            branch_latest,
+        )
+        .await
+        {
+            return Err(BranchAdvanced.into());
+        }
+        lore_debug!(
+            "Branch latest {branch_latest} is already part of staged revision {staged_revision}, \
+             committing on top of it"
+        );
     }
 
     let state_staged = State::deserialize(repository.clone(), staged_revision)
@@ -1615,6 +1642,68 @@ async fn commit_staged_revision(
     }
 }
 
+/// How far back the reachability walk below is willing to look for the branch
+/// latest. Generous for the shapes it exists to answer — a sync merge finds it
+/// one step in — and bounded so a pathological history cannot turn a commit
+/// into a long walk.
+const BRANCH_LATEST_SEARCH_LIMIT: usize = 512;
+
+/// Whether committing `staged` would publish `branch_latest` rather than
+/// replace it — i.e. whether the latest is already part of the staged state's
+/// ancestry.
+///
+/// Answers the question [`commit`]'s branch-advanced check actually cares
+/// about. The cheap cases are the two parents: a staged state built directly on
+/// the latest, and a merge that took it as its second parent. Otherwise walk
+/// first parents back from the staged state's own parent, stopping as soon as
+/// the revision numbers fall below the latest's, since nothing older can be it.
+/// In the shape this exists for — sync staging a merge of a remote target that
+/// descends from the latest — the walk ends on the first or second step, and
+/// for a plain commit on a stale anchor it stops immediately.
+///
+/// **Fails closed.** Anything that cannot be established — an unreadable state,
+/// a walk that runs past its bound, a history this clone only partly holds —
+/// answers `false` and the commit is refused. A gap here costs a refusal the
+/// user can resolve by syncing; the opposite mistake costs somebody's work.
+async fn incorporates_branch_latest(
+    repository: Arc<RepositoryContext>,
+    branch: BranchId,
+    staged: Hash,
+    branch_latest: Hash,
+) -> bool {
+    let Ok(state_staged) = State::deserialize(repository.clone(), staged).await else {
+        return false;
+    };
+    if state_staged.parent_self() == branch_latest || state_staged.parent_other() == branch_latest {
+        return true;
+    }
+
+    let Ok(state_latest) = State::deserialize(repository.clone(), branch_latest).await else {
+        return false;
+    };
+    let latest_number = state_latest.revision_number();
+
+    crate::find::find_revision(
+        repository,
+        branch,
+        state_staged.parent_self(),
+        false,
+        Some(BRANCH_LATEST_SEARCH_LIMIT),
+        |state, _| {
+            if state.revision() == branch_latest {
+                crate::find::FindMatchResult::Match
+            } else if state.revision_number() < latest_number {
+                // Walked past the point the latest could still appear.
+                crate::find::FindMatchResult::Abort
+            } else {
+                crate::find::FindMatchResult::Continue
+            }
+        },
+    )
+    .await
+    .is_ok()
+}
+
 /// Publish `signature` as `branch`'s tip and anchor it as the current revision.
 ///
 /// `previous` is the tip the caller observed when it decided to commit; the

lore-revision/tests/commit.rs

@@ -600,4 +600,252 @@ mod tests {
             "expected NO RevisionCommitRevision event (discriminant {commit_discriminant}) in captured {captured:?}"
         );
     }
+
+    // ---------------------------------------------------------------------
+    // Branch-advanced gate: the check is "does this commit drop what another
+    // instance added", not "is your anchor the branch latest". These two
+    // tests are the pair that defines the difference — one staged state that
+    // already carries the latest, one that does not, with identical pointers.
+    //
+    // The pointers are set directly rather than produced by a race, because
+    // the gate only ever reads them: how the anchor came to differ from the
+    // latest (a server-side fast-forward merge, a forced commit) is not
+    // something it can see.
+    // ---------------------------------------------------------------------
+
+    /// Build a repository with three linear revisions and return
+    /// `(repository, write_token, [r1, r2, r3])`, leaving the anchor and the
+    /// branch latest both on `r3`.
+    async fn three_revision_chain(
+        path: &std::path::Path,
+        immutable_store: Arc<dyn lore_storage::ImmutableStore>,
+        mutable_store: Arc<dyn lore_storage::MutableStore>,
+    ) -> (
+        Arc<RepositoryContext>,
+        repository::RepositoryWriteToken,
+        BranchId,
+        Vec<lore_base::types::Hash>,
+    ) {
+        std::fs::create_dir_all(path).expect("Create directory failed");
+        let repository_id = RepositoryId::from(uuid::Uuid::now_v7());
+        let default_branch_id = BranchId::from(uuid::Uuid::now_v7());
+        let write_token = repository::RepositoryWriteToken::acquire(path).await;
+        let created_repo = repository::create_local(
+            path,
+            &write_token,
+            repository_id,
+            default_branch_id,
+            branch::DEFAULT_DEFAULT_NAME.to_string(),
+            repository::RepositoryConfig::default(),
+            false,
+        )
+        .await
+        .expect("Failed to initialize repository");
+
+        let repository = Arc::new(
+            RepositoryContext::new(
+                default_repository_creation_args(immutable_store, mutable_store)
+                    .with_path(path)
+                    .with_id(repository_id)
+                    .with_instance_id(created_repo.instance_id),
+            )
+            .with_write_token(write_token.share()),
+        );
+        lore_revision::instance::store_current_anchor_branch(&repository, default_branch_id)
+            .await
+            .expect("Failed to store anchor branch");
+
+        let mut revisions = Vec::new();
+        for i in 0..3u8 {
+            let file_path = path.join(format!("file{i}.bin"));
+            {
+                let mut file = std::fs::File::options()
+                    .create(true)
+                    .truncate(true)
+                    .write(true)
+                    .open(&file_path)
+                    .expect("Failed to create test file");
+                file.write_all(&[i, i, i])
+                    .expect("Failed to write test file");
+            }
+            file::stage::stage(
+                repository.clone(),
+                &write_token,
+                LoreArray::from_vec(vec![LoreString::from(&file_path)]),
+                StageOptions {
+                    case_change: stage::StageCaseChange::Error,
+                    node_flags: NodeFlags::NoFlags,
+                    file_id: None,
+                    no_children: false,
+                    scan: true,
+                },
+            )
+            .await
+            .expect("Failed to stage file");
+
+            let signature = Box::pin(commit::commit(
+                repository.clone(),
+                &write_token,
+                CommitOptions {
+                    message: format!("r{i}"),
+                    link_messages: std::collections::HashMap::new(),
+                    link: None,
+                    layer_messages: std::collections::HashMap::new(),
+                    layer: None,
+                },
+            ))
+            .await
+            .expect("Failed to commit revision");
+            revisions.push(signature);
+        }
+
+        (repository, write_token, default_branch_id, revisions)
+    }
+
+    /// Stage a state with a chosen ancestry, the way a merge leaves one behind:
+    /// a serialized state stored as the staged anchor. `parent_other` set makes
+    /// it a merge, which is the shape sync stages — `parent_self` the remote
+    /// target it is synchronizing to, `parent_other` the local revision.
+    async fn stage_state_on(
+        repository: &Arc<RepositoryContext>,
+        write_token: &repository::RepositoryWriteToken,
+        parent_self: lore_base::types::Hash,
+        parent_other: lore_base::types::Hash,
+    ) -> lore_base::types::Hash {
+        let state = state::State::deserialize(repository.clone(), parent_self)
+            .await
+            .expect("deserialize parent state");
+        state.set_parent_self(parent_self);
+        state.set_parent_other(parent_other);
+        state.set_revision_number(0);
+        state.set_metadata_hash(Default::default());
+        state.mark_dirty();
+        let signature = state
+            .serialize(repository.clone(), write_token)
+            .await
+            .expect("serialize staged state");
+        lore_revision::instance::store_staged_anchor(repository, signature)
+            .await
+            .expect("store staged anchor");
+        signature
+    }
+
+    /// The branch latest is already in the staged state's ancestry — two steps
+    /// back, the shape a sync's own merge commit has. Committing it publishes
+    /// that work rather than replacing it, so it is allowed even though the
+    /// anchor is not the latest.
+    #[tokio::test]
+    async fn commit_allowed_when_staged_state_already_carries_the_branch_latest() {
+        let (immutable_store, mutable_store, execution) =
+            test_store_create().await.expect("Failed to create stores");
+
+        #[allow(clippy::disallowed_methods)]
+        runtime()
+            .spawn(LORE_CONTEXT.scope(execution.clone(), async move {
+                let tempdir = generate_tempdir();
+                let (repository, write_token, branch_id, revisions) =
+                    three_revision_chain(tempdir.to_path_buf().as_path(), immutable_store, mutable_store)
+                        .await;
+                let (r1, r2, r3) = (revisions[0], revisions[1], revisions[2]);
+
+                // Anchor behind the latest, and the latest itself behind the
+                // revision the staged state is built on: r1 <- r2 <- r3.
+                lore_revision::instance::store_current_anchor(&repository, r1)
+                    .await
+                    .expect("store anchor");
+                branch::store_latest(
+                    repository.clone(),
+                    branch_id,
+                    r3,
+                    r2,
+                    branch::BranchLatestStatus::Divergent,
+                )
+                .await
+                .expect("store latest");
+
+                // The shape sync stages: parent_self the target it is
+                // synchronizing to, parent_other the revision it is on.
+                let staged = stage_state_on(&repository, &write_token, r3, r1).await;
+                assert_ne!(staged, r3, "staged state must be its own revision");
+
+                let result = Box::pin(commit::commit(
+                    repository.clone(),
+                    &write_token,
+                    CommitOptions {
+                        message: "carries the latest".to_string(),
+                        link_messages: std::collections::HashMap::new(),
+                        link: None,
+                        layer_messages: std::collections::HashMap::new(),
+                        layer: None,
+                    },
+                ))
+                .await;
+
+                assert!(
+                    result.is_ok(),
+                    "a staged state whose ancestry contains the branch latest must commit, got {result:?}"
+                );
+            }))
+            .await
+            .expect("Test task failed");
+    }
+
+    /// The counterpart, and the reason the check still earns its place: the
+    /// staged state hangs off the stale anchor and never reaches the latest, so
+    /// committing it would drop whatever advanced the branch. Still refused.
+    #[tokio::test]
+    async fn commit_refused_when_staged_state_cannot_reach_the_branch_latest() {
+        let (immutable_store, mutable_store, execution) =
+            test_store_create().await.expect("Failed to create stores");
+
+        #[allow(clippy::disallowed_methods)]
+        runtime()
+            .spawn(LORE_CONTEXT.scope(execution.clone(), async move {
+                let tempdir = generate_tempdir();
+                let (repository, write_token, branch_id, revisions) = three_revision_chain(
+                    tempdir.to_path_buf().as_path(),
+                    immutable_store,
+                    mutable_store,
+                )
+                .await;
+                let (r1, r2, r3) = (revisions[0], revisions[1], revisions[2]);
+
+                // Same pointers as the test above — only the staged state
+                // differs: it is built on r1, which cannot reach r2.
+                lore_revision::instance::store_current_anchor(&repository, r1)
+                    .await
+                    .expect("store anchor");
+                branch::store_latest(
+                    repository.clone(),
+                    branch_id,
+                    r3,
+                    r2,
+                    branch::BranchLatestStatus::Divergent,
+                )
+                .await
+                .expect("store latest");
+
+                stage_state_on(&repository, &write_token, r1, Default::default()).await;
+
+                let result = Box::pin(commit::commit(
+                    repository.clone(),
+                    &write_token,
+                    CommitOptions {
+                        message: "would drop the advance".to_string(),
+                        link_messages: std::collections::HashMap::new(),
+                        link: None,
+                        layer_messages: std::collections::HashMap::new(),
+                        layer: None,
+                    },
+                ))
+                .await;
+
+                assert!(
+                    result.is_err(),
+                    "a staged state that cannot reach the branch latest must still be refused"
+                );
+            }))
+            .await
+            .expect("Test task failed");
+    }
 }