receive: write a review's two refs as one atomic proposal
commit
2a4a4dfreceive: write a review's two refs as one atomic proposal
A review’s entity ref and its retention pin must both exist for the review to exist (model.review, model.review-pin), but review new wrote them as two sequential propose_entity/propose_pin calls — a failure between them left an entity with no pin, silently breaking the retention guarantee. propose_entity_with_pin batches both transitions into one Proposal / one receive() call, so the ref-store’s atomic multi-ref CAS admits or refuses them together (receive.multi-ref-atomicity).
receive: add propose_entity_with_pin for atomic entity+pin writes model: write review new’s two refs through one atomic proposal Assisted-by: Claude:claude-fable-5
Reviews
No reviews of this commit yet — record a verdict below.
Start a review
crates/kernel/ents-receive/src/lib.rs
@@ -96,7 +96,7 @@
pub use error::{Error, Result};
pub use outcome::{Mode, Outcome, TxResult};
pub use proposal::{Proposal, RefTransition, TransportAuth};
-pub use propose::{Identity, propose_delete, propose_entity, propose_pin};
+pub use propose::{Identity, propose_delete, propose_entity, propose_entity_with_pin, propose_pin};
pub use receive::receive;
pub use reconcile::reconcile;
pub use sink::{EventSink, MemoryEventSink, NullEventSink};
crates/kernel/ents-receive/src/propose.rs
@@ -108,23 +108,41 @@
subject: &str,
mode: Mode,
) -> Result<Outcome> {
- let tree = facet_git_tree::serialize_into(entity, objects)?;
- let old = refs.get(name.as_ref())?;
- let parents: Vec<_> = old.into_iter().collect();
- let tip = signed_commit(objects, &name, tree, parents, identity, subject)?;
-
+ let (transition, tip) = entity_transition(refs, objects, &name, entity, identity, subject)?;
let proposal = Proposal {
- transitions: vec![RefTransition {
- name,
- old,
- new: Some(tip),
- }],
+ transitions: vec![transition],
objects: vec![tip],
auth: None,
};
crate::receive::receive(refs, objects, events, &proposal, mode)
}
+/// Build the entity-mutation [`RefTransition`] `propose_entity` and
+/// `propose_entity_with_pin` share: serialize `entity`, wrap it in a signed
+/// commit whose only parent is `name`'s current tip, and return that
+/// transition alongside the tip oid the [`Proposal`] must carry.
+fn entity_transition<T: for<'facet> facet::Facet<'facet>>(
+ refs: &dyn RefStore,
+ objects: &(impl Find + Write),
+ name: &FullName,
+ entity: &T,
+ identity: &Identity<'_>,
+ subject: &str,
+) -> Result<(RefTransition, gix_hash::ObjectId)> {
+ let tree = facet_git_tree::serialize_into(entity, objects)?;
+ let old = refs.get(name.as_ref())?;
+ let parents: Vec<_> = old.into_iter().collect();
+ let tip = signed_commit(objects, name, tree, parents, identity, subject)?;
+ Ok((
+ RefTransition {
+ name: name.clone(),
+ old,
+ new: Some(tip),
+ },
+ tip,
+ ))
+}
+
/// Advance the retention pin at `name` to keep `retain` (and its ancestry)
/// reachable (`model.review-pin`): a signed commit carrying the empty tree
/// — a pin's commits anchor other content's reachability and carry no
@@ -198,18 +216,132 @@
subject: &str,
mode: Mode,
) -> Result<Outcome> {
+ let (transition, tip) = pin_transition(refs, objects, &name, retain, identity, subject)?;
+ let proposal = Proposal {
+ transitions: vec![transition],
+ objects: vec![tip],
+ auth: None,
+ };
+ crate::receive::receive(refs, objects, events, &proposal, mode)
+}
+
+/// Build the retention-pin [`RefTransition`] `propose_pin` and
+/// `propose_entity_with_pin` share: an empty-tree, merge-shaped signed
+/// commit whose parents are `name`'s current tip (if any) followed by
+/// `retain` (`model.review-pin`), returned alongside the tip oid the
+/// [`Proposal`] must carry.
+fn pin_transition(
+ refs: &dyn RefStore,
+ objects: &(impl Find + Write),
+ name: &FullName,
+ retain: gix_hash::ObjectId,
+ identity: &Identity<'_>,
+ subject: &str,
+) -> Result<(RefTransition, gix_hash::ObjectId)> {
let tree = objects.write(&gix_object::Tree { entries: vec![] })?;
let old = refs.get(name.as_ref())?;
let parents: Vec<_> = old.into_iter().chain(std::iter::once(retain)).collect();
- let tip = signed_commit(objects, &name, tree, parents, identity, subject)?;
-
- let proposal = Proposal {
- transitions: vec![RefTransition {
- name,
+ let tip = signed_commit(objects, name, tree, parents, identity, subject)?;
+ Ok((
+ RefTransition {
+ name: name.clone(),
old,
new: Some(tip),
- }],
- objects: vec![tip],
+ },
+ tip,
+ ))
+}
+
+/// Write an entity and its retention pin as one atomic mutation
+/// (`receive.multi-ref-atomicity`): the entity commit at `entity_name` and
+/// the empty-tree pin commit at `pin_name` (retaining `retain`) travel in a
+/// single [`Proposal`] through one [`crate::receive`] call, so the
+/// ref-store's atomic multi-ref compare-and-swap admits or refuses both
+/// together — a review is never observable with its entity written but its
+/// pin missing (`model.review`, `model.review-pin`).
+///
+/// # Errors
+///
+/// As [`propose_entity`] and [`propose_pin`], for either ref; a
+/// reached-but-negative [`Outcome`] on either transition refuses the whole
+/// batch and is returned as `Ok`.
+///
+/// # Examples
+///
+/// ```
+/// use ents_model::{Provenance, namespace};
+/// use ents_receive::{Identity, Mode, NullEventSink, TxResult, propose_entity_with_pin};
+/// use ents_testutil::{CommitSpec, Keypair, MemRefStore, ObjectStore, empty_tree, enroll_member};
+/// use facet::Facet;
+///
+/// # #[derive(Facet)]
+/// # struct Review { verdict: String }
+/// let refs = MemRefStore::default();
+/// let objects = ObjectStore::default();
+/// let admin = Keypair::from_seed(1);
+/// enroll_member(&refs, &objects, "admin", &admin, Provenance::AdminRegistered, 100);
+///
+/// let tree = empty_tree(&objects);
+/// let reviewed = ents_testutil::write_commit(
+/// &objects,
+/// &CommitSpec { tree, parents: vec![], message: "reviewed work".into(), seconds: 200 },
+/// None,
+/// );
+///
+/// let identity = Identity {
+/// actor: gix::actor::Signature {
+/// name: "admin".into(),
+/// email: "admin@ents.test".into(),
+/// time: gix::date::Time { seconds: 300, offset: 0 },
+/// },
+/// sign: &|payload| admin.sign(payload),
+/// };
+///
+/// let outcome = propose_entity_with_pin(
+/// &refs, &objects, &NullEventSink,
+/// namespace::review_ref("7").expect("valid"), &Review { verdict: "approve".into() },
+/// namespace::review_pin_ref("7").expect("valid"), reviewed,
+/// &identity, "Review 7", "Pin review 7", Mode::Advisory,
+/// )
+/// .expect("reaches an outcome");
+/// assert_eq!(outcome.result, TxResult::Applied);
+/// // Both refs advanced together.
+/// assert!(refs.get(namespace::review_ref("7").expect("valid").as_ref()).expect("read").is_some());
+/// assert!(refs.get(namespace::review_pin_ref("7").expect("valid").as_ref()).expect("read").is_some());
+/// ```
+// @relation(receive.multi-ref-atomicity, model.review, model.review-pin, scope=function)
+#[expect(
+ clippy::too_many_arguments,
+ reason = "one field per ref this entity spans (entity name+value, pin name+retained commit) \
+ plus the shared identity/subjects/mode; the atomic counterpart of propose_entity \
+ and propose_pin, which carry the same justification"
+)]
+pub fn propose_entity_with_pin<T: for<'facet> facet::Facet<'facet>>(
+ refs: &dyn RefStore,
+ objects: &(impl Find + Write),
+ events: &dyn EventSink,
+ entity_name: FullName,
+ entity: &T,
+ pin_name: FullName,
+ retain: gix_hash::ObjectId,
+ identity: &Identity<'_>,
+ entity_subject: &str,
+ pin_subject: &str,
+ mode: Mode,
+) -> Result<Outcome> {
+ let (entity_transition, entity_tip) = entity_transition(
+ refs,
+ objects,
+ &entity_name,
+ entity,
+ identity,
+ entity_subject,
+ )?;
+ let (pin_transition, pin_tip) =
+ pin_transition(refs, objects, &pin_name, retain, identity, pin_subject)?;
+ let proposal = Proposal {
+ transitions: vec![entity_transition, pin_transition],
+ objects: vec![entity_tip, pin_tip],
auth: None,
};
crate::receive::receive(refs, objects, events, &proposal, mode)
crates/cli/ents-web/src/pages/commits.rs
@@ -489,7 +489,7 @@
verdict: form.verdict,
body: form.body,
};
- let (_id, entity_outcome, pin_outcome) = ents_forge::review::new(
+ let (_id, outcome) = ents_forge::review::new(
state.refs.as_ref(),
&*state.objects(),
state.events.as_ref(),
@@ -498,8 +498,7 @@
&crate::receive_identity!(identity),
state.mode,
)?;
- crate::error::outcome_to_result(entity_outcome)?;
- crate::error::outcome_to_result(pin_outcome)?;
+ crate::error::outcome_to_result(outcome)?;
Ok(Redirect::to(&format!("/commit/{oid}")))
}
crates/cli/git-ents/src/commands/review.rs
@@ -29,7 +29,7 @@
actor: actor(&signer),
sign: &|payload| signer.sign(payload),
};
- let (id, entity_outcome, pin_outcome) = review::new(
+ let (id, outcome) = review::new(
&root.refs,
&root.objects,
&root.events,
@@ -38,8 +38,7 @@
&identity,
root.mode(),
)?;
- outcome_to_result(entity_outcome, None)?;
- outcome_to_result(pin_outcome, None)?;
+ outcome_to_result(outcome, None)?;
Ok(id)
}
crates/forge/ents-forge/src/review/command.rs
@@ -10,7 +10,7 @@
//! composition root wires the concrete types and calls these functions,
//! never the other way around (`lens.parity`).
-use ents_receive::{Identity, Mode, Outcome, propose_entity, propose_pin};
+use ents_receive::{Identity, Mode, Outcome, propose_entity_with_pin};
use gix_hash::ObjectId;
use gix_object::{CommitRef, Find, Kind, Write};
use gix_ref_store::{RefStore, RefStoreRead};
@@ -93,20 +93,17 @@
/// ancestry) reachable (`model.review-pin`) — under one locally generated
/// id shared by both.
///
-/// The two refs are written as two separate proposals to [`crate::comment::add`]'s
-/// sibling primitives, [`propose_entity`] and [`propose_pin`]: `receive`
-/// applies one `Proposal`'s transitions atomically, but a `Proposal` is not
-/// itself parameterized to mix an entity-tree transition with an
-/// empty-tree pin transition in one call, so this command reaches two
-/// separate, sequential outcomes rather than one atomic batch — this is
-/// the two-proposals shape the model accepts (`model.review`,
-/// `model.review-pin`), not a gap to close.
+/// The two refs travel in one atomic mutation via
+/// [`propose_entity_with_pin`] (`receive.multi-ref-atomicity`): the
+/// ref-store's atomic multi-ref compare-and-swap admits or refuses both
+/// transitions together, so a review is never left with its entity written
+/// but its retention pin missing. One [`Outcome`] covers the whole batch.
///
/// # Errors
///
/// [`Error::InvalidArgument`] if `new.target` does not resolve to a commit;
/// otherwise propagates serialization or `receive` failures.
-// @relation(model.review, model.review-pin, lens.parity, scope=function)
+// @relation(model.review, model.review-pin, receive.multi-ref-atomicity, lens.parity, scope=function)
pub fn new(
refs: &dyn RefStore,
objects: &(impl Find + Write),
@@ -115,7 +112,7 @@
new: NewReview,
identity: &Identity<'_>,
mode: Mode,
-) -> Result<(String, Outcome, Outcome)> {
+) -> Result<(String, Outcome)> {
let reviewed = resolve_commit(repo_path, &new.target)?;
let review = Review::new(reviewed, new.verdict, new.body);
@@ -124,31 +121,21 @@
// mirroring `crate::comment::command::add`'s own locally generated id.
let id = uuid::Uuid::new_v4().simple().to_string();
- let entity_ref = ents_model::namespace::review_ref(&id)?;
- let entity_outcome = propose_entity(
+ let outcome = propose_entity_with_pin(
refs,
objects,
events,
- entity_ref,
+ ents_model::namespace::review_ref(&id)?,
&review,
- identity,
- &format!("Review {reviewed}"),
- mode,
- )?;
-
- let pin_ref = ents_model::namespace::review_pin_ref(&id)?;
- let pin_outcome = propose_pin(
- refs,
- objects,
- events,
- pin_ref,
+ ents_model::namespace::review_pin_ref(&id)?,
reviewed,
identity,
+ &format!("Review {reviewed}"),
&format!("Pin review {id}"),
mode,
)?;
- Ok((id, entity_outcome, pin_outcome))
+ Ok((id, outcome))
}
/// `git ents review list [--target rev]`: every review recorded in this