Radish alpha
h
Radicle Heartwood Protocol & Stack
Radicle
Git (anonymous pull)
Log in to clone via SSH
cob: Redesign merge state
Alexis Sellier committed 3 years ago
commit 2d55ffba81942e9c239497dd52cc246f57bb1d1b
parent 8d8751655f1fbf990244d53fe0110731c68da07a
11 files changed +279 -97
modified radicle-cli/src/commands/patch.rs
@@ -101,6 +101,27 @@ pub enum OperationName {
    Edit,
}

+
pub struct Filter(fn(&patch::State) -> bool);
+

+
impl Filter {
+
    /// Match everything.
+
    fn all() -> Self {
+
        Self(|_| true)
+
    }
+
}
+

+
impl Default for Filter {
+
    fn default() -> Self {
+
        Self(|state| matches!(state, patch::State::Open { .. }))
+
    }
+
}
+

+
impl std::fmt::Debug for Filter {
+
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+
        write!(f, "Filter(..)")
+
    }
+
}
+

#[derive(Debug)]
pub enum Operation {
    Open {
@@ -131,7 +152,7 @@ pub enum Operation {
        patch_id: Rev,
    },
    List {
-
        filter: Option<patch::State>,
+
        filter: Filter,
    },
    Edit {
        patch_id: Rev,
@@ -160,7 +181,7 @@ impl Args for Options {
        let mut patch_id = None;
        let mut message = Message::default();
        let mut push = true;
-
        let mut filter = Some(patch::State::Open);
+
        let mut filter = Filter::default();
        let mut diff = false;
        let mut draft = false;
        let mut undo = false;
@@ -220,19 +241,19 @@ impl Args for Options {

                // List options.
                Long("all") => {
-
                    filter = None;
+
                    filter = Filter::all();
                }
                Long("draft") => {
-
                    filter = Some(patch::State::Draft);
+
                    filter = Filter(|s| s == &patch::State::Draft);
                }
                Long("archived") => {
-
                    filter = Some(patch::State::Archived);
+
                    filter = Filter(|s| s == &patch::State::Archived);
                }
                Long("merged") => {
-
                    filter = Some(patch::State::Merged);
+
                    filter = Filter(|s| matches!(s, patch::State::Merged { .. }));
                }
                Long("open") => {
-
                    filter = Some(patch::State::Open);
+
                    filter = Filter(|s| matches!(s, patch::State::Open { .. }));
                }

                // Common.
@@ -352,8 +373,8 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> {
                options,
            )?;
        }
-
        Operation::List { filter } => {
-
            list::run(&repository, &profile, filter)?;
+
        Operation::List { filter: Filter(f) } => {
+
            list::run(&repository, &profile, f)?;
        }
        Operation::Show { patch_id, diff } => {
            let patch_id = patch_id.resolve(&repository.backend)?;
modified radicle-cli/src/commands/patch/common.rs
@@ -178,9 +178,7 @@ pub fn find_unmerged_with_base(
    let mut matches = Vec::new();

    for (id, patch, clock) in proposed {
-
        let (_, rev) = patch.latest().unwrap();
-

-
        if !rev.merges().count() == 0 {
+
        if patch.merges().count() != 0 {
            continue;
        }
        if **patch.head() == patch_head {
modified radicle-cli/src/commands/patch/list.rs
@@ -16,7 +16,7 @@ use super::common;
pub fn run(
    repository: &Repository,
    profile: &Profile,
-
    filter: Option<patch::State>,
+
    filter: fn(&patch::State) -> bool,
) -> anyhow::Result<()> {
    let patches = Patches::open(repository)?;

@@ -24,10 +24,8 @@ pub fn run(
    for patch in patches.all()? {
        let (id, patch, _) = patch?;

-
        if let Some(filter) = filter {
-
            if patch.state() != filter {
-
                continue;
-
            }
+
        if !filter(patch.state()) {
+
            continue;
        }
        all.push((id, patch));
    }
@@ -105,7 +103,7 @@ pub fn row(

    Ok([
        match state {
-
            patch::State::Open => term::format::positive("●").into(),
+
            patch::State::Open { .. } => term::format::positive("●").into(),
            patch::State::Archived { .. } => term::format::yellow("●").into(),
            patch::State::Draft => term::format::dim("●").into(),
            patch::State::Merged { .. } => term::format::primary("✔").into(),
@@ -172,8 +170,8 @@ pub fn timeline(
            ));
        }

-
        for merge in revision.merges() {
-
            let peer = repository.remote(&merge.node)?;
+
        for (nid, merge) in patch.merges().filter(|(_, m)| m.revision == *revision_id) {
+
            let peer = repository.remote(nid)?;
            let mut badges = Vec::new();

            if peer.id == *whoami {
modified radicle-cli/src/commands/patch/show.rs
@@ -105,10 +105,10 @@ pub fn run(
    attrs.push([
        term::format::tertiary("Status".to_owned()).into(),
        match state {
-
            patch::State::Open => term::format::positive(state.to_string()),
+
            patch::State::Open { .. } => term::format::positive(state.to_string()),
            patch::State::Draft => term::format::dim(state.to_string()),
            patch::State::Archived => term::format::yellow(state.to_string()),
-
            patch::State::Merged => term::format::primary(state.to_string()),
+
            patch::State::Merged { .. } => term::format::primary(state.to_string()),
        }
        .into(),
    ]);
modified radicle-httpd/src/api.rs
@@ -153,10 +153,10 @@ pub enum PatchState {
impl PatchState {
    pub fn matches(&self, patch: &patch::State) -> bool {
        match self {
-
            Self::Open => matches!(patch, patch::State::Open),
+
            Self::Open => matches!(patch, patch::State::Open { .. }),
            Self::Draft => matches!(patch, patch::State::Draft),
            Self::Archived => matches!(patch, patch::State::Archived),
-
            Self::Merged => matches!(patch, patch::State::Merged),
+
            Self::Merged => matches!(patch, patch::State::Merged { .. }),
        }
    }
}
modified radicle-httpd/src/api/json.rs
@@ -123,6 +123,14 @@ pub(crate) fn patch(
        "state": patch.state(),
        "target": patch.target(),
        "tags": patch.tags().collect::<Vec<_>>(),
+
        "merges": patch.merges().map(|(a, m)| {
+
            json!({
+
                "author": a,
+
                "revision": m.revision,
+
                "commit": m.commit,
+
                "timestamp": m.timestamp
+
            })
+
        }).collect::<Vec<_>>(),
        "revisions": patch.revisions().map(|(id, rev)| {
            json!({
                "id": id,
@@ -130,7 +138,6 @@ pub(crate) fn patch(
                "base": rev.base(),
                "oid": rev.head(),
                "refs": get_refs(repo, patch.author().id(), &rev.head()).unwrap_or(vec![]),
-
                "merges": rev.merges().collect::<Vec<_>>(),
                "discussions": rev.discussion().comments()
                  .map(|(id, comment)| Comment::new(id, comment, rev.discussion(), aliases))
                  .collect::<Vec<_>>(),
modified radicle-httpd/src/api/v1/projects.rs
@@ -1745,6 +1745,7 @@ mod routes {
                "state": { "status": "open" },
                "target": "delegates",
                "tags": [],
+
                "merges": [],
                "revisions": [
                  {
                    "id": CONTRIBUTOR_PATCH_ID,
@@ -1754,7 +1755,6 @@ mod routes {
                    "refs": [
                      "refs/heads/master",
                    ],
-
                    "merges": [],
                    "discussions": [],
                    "timestamp": TIMESTAMP,
                    "reviews": [],
@@ -1784,6 +1784,7 @@ mod routes {
                "state": { "status": "open" },
                "target": "delegates",
                "tags": [],
+
                "merges": [],
                "revisions": [
                  {
                    "id": CONTRIBUTOR_PATCH_ID,
@@ -1793,7 +1794,6 @@ mod routes {
                    "refs": [
                      "refs/heads/master",
                    ],
-
                    "merges": [],
                    "discussions": [],
                    "timestamp": TIMESTAMP,
                    "reviews": [],
@@ -1862,6 +1862,7 @@ mod routes {
                "state": { "status": "open" },
                "target": "delegates",
                "tags": [],
+
                "merges": [],
                "revisions": [
                  {
                    "id": CREATED_PATCH_ID,
@@ -1871,7 +1872,6 @@ mod routes {
                    "refs": [
                      "refs/heads/master",
                    ],
-
                    "merges": [],
                    "discussions": [],
                    "timestamp": TIMESTAMP,
                    "reviews": [],
@@ -1925,6 +1925,7 @@ mod routes {
                "bug",
                "design"
              ],
+
              "merges": [],
              "revisions": [
                {
                  "id": CONTRIBUTOR_PATCH_ID,
@@ -1934,7 +1935,6 @@ mod routes {
                  "refs": [
                    "refs/heads/master",
                  ],
-
                  "merges": [],
                  "discussions": [],
                  "timestamp": TIMESTAMP,
                  "reviews": [],
@@ -1985,6 +1985,7 @@ mod routes {
              "state": { "status": "open" },
              "target": "delegates",
              "tags": [],
+
              "merges": [],
              "revisions": [
                {
                  "id": CONTRIBUTOR_PATCH_ID,
@@ -1994,7 +1995,6 @@ mod routes {
                  "refs": [
                    "refs/heads/master",
                  ],
-
                  "merges": [],
                  "discussions": [],
                  "timestamp": TIMESTAMP,
                  "reviews": [],
@@ -2007,7 +2007,6 @@ mod routes {
                  "refs": [
                    "refs/heads/master",
                  ],
-
                  "merges": [],
                  "discussions": [],
                  "timestamp": TIMESTAMP,
                  "reviews": [],
@@ -2058,6 +2057,7 @@ mod routes {
              "state": { "status": "open" },
              "target": "delegates",
              "tags": [],
+
              "merges": [],
              "revisions": [
                {
                  "id": CONTRIBUTOR_PATCH_ID,
@@ -2067,7 +2067,6 @@ mod routes {
                  "refs": [
                    "refs/heads/master",
                  ],
-
                  "merges": [],
                  "discussions": [],
                  "timestamp": TIMESTAMP,
                  "reviews": [],
@@ -2140,6 +2139,7 @@ mod routes {
              "state": { "status": "open" },
              "target": "delegates",
              "tags": [],
+
              "merges": [],
              "revisions": [
                {
                  "id": CONTRIBUTOR_PATCH_ID,
@@ -2149,7 +2149,6 @@ mod routes {
                  "refs": [
                    "refs/heads/master",
                  ],
-
                  "merges": [],
                  "discussions": [
                    {
                      "id": CONTRIBUTOR_COMMENT_1,
@@ -2236,6 +2235,7 @@ mod routes {
              "state": { "status": "open" },
              "target": "delegates",
              "tags": [],
+
              "merges": [],
              "revisions": [
                {
                  "id": CONTRIBUTOR_PATCH_ID,
@@ -2245,7 +2245,6 @@ mod routes {
                  "refs": [
                    "refs/heads/master",
                  ],
-
                  "merges": [],
                  "discussions": [],
                  "timestamp": TIMESTAMP,
                  "reviews": [
@@ -2316,9 +2315,19 @@ mod routes {
              },
              "title": "A new `hello world`",
              "description": "change `hello world` in README to something else",
-
              "state": { "status": "merged" },
+
              "state": {
+
                  "status": "merged",
+
                  "revision": CONTRIBUTOR_PATCH_ID,
+
                  "commit": PARENT,
+
              },
              "target": "delegates",
              "tags": [],
+
              "merges": [{
+
                  "author": CONTRIBUTOR_NID,
+
                  "revision": CONTRIBUTOR_PATCH_ID,
+
                  "commit": PARENT,
+
                  "timestamp": TIMESTAMP,
+
              }],
              "revisions": [
                {
                  "id": CONTRIBUTOR_PATCH_ID,
@@ -2328,13 +2337,6 @@ mod routes {
                  "refs": [
                    "refs/heads/master",
                  ],
-
                  "merges": [
-
                    {
-
                      "node": CONTRIBUTOR_NID,
-
                      "commit": PARENT,
-
                      "timestamp": TIMESTAMP,
-
                    },
-
                  ],
                  "discussions": [],
                  "timestamp": TIMESTAMP,
                  "reviews": [],
modified radicle/src/cob/patch.rs
@@ -1,4 +1,5 @@
#![allow(clippy::too_many_arguments)]
+
use std::collections::HashMap;
use std::fmt;
use std::ops::Deref;
use std::ops::Range;
@@ -11,7 +12,7 @@ use serde::{Deserialize, Serialize};
use thiserror::Error;

use radicle_crdt::clock;
-
use radicle_crdt::{GMap, GSet, LWWReg, LWWSet, Lamport, Max, Redactable, Semilattice};
+
use radicle_crdt::{GMap, GSet, LWWMap, LWWReg, LWWSet, Lamport, Max, Redactable, Semilattice};

use crate::cob;
use crate::cob::common::{Author, Tag, Timestamp};
@@ -64,6 +65,9 @@ pub enum ApplyError {
    /// Error loading the identity document committed to by an operation.
    #[error("identity doc failed to load: {0}")]
    Doc(#[from] DocError),
+
    /// The merge operation is invalid.
+
    #[error("invalid merge operation in {0}")]
+
    InvalidMerge(EntryId),
}

/// Error updating or creating patches.
@@ -157,6 +161,8 @@ pub struct Patch {
    target: LWWReg<Max<MergeTarget>>,
    /// Associated tags.
    tags: LWWSet<Tag>,
+
    /// Patch merges.
+
    merges: LWWMap<ActorId, Redactable<Merge>>,
    /// List of patch revisions. The initial changeset is part of the
    /// first revision.
    revisions: GMap<RevisionId, Redactable<Revision>>,
@@ -170,6 +176,7 @@ impl Semilattice for Patch {
        self.description.merge(other.description);
        self.state.merge(other.state);
        self.target.merge(other.target);
+
        self.merges.merge(other.merges);
        self.tags.merge(other.tags);
        self.revisions.merge(other.revisions);
        self.timeline.merge(other.timeline);
@@ -184,6 +191,7 @@ impl Default for Patch {
            state: LWWReg::initial(Max::from(State::default())),
            target: LWWReg::initial(Max::from(MergeTarget::default())),
            tags: LWWSet::default(),
+
            merges: LWWMap::default(),
            revisions: GMap::default(),
            timeline: GSet::default(),
        }
@@ -197,8 +205,8 @@ impl Patch {
    }

    /// Current state of the patch.
-
    pub fn state(&self) -> State {
-
        *self.state.get().get()
+
    pub fn state(&self) -> &State {
+
        self.state.get().get()
    }

    /// Target this patch is meant to be merged in.
@@ -253,6 +261,13 @@ impl Patch {
        })
    }

+
    /// Get the merges.
+
    pub fn merges(&self) -> impl Iterator<Item = (&ActorId, &Merge)> {
+
        self.merges
+
            .iter()
+
            .filter_map(|(a, m)| m.get().map(|m| (a, m)))
+
    }
+

    /// Reference to the Git object containing the code on the latest revision.
    pub fn head(&self) -> &git::Oid {
        &self
@@ -287,7 +302,7 @@ impl Patch {

    /// Check if the patch is open.
    pub fn is_open(&self) -> bool {
-
        matches!(self.state(), State::Open)
+
        matches!(self.state(), State::Open { .. })
    }

    /// Check if the patch is archived.
@@ -404,25 +419,56 @@ impl store::FromHistory for Patch {
                    }
                }
                Action::Merge { revision, commit } => {
-
                    if let Some(Redactable::Present(revision)) = self.revisions.get_mut(&revision) {
-
                        revision.merges.insert(
-
                            Merge {
-
                                node: op.author,
+
                    if let Some(Redactable::Present(_)) = self.revisions.get_mut(&revision) {
+
                        let doc = repo.identity_doc_at(op.identity)?;
+
                        if !doc.is_delegate(&op.author) {
+
                            return Err(ApplyError::InvalidMerge(op.id));
+
                        }
+
                        self.merges.insert(
+
                            op.author,
+
                            Redactable::Present(Merge {
+
                                revision,
                                commit,
                                timestamp,
-
                            }
-
                            .into(),
+
                            }),
                            op.clock,
                        );
-
                        let doc = repo.identity_doc_at(op.identity)?;

-
                        if revision
-
                            .merges()
-
                            .filter(|m| doc.is_delegate(&m.node))
-
                            .count()
-
                            >= doc.threshold
-
                        {
-
                            self.state.set(State::Merged, op.clock);
+
                        let mut merges = self.merges.iter().fold(
+
                            HashMap::<(RevisionId, git::Oid), usize>::new(),
+
                            |mut acc, (_, merge)| {
+
                                if let Some(merge) = merge.get() {
+
                                    *acc.entry((merge.revision, merge.commit)).or_default() += 1;
+
                                }
+
                                acc
+
                            },
+
                        );
+
                        // Discard revisions that weren't merged by a threshold of delegates.
+
                        merges.retain(|_, count| *count >= doc.threshold);
+

+
                        match merges.into_keys().collect::<Vec<_>>().as_slice() {
+
                            [] => {
+
                                // None of the revisions met the quorum.
+
                            }
+
                            [(revision, commit)] => {
+
                                // Patch is merged.
+
                                self.state.set(
+
                                    State::Merged {
+
                                        revision: *revision,
+
                                        commit: *commit,
+
                                    },
+
                                    op.clock,
+
                                );
+
                            }
+
                            revisions => {
+
                                // More than one revision met the quorum.
+
                                self.state.set(
+
                                    State::Open {
+
                                        conflicts: revisions.to_vec(),
+
                                    },
+
                                    op.clock,
+
                                );
+
                            }
                        }
                    } else {
                        return Err(ApplyError::Missing(revision));
@@ -466,8 +512,6 @@ pub struct Revision {
    oid: git::Oid,
    /// Discussion around this revision.
    discussion: Thread,
-
    /// Merges of this revision into other repositories.
-
    merges: LWWSet<Max<Merge>>,
    /// Reviews of this revision's changes (one per actor).
    reviews: GMap<ActorId, Review>,
    /// When this revision was created.
@@ -489,7 +533,6 @@ impl Revision {
            base,
            oid,
            discussion: Thread::default(),
-
            merges: LWWSet::default(),
            reviews: GMap::default(),
            timestamp,
        }
@@ -524,11 +567,6 @@ impl Revision {
        &self.discussion
    }

-
    /// Merges of this revision into other repositories.
-
    pub fn merges(&self) -> impl Iterator<Item = &Merge> {
-
        self.merges.iter().map(|m| m.get())
-
    }
-

    /// Reviews of this revision's changes (one per actor).
    pub fn reviews(&self) -> impl DoubleEndedIterator<Item = (&PublicKey, &Review)> {
        self.reviews.iter()
@@ -536,14 +574,29 @@ impl Revision {
}

/// Patch state.
-
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)]
+
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)]
#[serde(rename_all = "camelCase", tag = "status")]
pub enum State {
-
    #[default]
-
    Open,
    Draft,
+
    Open {
+
        /// Revisions that were merged and are conflicting.
+
        #[serde(skip_serializing_if = "Vec::is_empty")]
+
        #[serde(default)]
+
        conflicts: Vec<(RevisionId, git::Oid)>,
+
    },
    Archived,
-
    Merged,
+
    Merged {
+
        /// The revision that was merged.
+
        revision: RevisionId,
+
        /// The commit in the target branch that contains the changes.
+
        commit: git::Oid,
+
    },
+
}
+

+
impl Default for State {
+
    fn default() -> Self {
+
        Self::Open { conflicts: vec![] }
+
    }
}

impl fmt::Display for State {
@@ -551,8 +604,8 @@ impl fmt::Display for State {
        match self {
            Self::Archived => write!(f, "archived"),
            Self::Draft => write!(f, "draft"),
-
            Self::Open => write!(f, "open"),
-
            Self::Merged => write!(f, "merged"),
+
            Self::Open { .. } => write!(f, "open"),
+
            Self::Merged { .. } => write!(f, "merged"),
        }
    }
}
@@ -561,11 +614,11 @@ impl fmt::Display for State {
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)]
#[serde(rename_all = "camelCase")]
pub struct Merge {
-
    /// Owner of repository that this patch was merged into.
-
    pub node: NodeId,
+
    /// Revision that was merged.
+
    pub revision: RevisionId,
    /// Base branch commit that contains the revision.
    pub commit: git::Oid,
-
    /// When this merged was performed.
+
    /// When this merge was performed.
    pub timestamp: Timestamp,
}

@@ -1002,14 +1055,14 @@ impl<'a, 'g> PatchMut<'a, 'g> {
        if !self.is_draft() {
            return Ok(false);
        }
-
        self.lifecycle(State::Open, signer)?;
+
        self.lifecycle(State::Open { conflicts: vec![] }, signer)?;

        Ok(true)
    }

-
    /// Mark an open patch as a draft. Returns `false` if the patch was not open.
+
    /// Mark an open patch as a draft. Returns `false` if the patch was not open and free of merges.
    pub fn unready<G: Signer>(&mut self, signer: &G) -> Result<bool, Error> {
-
        if !self.is_open() {
+
        if !matches!(self.state(), State::Open { conflicts } if conflicts.is_empty()) {
            return Ok(false);
        }
        self.lifecycle(State::Draft, signer)?;
@@ -1120,9 +1173,9 @@ impl<'a> Patches<'a> {
                .fold(PatchCounts::default(), |mut state, (_, p, _)| {
                    match p.state() {
                        State::Draft => state.draft += 1,
-
                        State::Open => state.open += 1,
+
                        State::Open { .. } => state.open += 1,
                        State::Archived => state.archived += 1,
-
                        State::Merged => state.merged += 1,
+
                        State::Merged { .. } => state.merged += 1,
                    }
                    state
                });
@@ -1229,10 +1282,12 @@ mod test {

    use radicle_crdt::test::{assert_laws, WeightedGenerator};

+
    use nonempty::nonempty;
    use pretty_assertions::assert_eq;
    use qcheck::{Arbitrary, TestResult};

    use super::*;
+
    use crate::assert_matches;
    use crate::cob::test::Actor;
    use crate::crypto::test::signer::MockSigner;
    use crate::test;
@@ -1429,7 +1484,7 @@ mod test {
        assert_eq!(patch.title(), "My first patch");
        assert_eq!(patch.description(), "Blah blah blah.");
        assert_eq!(patch.author().id(), &author);
-
        assert_eq!(patch.state(), State::Open);
+
        assert_eq!(patch.state(), &State::Open { conflicts: vec![] });
        assert_eq!(patch.target(), target);
        assert_eq!(patch.version(), 0);

@@ -1504,13 +1559,63 @@ mod test {

        let patch = patches.get(&id).unwrap().unwrap();

-
        let (_, r) = patch.revisions().next().unwrap();
-
        let merges = r.merges.iter().collect::<Vec<_>>();
+
        let merges = patch.merges.iter().collect::<Vec<_>>();
        assert_eq!(merges.len(), 1);

-
        let merge = merges.first().unwrap();
-
        assert_eq!(merge.node, *signer.public_key());
-
        assert_eq!(merge.commit, pr.base);
+
        let (merger, merge) = merges.first().unwrap();
+
        assert_eq!(*merger, signer.public_key());
+
        assert_eq!(merge.get().unwrap().commit, pr.base);
+
    }
+

+
    #[test]
+
    fn test_patch_merge_and_archive() {
+
        let rid = gen::<Id>(1);
+
        let base = git::Oid::from_str("d8711a8d43dc919fe39ae4b7c2f7b24667f5d470").unwrap();
+
        let commit = git::Oid::from_str("cb18e95ada2bb38aadd8e6cef0963ce37a87add3").unwrap();
+

+
        let mut alice = Actor::<MockSigner, _>::default();
+
        let mut bob = Actor::<MockSigner, _>::default();
+

+
        let proj = gen::<Project>(1);
+
        let doc = Doc::new(proj, nonempty![alice.did(), bob.did()], 1)
+
            .verified()
+
            .unwrap();
+
        let repo = MockRepository::new(rid, doc);
+
        let patch = alice
+
            .patch("Some changes", "", base, commit, &repo)
+
            .unwrap();
+
        let (revision, _) = patch.revisions().next().unwrap();
+

+
        // Create two concurrent operations.
+
        let clock = Lamport::from(2);
+
        let identity = repo.identity_head().unwrap();
+
        let ops = [
+
            alice.op_with(
+
                Action::Merge {
+
                    revision: *revision,
+
                    commit,
+
                },
+
                clock,
+
                identity,
+
            ),
+
            bob.op_with(
+
                Action::Lifecycle {
+
                    state: State::Archived,
+
                },
+
                clock,
+
                identity,
+
            ),
+
        ];
+

+
        let mut patch1 = patch.clone();
+
        let mut patch2 = patch.clone();
+

+
        // Apply the ops in different orders and expect the patch state to remain the same.
+
        patch1.apply(ops.iter().cloned(), &repo).unwrap();
+
        patch2.apply(ops.iter().cloned().rev(), &repo).unwrap();
+

+
        assert_matches!(patch1.state(), &State::Merged { .. });
+
        assert_matches!(patch2.state(), &State::Merged { .. });
    }

    #[test]
@@ -1615,8 +1720,12 @@ mod test {
    fn test_revision_merge_reinsert() {
        let base = git::Oid::from_str("cb18e95ada2bb38aadd8e6cef0963ce37a87add3").unwrap();
        let oid = git::Oid::from_str("518d5069f94c03427f694bb494ac1cd7d1339380").unwrap();
-
        let repo = gen::<MockRepository>(1);
+
        let id = gen::<Id>(1);
        let mut alice = Actor::<_, Action>::new(MockSigner::default());
+
        let mut doc = gen::<Doc<Verified>>(1);
+
        doc.delegates.push(alice.signer.public_key().into());
+
        let repo = MockRepository::new(id, doc);
+

        let mut p1 = Patch::default();
        let mut p2 = Patch::default();

modified radicle/src/cob/test.rs
@@ -7,10 +7,15 @@ use serde::Serialize;

use crate::cob::common::clock;
use crate::cob::op::{Op, Ops};
+
use crate::cob::patch;
+
use crate::cob::patch::Patch;
use crate::cob::store::encoding;
use crate::cob::History;
use crate::crypto::{PublicKey, Signer};
+
use crate::git;
use crate::git::Oid;
+
use crate::prelude::Did;
+
use crate::storage::ReadRepository;
use crate::test::arbitrary;

use super::store::FromHistory;
@@ -146,22 +151,62 @@ impl<G: Signer, A: Clone + Serialize> Actor<G, A> {
    }

    /// Create a new operation.
-
    pub fn op(&mut self, action: A) -> Op<A> {
+
    pub fn op_with(&mut self, action: A, clock: clock::Lamport, identity: Oid) -> Op<A> {
        let id = arbitrary::oid().into();
        let author = *self.signer.public_key();
-
        let clock = self.clock.tick();
        let timestamp = clock::Physical::now();
-
        let identity = arbitrary::oid();
-
        let op = Op {
+

+
        Op {
            id,
            action,
            author,
            clock,
            timestamp,
            identity,
-
        };
-
        self.ops.insert((self.clock, author), op.clone());
+
        }
+
    }
+

+
    /// Create a new operation.
+
    pub fn op(&mut self, action: A) -> Op<A> {
+
        let clock = self.clock.tick();
+
        let identity = arbitrary::oid();
+
        let op = self.op_with(action, clock, identity);
+

+
        self.ops.insert((self.clock, op.author), op.clone());

        op
    }
+

+
    /// Get the actor's DID.
+
    pub fn did(&self) -> Did {
+
        self.signer.public_key().into()
+
    }
+
}
+

+
impl<G: Signer> Actor<G, super::patch::Action> {
+
    /// Create a patch.
+
    pub fn patch<R: ReadRepository>(
+
        &mut self,
+
        title: impl ToString,
+
        description: impl ToString,
+
        base: git::Oid,
+
        oid: git::Oid,
+
        repo: &R,
+
    ) -> Result<Patch, patch::ApplyError> {
+
        Patch::from_ops(
+
            [
+
                self.op(patch::Action::Revision {
+
                    description: description.to_string(),
+
                    base,
+
                    oid,
+
                }),
+
                self.op(patch::Action::Edit {
+
                    title: title.to_string(),
+
                    description: description.to_string(),
+
                    target: patch::MergeTarget::default(),
+
                }),
+
            ],
+
            repo,
+
        )
+
    }
}
modified radicle/src/test.rs
@@ -59,6 +59,7 @@ pub mod setup {
            let oid = commit(&self.working, &refname, blobs, &[&parent]);

            git::push(&self.working, &REMOTE_NAME, [(&refname, &refname)]).unwrap();
+

            BranchWith {
                base: base.into(),
                oid,
modified radicle/src/test/storage.rs
@@ -1,6 +1,7 @@
use std::collections::HashMap;
use std::io;
use std::path::{Path, PathBuf};
+
use std::str::FromStr;

use git_ext::ref_format as fmt;

@@ -222,11 +223,11 @@ impl ReadRepository for MockRepository {
    }

    fn identity_head(&self) -> Result<Oid, IdentityError> {
-
        todo!()
+
        self.canonical_identity_head()
    }

    fn canonical_identity_head(&self) -> Result<Oid, IdentityError> {
-
        todo!()
+
        Ok(Oid::from_str("cb18e95ada2bb38aadd8e6cef0963ce37a87add3").unwrap())
    }
}