fix(merge-request): share immutable integration and requirement evidence
T-716: validate recorded source/approval/request identity, chronology and revocation independently of mutable selectors. Reuse exact revision and complete linked-source attestation in CompleteTicket. Assignment and lifecycle mutation are unchanged.\n\nValidation: cargo test -p merge-request (9 evidence and 21 store tests).
This commit is contained in:
+251
-68
@@ -236,10 +236,244 @@ pub struct MergeRequest {
|
|||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
pub thread: Vec<MergeRequestThreadEvent>,
|
pub thread: Vec<MergeRequestThreadEvent>,
|
||||||
}
|
}
|
||||||
|
/// Bounded failures from pure evaluation of persisted review and merge evidence.
|
||||||
|
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||||
|
pub enum MergeRequestEvidenceError {
|
||||||
|
NotMerged,
|
||||||
|
MergeResultMissing,
|
||||||
|
ApprovalMissing,
|
||||||
|
ApprovalRevoked,
|
||||||
|
ApprovalNotApproved,
|
||||||
|
ApprovalSourceMismatch,
|
||||||
|
ApprovalTimelineMismatch,
|
||||||
|
ReviewRequestMissing,
|
||||||
|
ReviewRequestMismatch,
|
||||||
|
ApprovalNotEffective,
|
||||||
|
ItemRevisionMismatch,
|
||||||
|
SourceSnapshotMismatch,
|
||||||
|
RequirementApprovalMissing,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl MergeRequestEvidenceError {
|
||||||
|
/// Stable machine-readable diagnostic code, separate from the human message.
|
||||||
|
pub fn code(self) -> &'static str {
|
||||||
|
match self {
|
||||||
|
Self::NotMerged => "not_merged",
|
||||||
|
Self::MergeResultMissing => "merge_result_missing",
|
||||||
|
Self::ApprovalMissing => "approval_missing",
|
||||||
|
Self::ApprovalRevoked => "approval_revoked",
|
||||||
|
Self::ApprovalNotApproved => "approval_not_approved",
|
||||||
|
Self::ApprovalSourceMismatch => "approval_source_mismatch",
|
||||||
|
Self::ApprovalTimelineMismatch => "approval_timeline_mismatch",
|
||||||
|
Self::ReviewRequestMissing => "review_request_missing",
|
||||||
|
Self::ReviewRequestMismatch => "review_request_mismatch",
|
||||||
|
Self::ApprovalNotEffective => "approval_not_effective",
|
||||||
|
Self::ItemRevisionMismatch => "item_revision_mismatch",
|
||||||
|
Self::SourceSnapshotMismatch => "source_snapshot_mismatch",
|
||||||
|
Self::RequirementApprovalMissing => "requirement_approval_missing",
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
pub fn as_str(self) -> &'static str {
|
||||||
|
match self {
|
||||||
|
Self::NotMerged => "Merge Request has no merged result",
|
||||||
|
Self::MergeResultMissing => "merged Merge Request has no MergeResult",
|
||||||
|
Self::ApprovalMissing => "integration approval is missing",
|
||||||
|
Self::ApprovalRevoked => "Reviewer approval is revoked",
|
||||||
|
Self::ApprovalNotApproved => "Reviewer decision is not an approval",
|
||||||
|
Self::ApprovalSourceMismatch => "Reviewer approval does not match the exact source ref",
|
||||||
|
Self::ApprovalTimelineMismatch => {
|
||||||
|
"integration approval does not predate the merged result"
|
||||||
|
}
|
||||||
|
Self::ReviewRequestMissing => "linked ReviewRequested evidence is missing",
|
||||||
|
Self::ReviewRequestMismatch => {
|
||||||
|
"Reviewer approval does not match linked ReviewRequested evidence"
|
||||||
|
}
|
||||||
|
Self::ApprovalNotEffective => "Reviewer approval is not the effective review",
|
||||||
|
Self::ItemRevisionMismatch => {
|
||||||
|
"Reviewer approval does not attest the current Ticket revision"
|
||||||
|
}
|
||||||
|
Self::SourceSnapshotMismatch => {
|
||||||
|
"Reviewer approval does not attest the exact linked source set"
|
||||||
|
}
|
||||||
|
Self::RequirementApprovalMissing => {
|
||||||
|
"no effective Reviewer approval attests the current Ticket revision and exact linked source set"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
impl std::fmt::Display for MergeRequestEvidenceError {
|
||||||
|
fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||||
|
formatter.write_str(self.as_str())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
impl std::error::Error for MergeRequestEvidenceError {}
|
||||||
|
|
||||||
impl MergeRequest {
|
impl MergeRequest {
|
||||||
pub fn effective_review(&self, subject: &str) -> Option<&ReviewEvent> {
|
pub fn effective_review(&self, subject: &str) -> Option<&ReviewEvent> {
|
||||||
self.thread.iter().rev().find_map(|e|match e{MergeRequestThreadEvent::Review(v)if v.subject_ref==subject&&!self.thread.iter().any(|x|matches!(x,MergeRequestThreadEvent::ReviewRevoked(r)if r.review_event_id==v.event_id))=>Some(v),_=>None})
|
self.thread.iter().rev().find_map(|e|match e{MergeRequestThreadEvent::Review(v)if v.subject_ref==subject&&!self.thread.iter().any(|x|matches!(x,MergeRequestThreadEvent::ReviewRevoked(r)if r.review_event_id==v.event_id))=>Some(v),_=>None})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Returns the recorded merged result, without resolving selectors or mutating state.
|
||||||
|
pub fn merged_result(&self) -> Result<&MergeEvent, MergeRequestEvidenceError> {
|
||||||
|
if self.state != MergeRequestState::Merged {
|
||||||
|
return Err(MergeRequestEvidenceError::NotMerged);
|
||||||
|
}
|
||||||
|
self.thread
|
||||||
|
.iter()
|
||||||
|
.rev()
|
||||||
|
.find_map(|event| match event {
|
||||||
|
MergeRequestThreadEvent::Merge(event) => Some(event),
|
||||||
|
_ => None,
|
||||||
|
})
|
||||||
|
.ok_or(MergeRequestEvidenceError::MergeResultMissing)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Validates the specific approval recorded by the merged result. A later
|
||||||
|
/// requirement review does not replace this integration evidence.
|
||||||
|
pub fn integration_approval(&self) -> Result<&ReviewEvent, MergeRequestEvidenceError> {
|
||||||
|
let merge = self.merged_result()?;
|
||||||
|
let review = self
|
||||||
|
.thread
|
||||||
|
.iter()
|
||||||
|
.find_map(|event| match event {
|
||||||
|
MergeRequestThreadEvent::Review(review)
|
||||||
|
if review.event_id == merge.approval_event_id =>
|
||||||
|
{
|
||||||
|
Some(review)
|
||||||
|
}
|
||||||
|
_ => None,
|
||||||
|
})
|
||||||
|
.ok_or(MergeRequestEvidenceError::ApprovalMissing)?;
|
||||||
|
if merge.approved_source_ref.trim().is_empty()
|
||||||
|
|| review.subject_ref != merge.approved_source_ref
|
||||||
|
{
|
||||||
|
return Err(MergeRequestEvidenceError::ApprovalSourceMismatch);
|
||||||
|
}
|
||||||
|
if review.sequence >= merge.sequence {
|
||||||
|
return Err(MergeRequestEvidenceError::ApprovalTimelineMismatch);
|
||||||
|
}
|
||||||
|
self.approval_evidence(review)?;
|
||||||
|
Ok(review)
|
||||||
|
}
|
||||||
|
|
||||||
|
fn approval_evidence(&self, review: &ReviewEvent) -> Result<(), MergeRequestEvidenceError> {
|
||||||
|
if self.thread.iter().any(|event| {
|
||||||
|
matches!(event, MergeRequestThreadEvent::ReviewRevoked(revoked)
|
||||||
|
if revoked.review_event_id == review.event_id)
|
||||||
|
}) {
|
||||||
|
return Err(MergeRequestEvidenceError::ApprovalRevoked);
|
||||||
|
}
|
||||||
|
if review.decision != ReviewDecision::Approve {
|
||||||
|
return Err(MergeRequestEvidenceError::ApprovalNotApproved);
|
||||||
|
}
|
||||||
|
let requested = self
|
||||||
|
.thread
|
||||||
|
.iter()
|
||||||
|
.find_map(|event| match event {
|
||||||
|
MergeRequestThreadEvent::ReviewRequested(requested)
|
||||||
|
if requested.event_id == review.request_event_id =>
|
||||||
|
{
|
||||||
|
Some(requested)
|
||||||
|
}
|
||||||
|
_ => None,
|
||||||
|
})
|
||||||
|
.ok_or(MergeRequestEvidenceError::ReviewRequestMissing)?;
|
||||||
|
if requested.subject_ref != review.subject_ref
|
||||||
|
|| requested.ticket_item_revision != review.ticket_item_revision
|
||||||
|
|| requested.ticket_merge_request_subjects != review.ticket_merge_request_subjects
|
||||||
|
|| requested.reviewer != review.reviewer
|
||||||
|
|| requested.sequence >= review.sequence
|
||||||
|
{
|
||||||
|
return Err(MergeRequestEvidenceError::ReviewRequestMismatch);
|
||||||
|
}
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Finds an effective approval of the authoritative current Ticket revision and
|
||||||
|
/// exact linked source snapshot. Callers supply all linked requests and resolved
|
||||||
|
/// source refs (recorded merged refs for completion); this function does not read
|
||||||
|
/// authority, resolve selectors, require merged state, or mutate lifecycle state.
|
||||||
|
/// An explicit event id validates that approval only; `None` discovers any valid
|
||||||
|
/// attestation. Snapshot ordering is immaterial, but duplicate ids are rejected.
|
||||||
|
pub fn requirement_approval<'a>(
|
||||||
|
requests: &'a [MergeRequest],
|
||||||
|
item_revision: &str,
|
||||||
|
subjects: &[MergeRequestReviewSubject],
|
||||||
|
event_id: Option<&str>,
|
||||||
|
) -> Result<&'a ReviewEvent, MergeRequestEvidenceError> {
|
||||||
|
if requests.len() != subjects.len()
|
||||||
|
|| !same_source_snapshot(subjects, subjects)
|
||||||
|
|| requests.iter().any(|request| {
|
||||||
|
requests
|
||||||
|
.iter()
|
||||||
|
.filter(|other| other.merge_request_id == request.merge_request_id)
|
||||||
|
.count()
|
||||||
|
!= 1
|
||||||
|
|| !subjects
|
||||||
|
.iter()
|
||||||
|
.any(|subject| subject.merge_request_id == request.merge_request_id)
|
||||||
|
})
|
||||||
|
{
|
||||||
|
return Err(MergeRequestEvidenceError::SourceSnapshotMismatch);
|
||||||
|
}
|
||||||
|
for request in requests {
|
||||||
|
for event in request.thread.iter().rev() {
|
||||||
|
let MergeRequestThreadEvent::Review(review) = event else {
|
||||||
|
continue;
|
||||||
|
};
|
||||||
|
if event_id.is_some_and(|id| id != review.event_id) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
let evidence = (|| {
|
||||||
|
request.approval_evidence(review)?;
|
||||||
|
if !request
|
||||||
|
.effective_review(&review.subject_ref)
|
||||||
|
.is_some_and(|effective| effective.event_id == review.event_id)
|
||||||
|
{
|
||||||
|
return Err(MergeRequestEvidenceError::ApprovalNotEffective);
|
||||||
|
}
|
||||||
|
if review.ticket_item_revision != item_revision || item_revision.trim().is_empty() {
|
||||||
|
return Err(MergeRequestEvidenceError::ItemRevisionMismatch);
|
||||||
|
}
|
||||||
|
if !same_source_snapshot(&review.ticket_merge_request_subjects, subjects) {
|
||||||
|
return Err(MergeRequestEvidenceError::SourceSnapshotMismatch);
|
||||||
|
}
|
||||||
|
if !subjects.iter().any(|subject| {
|
||||||
|
subject.merge_request_id == request.merge_request_id
|
||||||
|
&& subject.subject_ref == review.subject_ref
|
||||||
|
}) {
|
||||||
|
return Err(MergeRequestEvidenceError::ApprovalSourceMismatch);
|
||||||
|
}
|
||||||
|
Ok(review)
|
||||||
|
})();
|
||||||
|
if evidence.is_ok() || event_id.is_some() {
|
||||||
|
return evidence;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
Err(MergeRequestEvidenceError::RequirementApprovalMissing)
|
||||||
|
}
|
||||||
|
|
||||||
|
fn same_source_snapshot(
|
||||||
|
left: &[MergeRequestReviewSubject],
|
||||||
|
right: &[MergeRequestReviewSubject],
|
||||||
|
) -> bool {
|
||||||
|
!left.is_empty()
|
||||||
|
&& left.len() == right.len()
|
||||||
|
&& left.iter().all(|subject| {
|
||||||
|
!subject.merge_request_id.trim().is_empty()
|
||||||
|
&& !subject.subject_ref.trim().is_empty()
|
||||||
|
&& left
|
||||||
|
.iter()
|
||||||
|
.filter(|other| other.merge_request_id == subject.merge_request_id)
|
||||||
|
.count()
|
||||||
|
== 1
|
||||||
|
&& right.iter().filter(|other| *other == subject).count() == 1
|
||||||
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Clone, Debug, Default)]
|
#[derive(Clone, Debug, Default)]
|
||||||
@@ -1201,81 +1435,30 @@ impl MergeRequestStore {
|
|||||||
for merge_request_id in &actual_ids {
|
for merge_request_id in &actual_ids {
|
||||||
let request = load_mr(&transaction, &input.auth.workspace_id, merge_request_id)?
|
let request = load_mr(&transaction, &input.auth.workspace_id, merge_request_id)?
|
||||||
.ok_or(MergeRequestError::NotFound)?;
|
.ok_or(MergeRequestError::NotFound)?;
|
||||||
if request.state != MergeRequestState::Merged {
|
request.integration_approval().map_err(|error| {
|
||||||
return Err(MergeRequestError::NotReady(format!(
|
let message = format!("Merge Request `{merge_request_id}`: {}", error.as_str());
|
||||||
"Merge Request `{merge_request_id}` has no merged result"
|
if error == MergeRequestEvidenceError::MergeResultMissing {
|
||||||
)));
|
MergeRequestError::Corrupt(message)
|
||||||
}
|
} else {
|
||||||
|
MergeRequestError::NotReady(message)
|
||||||
|
}
|
||||||
|
})?;
|
||||||
let merge = request
|
let merge = request
|
||||||
.thread
|
.merged_result()
|
||||||
.iter()
|
.map_err(|error| MergeRequestError::Corrupt(error.as_str().into()))?;
|
||||||
.rev()
|
|
||||||
.find_map(|event| match event {
|
|
||||||
MergeRequestThreadEvent::Merge(event) => Some(event),
|
|
||||||
_ => None,
|
|
||||||
})
|
|
||||||
.ok_or_else(|| {
|
|
||||||
MergeRequestError::Corrupt(format!(
|
|
||||||
"merged Merge Request `{merge_request_id}` has no MergeResult"
|
|
||||||
))
|
|
||||||
})?;
|
|
||||||
let review = request
|
|
||||||
.thread
|
|
||||||
.iter()
|
|
||||||
.find_map(|event| match event {
|
|
||||||
MergeRequestThreadEvent::Review(review)
|
|
||||||
if review.event_id == merge.approval_event_id
|
|
||||||
&& review.subject_ref == merge.approved_source_ref =>
|
|
||||||
{
|
|
||||||
Some(review)
|
|
||||||
}
|
|
||||||
_ => None,
|
|
||||||
})
|
|
||||||
.filter(|review| {
|
|
||||||
!request.thread.iter().any(|event| {
|
|
||||||
matches!(
|
|
||||||
event,
|
|
||||||
MergeRequestThreadEvent::ReviewRevoked(revoked)
|
|
||||||
if revoked.review_event_id == review.event_id
|
|
||||||
)
|
|
||||||
})
|
|
||||||
})
|
|
||||||
.ok_or_else(|| {
|
|
||||||
MergeRequestError::NotReady(format!(
|
|
||||||
"Merge Request `{merge_request_id}` integration approval is missing or revoked"
|
|
||||||
))
|
|
||||||
})?;
|
|
||||||
if review.decision != ReviewDecision::Approve {
|
|
||||||
return Err(MergeRequestError::NotReady(format!(
|
|
||||||
"Merge Request `{merge_request_id}` lacks an approved integration result"
|
|
||||||
)));
|
|
||||||
}
|
|
||||||
merged_subjects.push(MergeRequestReviewSubject {
|
merged_subjects.push(MergeRequestReviewSubject {
|
||||||
merge_request_id: merge_request_id.clone(),
|
merge_request_id: merge_request_id.clone(),
|
||||||
subject_ref: merge.approved_source_ref.clone(),
|
subject_ref: merge.approved_source_ref.clone(),
|
||||||
});
|
});
|
||||||
requests.push(request);
|
requests.push(request);
|
||||||
}
|
}
|
||||||
let has_requirement_attestation = requests.iter().any(|request| {
|
requirement_approval(
|
||||||
request.thread.iter().rev().any(|event| {
|
&requests,
|
||||||
let MergeRequestThreadEvent::Review(review) = event else {
|
&input.item_revision,
|
||||||
return false;
|
&merged_subjects,
|
||||||
};
|
Some(&input.requirement_approval_event_id),
|
||||||
review.event_id == input.requirement_approval_event_id
|
)
|
||||||
&& review.decision == ReviewDecision::Approve
|
.map_err(|error| MergeRequestError::NotReady(error.as_str().into()))?;
|
||||||
&& review.ticket_item_revision == input.item_revision
|
|
||||||
&& review.ticket_merge_request_subjects == merged_subjects
|
|
||||||
&& request
|
|
||||||
.effective_review(&review.subject_ref)
|
|
||||||
.is_some_and(|effective| effective.event_id == review.event_id)
|
|
||||||
})
|
|
||||||
});
|
|
||||||
if !has_requirement_attestation {
|
|
||||||
return Err(MergeRequestError::NotReady(
|
|
||||||
"no effective Reviewer approval attests the current Ticket revision and exact merged source set"
|
|
||||||
.into(),
|
|
||||||
));
|
|
||||||
}
|
|
||||||
let updated = transaction.execute(
|
let updated = transaction.execute(
|
||||||
"UPDATE typed_tickets
|
"UPDATE typed_tickets
|
||||||
SET workflow_state='done',workflow_state_explicit=1,updated_at=?3
|
SET workflow_state='done',workflow_state_explicit=1,updated_at=?3
|
||||||
|
|||||||
@@ -0,0 +1,459 @@
|
|||||||
|
use chrono::{TimeZone, Utc};
|
||||||
|
use merge_request::*;
|
||||||
|
|
||||||
|
fn actor() -> WorkerIdentity {
|
||||||
|
WorkerIdentity {
|
||||||
|
runtime_id: "runtime".into(),
|
||||||
|
worker_id: "reviewer".into(),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn subjects() -> Vec<MergeRequestReviewSubject> {
|
||||||
|
vec![
|
||||||
|
MergeRequestReviewSubject {
|
||||||
|
merge_request_id: "MR-1".into(),
|
||||||
|
subject_ref: "source-one".into(),
|
||||||
|
},
|
||||||
|
MergeRequestReviewSubject {
|
||||||
|
merge_request_id: "MR-2".into(),
|
||||||
|
subject_ref: "source-two".into(),
|
||||||
|
},
|
||||||
|
]
|
||||||
|
}
|
||||||
|
|
||||||
|
fn add_review(
|
||||||
|
request: &mut MergeRequest,
|
||||||
|
event_id: &str,
|
||||||
|
revision: &str,
|
||||||
|
snapshot: &[MergeRequestReviewSubject],
|
||||||
|
decision: ReviewDecision,
|
||||||
|
) {
|
||||||
|
let subject = snapshot
|
||||||
|
.iter()
|
||||||
|
.find(|subject| subject.merge_request_id == request.merge_request_id)
|
||||||
|
.unwrap();
|
||||||
|
let sequence = request.thread.len() as u64 + 1;
|
||||||
|
let requested = ReviewRequestedEvent {
|
||||||
|
event_id: format!("request-{event_id}"),
|
||||||
|
sequence,
|
||||||
|
subject_ref: subject.subject_ref.clone(),
|
||||||
|
ticket_item_revision: revision.into(),
|
||||||
|
ticket_merge_request_subjects: snapshot.to_vec(),
|
||||||
|
requested_by: actor(),
|
||||||
|
reviewer: actor(),
|
||||||
|
created_at: request.created_at,
|
||||||
|
};
|
||||||
|
let review = ReviewEvent {
|
||||||
|
event_id: event_id.into(),
|
||||||
|
sequence: sequence + 1,
|
||||||
|
request_event_id: requested.event_id.clone(),
|
||||||
|
subject_ref: requested.subject_ref.clone(),
|
||||||
|
ticket_item_revision: revision.into(),
|
||||||
|
ticket_merge_request_subjects: snapshot.to_vec(),
|
||||||
|
decision,
|
||||||
|
body: String::new(),
|
||||||
|
findings: vec![],
|
||||||
|
reviewer: actor(),
|
||||||
|
created_at: request.created_at,
|
||||||
|
};
|
||||||
|
request
|
||||||
|
.thread
|
||||||
|
.push(MergeRequestThreadEvent::ReviewRequested(requested));
|
||||||
|
request.thread.push(MergeRequestThreadEvent::Review(review));
|
||||||
|
}
|
||||||
|
|
||||||
|
fn merged_requests() -> Vec<MergeRequest> {
|
||||||
|
let now = Utc.with_ymd_and_hms(2026, 10, 8, 12, 0, 0).unwrap();
|
||||||
|
let snapshot = subjects();
|
||||||
|
snapshot
|
||||||
|
.iter()
|
||||||
|
.map(|subject| {
|
||||||
|
let mut request = MergeRequest {
|
||||||
|
workspace_id: "W".into(),
|
||||||
|
merge_request_id: subject.merge_request_id.clone(),
|
||||||
|
repository_id: subject.merge_request_id.clone(),
|
||||||
|
state: MergeRequestState::Merged,
|
||||||
|
selector_from: Some("work/source".into()),
|
||||||
|
selector_to: "main".into(),
|
||||||
|
ticket_ids: vec!["T".into()],
|
||||||
|
created_at: now,
|
||||||
|
updated_at: now,
|
||||||
|
thread: vec![],
|
||||||
|
};
|
||||||
|
let approval_id = format!("integration-{}", subject.merge_request_id);
|
||||||
|
add_review(
|
||||||
|
&mut request,
|
||||||
|
&approval_id,
|
||||||
|
"revision-one",
|
||||||
|
&snapshot,
|
||||||
|
ReviewDecision::Approve,
|
||||||
|
);
|
||||||
|
request
|
||||||
|
.thread
|
||||||
|
.push(MergeRequestThreadEvent::Merge(MergeEvent {
|
||||||
|
event_id: format!("merge-{}", subject.merge_request_id),
|
||||||
|
sequence: 3,
|
||||||
|
operation_id: format!("operation-{}", subject.merge_request_id),
|
||||||
|
approval_event_id: approval_id,
|
||||||
|
approved_source_ref: subject.subject_ref.clone(),
|
||||||
|
target_ref_before: "target-before".into(),
|
||||||
|
target_ref_after: "target-after".into(),
|
||||||
|
strategy: MergeStrategy::FastForward,
|
||||||
|
resolution: ConflictResolution::None,
|
||||||
|
merged_by: actor(),
|
||||||
|
created_at: now,
|
||||||
|
}));
|
||||||
|
request
|
||||||
|
})
|
||||||
|
.collect()
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn postmerge_latest_approval_attests_the_current_revision_without_replacing_integration() {
|
||||||
|
let mut requests = merged_requests();
|
||||||
|
let snapshot = subjects();
|
||||||
|
assert_eq!(
|
||||||
|
requirement_approval(
|
||||||
|
&requests,
|
||||||
|
"revision-two",
|
||||||
|
&snapshot,
|
||||||
|
Some("integration-MR-1")
|
||||||
|
),
|
||||||
|
Err(MergeRequestEvidenceError::ItemRevisionMismatch)
|
||||||
|
);
|
||||||
|
add_review(
|
||||||
|
&mut requests[0],
|
||||||
|
"postmerge",
|
||||||
|
"revision-two",
|
||||||
|
&snapshot,
|
||||||
|
ReviewDecision::Approve,
|
||||||
|
);
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
requests[0].integration_approval().unwrap().event_id,
|
||||||
|
"integration-MR-1"
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
requirement_approval(&requests, "revision-two", &snapshot, None)
|
||||||
|
.unwrap()
|
||||||
|
.event_id,
|
||||||
|
"postmerge"
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
requirement_approval(&requests, "revision-two", &snapshot, Some("postmerge"))
|
||||||
|
.unwrap()
|
||||||
|
.event_id,
|
||||||
|
"postmerge"
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
requirement_approval(
|
||||||
|
&requests,
|
||||||
|
"revision-one",
|
||||||
|
&snapshot,
|
||||||
|
Some("integration-MR-1")
|
||||||
|
),
|
||||||
|
Err(MergeRequestEvidenceError::ApprovalNotEffective)
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
requirement_approval(&requests, "revision-two", &snapshot, Some("unknown")),
|
||||||
|
Err(MergeRequestEvidenceError::RequirementApprovalMissing)
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn later_request_changes_prevents_older_requirement_approval_from_being_effective() {
|
||||||
|
let mut requests = merged_requests();
|
||||||
|
let snapshot = subjects();
|
||||||
|
add_review(
|
||||||
|
&mut requests[0],
|
||||||
|
"approved",
|
||||||
|
"revision-two",
|
||||||
|
&snapshot,
|
||||||
|
ReviewDecision::Approve,
|
||||||
|
);
|
||||||
|
add_review(
|
||||||
|
&mut requests[0],
|
||||||
|
"changes",
|
||||||
|
"revision-two",
|
||||||
|
&snapshot,
|
||||||
|
ReviewDecision::RequestChanges,
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
requirement_approval(&requests, "revision-two", &snapshot, None),
|
||||||
|
Err(MergeRequestEvidenceError::RequirementApprovalMissing)
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
requirement_approval(&requests, "revision-two", &snapshot, Some("approved")),
|
||||||
|
Err(MergeRequestEvidenceError::ApprovalNotEffective)
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
requirement_approval(&requests, "revision-two", &snapshot, Some("changes")),
|
||||||
|
Err(MergeRequestEvidenceError::ApprovalNotApproved)
|
||||||
|
);
|
||||||
|
assert!(requests[0].integration_approval().is_ok());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn changed_revision_or_any_linked_source_invalidates_a_multiple_mr_attestation() {
|
||||||
|
let requests = merged_requests();
|
||||||
|
let original = subjects();
|
||||||
|
assert!(requirement_approval(&requests, "revision-one", &original, None).is_ok());
|
||||||
|
assert_eq!(
|
||||||
|
requirement_approval(
|
||||||
|
&requests,
|
||||||
|
"revision-two",
|
||||||
|
&original,
|
||||||
|
Some("integration-MR-1")
|
||||||
|
),
|
||||||
|
Err(MergeRequestEvidenceError::ItemRevisionMismatch)
|
||||||
|
);
|
||||||
|
let mut changed = original.clone();
|
||||||
|
changed[1].subject_ref = "changed-source-two".into();
|
||||||
|
assert_eq!(
|
||||||
|
requirement_approval(
|
||||||
|
&requests,
|
||||||
|
"revision-one",
|
||||||
|
&changed,
|
||||||
|
Some("integration-MR-1")
|
||||||
|
),
|
||||||
|
Err(MergeRequestEvidenceError::SourceSnapshotMismatch)
|
||||||
|
);
|
||||||
|
let mut reversed = original.clone();
|
||||||
|
reversed.reverse();
|
||||||
|
assert!(requirement_approval(&requests, "revision-one", &reversed, None).is_ok());
|
||||||
|
|
||||||
|
// Even after the caller refreshes the linked request collection, an old
|
||||||
|
// attestation cannot authorize completion against a smaller linked set.
|
||||||
|
assert_eq!(
|
||||||
|
requirement_approval(
|
||||||
|
&requests[..1],
|
||||||
|
"revision-one",
|
||||||
|
&original[..1],
|
||||||
|
Some("integration-MR-1")
|
||||||
|
),
|
||||||
|
Err(MergeRequestEvidenceError::SourceSnapshotMismatch)
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
requirement_approval(&requests, "revision-one", &original[..1], None),
|
||||||
|
Err(MergeRequestEvidenceError::SourceSnapshotMismatch)
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
requirement_approval(
|
||||||
|
&requests,
|
||||||
|
"revision-one",
|
||||||
|
&[original[0].clone(), original[0].clone()],
|
||||||
|
None
|
||||||
|
),
|
||||||
|
Err(MergeRequestEvidenceError::SourceSnapshotMismatch)
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn refreshed_postmerge_approval_can_attest_a_changed_multiple_mr_set_and_revision() {
|
||||||
|
let mut requests = merged_requests();
|
||||||
|
let mut snapshot = subjects();
|
||||||
|
snapshot.pop();
|
||||||
|
requests.pop();
|
||||||
|
assert_eq!(
|
||||||
|
requirement_approval(&requests, "revision-two", &snapshot, None),
|
||||||
|
Err(MergeRequestEvidenceError::RequirementApprovalMissing)
|
||||||
|
);
|
||||||
|
add_review(
|
||||||
|
&mut requests[0],
|
||||||
|
"refreshed-set",
|
||||||
|
"revision-two",
|
||||||
|
&snapshot,
|
||||||
|
ReviewDecision::Approve,
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
requirement_approval(&requests, "revision-two", &snapshot, None)
|
||||||
|
.unwrap()
|
||||||
|
.event_id,
|
||||||
|
"refreshed-set"
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
requests[0].integration_approval().unwrap().event_id,
|
||||||
|
"integration-MR-1"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn requirement_approval_must_bind_its_own_mr_source_and_corresponding_request() {
|
||||||
|
let original = subjects();
|
||||||
|
for (case, expected) in [
|
||||||
|
("missing", MergeRequestEvidenceError::ReviewRequestMissing),
|
||||||
|
("revision", MergeRequestEvidenceError::ReviewRequestMismatch),
|
||||||
|
("snapshot", MergeRequestEvidenceError::ReviewRequestMismatch),
|
||||||
|
("reviewer", MergeRequestEvidenceError::ReviewRequestMismatch),
|
||||||
|
("source", MergeRequestEvidenceError::ApprovalSourceMismatch),
|
||||||
|
] {
|
||||||
|
let mut requests = merged_requests();
|
||||||
|
for event in &mut requests[0].thread {
|
||||||
|
match event {
|
||||||
|
MergeRequestThreadEvent::Review(review) if case == "missing" => {
|
||||||
|
review.request_event_id = "absent-request".into();
|
||||||
|
}
|
||||||
|
MergeRequestThreadEvent::ReviewRequested(requested) => match case {
|
||||||
|
"revision" => requested.ticket_item_revision = "other-revision".into(),
|
||||||
|
"snapshot" => {
|
||||||
|
requested.ticket_merge_request_subjects.pop();
|
||||||
|
}
|
||||||
|
"reviewer" => requested.reviewer.worker_id = "other-reviewer".into(),
|
||||||
|
"source" => requested.subject_ref = "unbound-source".into(),
|
||||||
|
_ => {}
|
||||||
|
},
|
||||||
|
MergeRequestThreadEvent::Review(review) if case == "source" => {
|
||||||
|
review.subject_ref = "unbound-source".into();
|
||||||
|
}
|
||||||
|
_ => {}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
assert_eq!(
|
||||||
|
requirement_approval(
|
||||||
|
&requests,
|
||||||
|
"revision-one",
|
||||||
|
&original,
|
||||||
|
Some("integration-MR-1")
|
||||||
|
),
|
||||||
|
Err(expected),
|
||||||
|
"{case}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn revoked_requirement_approval_is_rejected_and_discovery_uses_another_valid_mr() {
|
||||||
|
let mut requests = merged_requests();
|
||||||
|
let created_at = requests[0].created_at;
|
||||||
|
requests[0]
|
||||||
|
.thread
|
||||||
|
.push(MergeRequestThreadEvent::ReviewRevoked(ReviewRevokedEvent {
|
||||||
|
event_id: "revocation".into(),
|
||||||
|
sequence: 4,
|
||||||
|
review_event_id: "integration-MR-1".into(),
|
||||||
|
subject_ref: "source-one".into(),
|
||||||
|
reason: String::new(),
|
||||||
|
revoked_by: actor(),
|
||||||
|
created_at,
|
||||||
|
}));
|
||||||
|
assert_eq!(
|
||||||
|
requirement_approval(
|
||||||
|
&requests,
|
||||||
|
"revision-one",
|
||||||
|
&subjects(),
|
||||||
|
Some("integration-MR-1")
|
||||||
|
),
|
||||||
|
Err(MergeRequestEvidenceError::ApprovalRevoked)
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
requirement_approval(&requests, "revision-one", &subjects(), None)
|
||||||
|
.unwrap()
|
||||||
|
.event_id,
|
||||||
|
"integration-MR-2"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn integration_approval_must_strictly_predate_the_merge_sequence() {
|
||||||
|
for approval_sequence in [2, 3, 4] {
|
||||||
|
let mut requests = merged_requests();
|
||||||
|
for event in &mut requests[0].thread {
|
||||||
|
if let MergeRequestThreadEvent::Review(review) = event {
|
||||||
|
review.sequence = approval_sequence;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
let result = requests[0].integration_approval();
|
||||||
|
if approval_sequence < 3 {
|
||||||
|
assert!(result.is_ok());
|
||||||
|
} else {
|
||||||
|
assert_eq!(
|
||||||
|
result,
|
||||||
|
Err(MergeRequestEvidenceError::ApprovalTimelineMismatch),
|
||||||
|
"approval sequence {approval_sequence}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn blank_saved_source_is_not_valid_integration_evidence_even_when_the_review_matches() {
|
||||||
|
for source in ["", " \t\n"] {
|
||||||
|
let mut requests = merged_requests();
|
||||||
|
for event in &mut requests[0].thread {
|
||||||
|
match event {
|
||||||
|
MergeRequestThreadEvent::ReviewRequested(requested) => {
|
||||||
|
requested.subject_ref = source.into();
|
||||||
|
}
|
||||||
|
MergeRequestThreadEvent::Review(review) => {
|
||||||
|
review.subject_ref = source.into();
|
||||||
|
}
|
||||||
|
MergeRequestThreadEvent::Merge(merge) => {
|
||||||
|
merge.approved_source_ref = source.into();
|
||||||
|
}
|
||||||
|
_ => {}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
assert_eq!(
|
||||||
|
requests[0].integration_approval(),
|
||||||
|
Err(MergeRequestEvidenceError::ApprovalSourceMismatch)
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn evidence_error_codes_are_stable_machine_diagnostics_separate_from_messages() {
|
||||||
|
for (error, code) in [
|
||||||
|
(MergeRequestEvidenceError::NotMerged, "not_merged"),
|
||||||
|
(
|
||||||
|
MergeRequestEvidenceError::MergeResultMissing,
|
||||||
|
"merge_result_missing",
|
||||||
|
),
|
||||||
|
(
|
||||||
|
MergeRequestEvidenceError::ApprovalMissing,
|
||||||
|
"approval_missing",
|
||||||
|
),
|
||||||
|
(
|
||||||
|
MergeRequestEvidenceError::ApprovalRevoked,
|
||||||
|
"approval_revoked",
|
||||||
|
),
|
||||||
|
(
|
||||||
|
MergeRequestEvidenceError::ApprovalNotApproved,
|
||||||
|
"approval_not_approved",
|
||||||
|
),
|
||||||
|
(
|
||||||
|
MergeRequestEvidenceError::ApprovalSourceMismatch,
|
||||||
|
"approval_source_mismatch",
|
||||||
|
),
|
||||||
|
(
|
||||||
|
MergeRequestEvidenceError::ApprovalTimelineMismatch,
|
||||||
|
"approval_timeline_mismatch",
|
||||||
|
),
|
||||||
|
(
|
||||||
|
MergeRequestEvidenceError::ReviewRequestMissing,
|
||||||
|
"review_request_missing",
|
||||||
|
),
|
||||||
|
(
|
||||||
|
MergeRequestEvidenceError::ReviewRequestMismatch,
|
||||||
|
"review_request_mismatch",
|
||||||
|
),
|
||||||
|
(
|
||||||
|
MergeRequestEvidenceError::ApprovalNotEffective,
|
||||||
|
"approval_not_effective",
|
||||||
|
),
|
||||||
|
(
|
||||||
|
MergeRequestEvidenceError::ItemRevisionMismatch,
|
||||||
|
"item_revision_mismatch",
|
||||||
|
),
|
||||||
|
(
|
||||||
|
MergeRequestEvidenceError::SourceSnapshotMismatch,
|
||||||
|
"source_snapshot_mismatch",
|
||||||
|
),
|
||||||
|
(
|
||||||
|
MergeRequestEvidenceError::RequirementApprovalMissing,
|
||||||
|
"requirement_approval_missing",
|
||||||
|
),
|
||||||
|
] {
|
||||||
|
assert_eq!(error.code(), code);
|
||||||
|
assert_ne!(error.code(), error.as_str());
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -1098,3 +1098,143 @@ fn partial_integration_retains_ticket_and_assignment_until_guarded_ticket_comple
|
|||||||
.unwrap();
|
.unwrap();
|
||||||
assert_eq!(replay, completed);
|
assert_eq!(replay, completed);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn persisted_merged_result_requires_matching_nonrevoked_requested_approval_before_completion() {
|
||||||
|
for (case, expected) in [
|
||||||
|
(
|
||||||
|
"merge-missing",
|
||||||
|
MergeRequestEvidenceError::MergeResultMissing,
|
||||||
|
),
|
||||||
|
(
|
||||||
|
"approval-missing",
|
||||||
|
MergeRequestEvidenceError::ApprovalMissing,
|
||||||
|
),
|
||||||
|
("revoked", MergeRequestEvidenceError::ApprovalRevoked),
|
||||||
|
(
|
||||||
|
"source-mismatch",
|
||||||
|
MergeRequestEvidenceError::ApprovalSourceMismatch,
|
||||||
|
),
|
||||||
|
(
|
||||||
|
"not-approved",
|
||||||
|
MergeRequestEvidenceError::ApprovalNotApproved,
|
||||||
|
),
|
||||||
|
(
|
||||||
|
"request-missing",
|
||||||
|
MergeRequestEvidenceError::ReviewRequestMissing,
|
||||||
|
),
|
||||||
|
(
|
||||||
|
"request-mismatch",
|
||||||
|
MergeRequestEvidenceError::ReviewRequestMismatch,
|
||||||
|
),
|
||||||
|
(
|
||||||
|
"approval-after-merge",
|
||||||
|
MergeRequestEvidenceError::ApprovalTimelineMismatch,
|
||||||
|
),
|
||||||
|
] {
|
||||||
|
let (dir, store) = fixture();
|
||||||
|
open(&store);
|
||||||
|
let review = approve(&store, "merged-source", "integration-token");
|
||||||
|
let merge = store
|
||||||
|
.complete(CompleteMergeRequest {
|
||||||
|
merge_request_id: "MR".into(),
|
||||||
|
ticket_id: "T".into(),
|
||||||
|
operation_id: "merge".into(),
|
||||||
|
approval_event_id: review.event_id.clone(),
|
||||||
|
current_subject_ref: "merged-source".into(),
|
||||||
|
target_ref_before: "before".into(),
|
||||||
|
target_ref_after: "after".into(),
|
||||||
|
strategy: MergeStrategy::FastForward,
|
||||||
|
resolution: ConflictResolution::None,
|
||||||
|
auth: auth(),
|
||||||
|
now: at(5),
|
||||||
|
})
|
||||||
|
.unwrap();
|
||||||
|
let persisted = store.get_by_id("W", "MR").unwrap();
|
||||||
|
assert_eq!(persisted.merged_result().unwrap(), &merge);
|
||||||
|
assert_eq!(persisted.integration_approval().unwrap(), &review);
|
||||||
|
|
||||||
|
let connection = Connection::open(dir.path().join("db")).unwrap();
|
||||||
|
match case {
|
||||||
|
"merge-missing" | "approval-missing" | "request-missing" => {
|
||||||
|
let kind = match case {
|
||||||
|
"merge-missing" => "merge",
|
||||||
|
"approval-missing" => "review",
|
||||||
|
_ => "review_requested",
|
||||||
|
};
|
||||||
|
connection.execute(
|
||||||
|
"DELETE FROM merge_request_thread_events WHERE workspace_id='W' AND merge_request_id='MR' AND kind=?1",
|
||||||
|
[kind],
|
||||||
|
).unwrap();
|
||||||
|
}
|
||||||
|
"revoked" => {
|
||||||
|
store
|
||||||
|
.revoke_review(RevokeMergeRequestReview {
|
||||||
|
merge_request_id: "MR".into(),
|
||||||
|
ticket_id: "T".into(),
|
||||||
|
review_event_id: review.event_id.clone(),
|
||||||
|
reason: "invalid integration evidence".into(),
|
||||||
|
auth: auth(),
|
||||||
|
now: at(6),
|
||||||
|
})
|
||||||
|
.unwrap();
|
||||||
|
}
|
||||||
|
"approval-after-merge" => {
|
||||||
|
let mut altered = review.clone();
|
||||||
|
altered.sequence = merge.sequence + 1;
|
||||||
|
connection.execute(
|
||||||
|
"UPDATE merge_request_thread_events SET payload_json=?1,sequence=?2 WHERE workspace_id='W' AND merge_request_id='MR' AND kind='review'",
|
||||||
|
rusqlite::params![serde_json::to_string(&altered).unwrap(), altered.sequence],
|
||||||
|
).unwrap();
|
||||||
|
}
|
||||||
|
_ => {
|
||||||
|
let mut altered = review.clone();
|
||||||
|
match case {
|
||||||
|
"source-mismatch" => altered.subject_ref = "different-source".into(),
|
||||||
|
"not-approved" => altered.decision = ReviewDecision::RequestChanges,
|
||||||
|
"request-mismatch" => {
|
||||||
|
altered.ticket_item_revision = "different-revision".into()
|
||||||
|
}
|
||||||
|
_ => unreachable!(),
|
||||||
|
}
|
||||||
|
connection.execute(
|
||||||
|
"UPDATE merge_request_thread_events SET payload_json=?1 WHERE workspace_id='W' AND merge_request_id='MR' AND kind='review'",
|
||||||
|
[serde_json::to_string(&altered).unwrap()],
|
||||||
|
).unwrap();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
let reloaded = store.get_by_id("W", "MR").unwrap();
|
||||||
|
assert_eq!(reloaded.integration_approval(), Err(expected), "{case}");
|
||||||
|
let rejected = store
|
||||||
|
.complete_ticket(CompleteTicket {
|
||||||
|
ticket_id: "T".into(),
|
||||||
|
operation_id: "complete-ticket".into(),
|
||||||
|
item_revision: "t".into(),
|
||||||
|
merge_request_ids: vec!["MR".into()],
|
||||||
|
requirement_approval_event_id: review.event_id.clone(),
|
||||||
|
auth: auth(),
|
||||||
|
now: at(7),
|
||||||
|
})
|
||||||
|
.unwrap_err();
|
||||||
|
match rejected {
|
||||||
|
MergeRequestError::Corrupt(message)
|
||||||
|
if expected == MergeRequestEvidenceError::MergeResultMissing =>
|
||||||
|
{
|
||||||
|
assert!(message.contains(expected.as_str()), "{case}: {message}");
|
||||||
|
}
|
||||||
|
MergeRequestError::NotReady(message)
|
||||||
|
if expected != MergeRequestEvidenceError::MergeResultMissing =>
|
||||||
|
{
|
||||||
|
assert!(message.contains(expected.as_str()), "{case}: {message}");
|
||||||
|
}
|
||||||
|
other => panic!("{case}: unexpected completion error {other}"),
|
||||||
|
}
|
||||||
|
let (state, assigned): (String, bool) = connection.query_row(
|
||||||
|
"SELECT workflow_state, EXISTS(SELECT 1 FROM ticket_current_worker_assignments WHERE workspace_id='W' AND ticket_id='T') FROM typed_tickets WHERE workspace_id='W' AND ticket_id='T'",
|
||||||
|
[],
|
||||||
|
|row| Ok((row.get(0)?, row.get(1)?)),
|
||||||
|
).unwrap();
|
||||||
|
assert_eq!(state, "inprogress", "{case}");
|
||||||
|
assert!(assigned, "{case}");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user