feat(ticket): decouple audited lifecycle decisions from MR integration
T-718: unify state, completion, close and reopen with explicit CAS and operation receipts; keep MR integration proofs independent. Queue only the explicitly selected Ticket and fence target edits against active Worker grants. Align REST, Worker/WIP, evidence projection and generated contracts. Validation: root cargo check; full ticket, merge-request, client, worker, yoi-workspace-server and server-api tests; server-api typescript feature; OpenAPI and six generated API checkers; Svelte check and 36 narrow Web tests; fmt and diff checks.
This commit is contained in:
@@ -801,27 +801,6 @@ impl SqliteWorkspaceAuthority {
|
||||
let write_repository_keys = ticket_write_target_repository_keys(&targets);
|
||||
let has_target = !write_repository_keys.is_empty();
|
||||
let has_blockers = !ticket.relations.blockers.is_empty();
|
||||
let mut queue_assignment_blockers = Vec::new();
|
||||
for ticket_id in &dependency_check.queue_tickets {
|
||||
let assignments = self
|
||||
.store
|
||||
.list_active_ticket_role_assignments(&self.workspace_id, ticket_id)?;
|
||||
if !assignments
|
||||
.iter()
|
||||
.any(|assignment| assignment.role == TicketAssignmentRole::Orchestrator)
|
||||
{
|
||||
queue_assignment_blockers.push(format!(
|
||||
"Ticket {ticket_id} requires an active Orchestrator assignment"
|
||||
));
|
||||
}
|
||||
if assignments
|
||||
.iter()
|
||||
.any(|assignment| assignment.role == TicketAssignmentRole::Coder)
|
||||
{
|
||||
queue_assignment_blockers
|
||||
.push(format!("Ticket {ticket_id} has an active Coder assignment"));
|
||||
}
|
||||
}
|
||||
let mut assignment_diagnostics = Vec::new();
|
||||
if let Some(legacy_assignee) = ticket
|
||||
.meta
|
||||
@@ -833,19 +812,7 @@ impl SqliteWorkspaceAuthority {
|
||||
"legacy Ticket assignee `{legacy_assignee}` is not assignment authority"
|
||||
));
|
||||
}
|
||||
let mut action_blockers = Vec::new();
|
||||
if !has_target {
|
||||
action_blockers.push("Ticket target is required".to_string());
|
||||
}
|
||||
if !dependency_check.queue_guard.can_queue_for_orchestrator {
|
||||
if let Some(reason) = dependency_check.queue_guard.blocked_reason.clone() {
|
||||
action_blockers.push(reason);
|
||||
} else if let Some(reason) = dependency_check.queue_guard.reason.clone() {
|
||||
action_blockers.push(reason);
|
||||
}
|
||||
}
|
||||
let queue_assignments_valid = queue_assignment_blockers.is_empty();
|
||||
action_blockers.extend(queue_assignment_blockers);
|
||||
let action_blockers = Vec::new();
|
||||
let action_eligibility = TicketActionEligibility {
|
||||
can_assign_orchestrator: matches!(
|
||||
ticket.meta.workflow_state,
|
||||
@@ -857,12 +824,7 @@ impl SqliteWorkspaceAuthority {
|
||||
ticket.meta.workflow_state,
|
||||
TicketWorkflowState::Planning | TicketWorkflowState::Ready
|
||||
),
|
||||
can_queue: ticket.meta.workflow_state == TicketWorkflowState::Ready
|
||||
&& has_orchestrator
|
||||
&& !has_coder
|
||||
&& has_target
|
||||
&& dependency_check.queue_guard.can_queue_for_orchestrator
|
||||
&& queue_assignments_valid,
|
||||
can_queue: true,
|
||||
can_start_manual_coder: ticket.meta.workflow_state == TicketWorkflowState::Ready
|
||||
&& !has_orchestrator
|
||||
&& !has_coder
|
||||
@@ -1901,16 +1863,13 @@ fn ticket_requirement_approved(
|
||||
}
|
||||
|
||||
fn ticket_evidence_summary(
|
||||
ticket_write_repository_keys: &[&str],
|
||||
_ticket_write_repository_keys: &[&str],
|
||||
merge_requests: &[TicketMergeRequestSummary],
|
||||
requirement_approved: bool,
|
||||
) -> TicketEvidenceSummary {
|
||||
let relevant = merge_requests
|
||||
.iter()
|
||||
.filter(|request| {
|
||||
request.state != "closed"
|
||||
&& ticket_write_repository_keys.contains(&request.repository_key.as_str())
|
||||
})
|
||||
.filter(|request| request.state != "closed")
|
||||
.collect::<Vec<_>>();
|
||||
let has_merge_request = !relevant.is_empty();
|
||||
let has_current_subject_ref = has_merge_request
|
||||
@@ -1956,9 +1915,6 @@ fn ticket_evidence_summary(
|
||||
Some("pending".into())
|
||||
};
|
||||
let mut missing = Vec::new();
|
||||
if !has_merge_request {
|
||||
missing.push("merge_request".into());
|
||||
}
|
||||
if observation_unavailable {
|
||||
missing.push("source_ref_unavailable".into());
|
||||
}
|
||||
@@ -1968,16 +1924,16 @@ fn ticket_evidence_summary(
|
||||
{
|
||||
missing.push("integration_evidence".into());
|
||||
}
|
||||
if !has_current_subject_ref {
|
||||
if has_merge_request && !has_current_subject_ref {
|
||||
missing.push("current_subject_ref".into());
|
||||
}
|
||||
if !has_commit {
|
||||
if has_merge_request && !has_commit {
|
||||
missing.push("commit".into());
|
||||
}
|
||||
if unresolved_request_changes {
|
||||
missing.push("unresolved_request_changes".into());
|
||||
}
|
||||
if !approved_current_subject {
|
||||
if has_merge_request && !approved_current_subject {
|
||||
missing.push("approved_current_subject".into());
|
||||
}
|
||||
if approved_current_subject && !requirement_approved {
|
||||
@@ -1992,7 +1948,7 @@ fn ticket_evidence_summary(
|
||||
approved_current_subject,
|
||||
review_after_rescope,
|
||||
unresolved_request_changes,
|
||||
complete_for_integration: missing.is_empty(),
|
||||
complete_for_integration: has_merge_request && missing.is_empty(),
|
||||
missing,
|
||||
}
|
||||
}
|
||||
@@ -2202,7 +2158,7 @@ fn ticket_attention_matches(
|
||||
.iter()
|
||||
.any(|reason| reason == "review_after_rescope")
|
||||
}
|
||||
"missing_evidence" => !evidence.complete_for_integration,
|
||||
"missing_evidence" => evidence.has_merge_request && !evidence.complete_for_integration,
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
@@ -3227,7 +3183,7 @@ mod tests {
|
||||
assert!(
|
||||
ticket_evidence_summary(&["main", "other"], &summaries, true).complete_for_integration
|
||||
);
|
||||
assert!(!ticket_evidence_summary(&["docs"], &summaries, true).has_merge_request);
|
||||
assert!(ticket_evidence_summary(&["docs"], &summaries, true).has_merge_request);
|
||||
let mut closed = reviewed_merge_request(ReviewDecision::Approve, false);
|
||||
closed.state = MergeRequestState::Closed;
|
||||
assert!(
|
||||
@@ -3413,19 +3369,16 @@ VALUES ('workspace-test', 'ticket', 4);
|
||||
assert_eq!(tickets.items[0].record_source, "sqlite_yoi_ticket");
|
||||
assert_eq!(tickets.items[0].id, "00000000001J2");
|
||||
assert_eq!(tickets.items[0].state, "ready");
|
||||
assert_eq!(tickets.items[0].workspace_action_priority, "background");
|
||||
assert_eq!(
|
||||
tickets.items[0].workspace_action_priority,
|
||||
"ready_for_queue"
|
||||
);
|
||||
let ticket_by_key = authority.ticket(&tickets.items[0].resource_key).unwrap();
|
||||
assert_eq!(ticket_by_key.id, tickets.items[0].id);
|
||||
|
||||
let ticket = authority.ticket("00000000001J2").unwrap();
|
||||
assert!(!ticket.action_eligibility.can_queue);
|
||||
assert!(
|
||||
ticket
|
||||
.action_eligibility
|
||||
.blockers
|
||||
.iter()
|
||||
.any(|reason| reason.contains("00000000001J6"))
|
||||
);
|
||||
assert!(ticket.action_eligibility.can_queue);
|
||||
assert!(ticket.action_eligibility.blockers.is_empty());
|
||||
assert!(ticket.body.contains("Ticket body"));
|
||||
assert!(ticket.body_truncated);
|
||||
assert!(!ticket.body.contains("Deep Ticket marker"));
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -4,10 +4,9 @@
|
||||
use super::*;
|
||||
|
||||
use merge_request::{
|
||||
CompleteMergeRequest, CompleteTicket, ConflictResolution, MergeRequestAuth,
|
||||
MergeRequestReviewSubject, MergeRequestStore, MergeStrategy, OpenMergeRequest,
|
||||
RegisterReviewerChildSession, RequestMergeRequestReview, ReviewDecision, ReviewEvent,
|
||||
SubmitMergeRequestReview,
|
||||
CompleteMergeRequest, ConflictResolution, MergeRequestAuth, MergeRequestReviewSubject,
|
||||
MergeRequestStore, MergeStrategy, OpenMergeRequest, RegisterReviewerChildSession,
|
||||
RequestMergeRequestReview, ReviewDecision, ReviewEvent, SubmitMergeRequestReview,
|
||||
};
|
||||
use rusqlite::{Connection, params};
|
||||
use server_api::{
|
||||
@@ -192,15 +191,14 @@ impl EvidenceApiFixture {
|
||||
.unwrap()
|
||||
}
|
||||
|
||||
fn completion(&self, approval: &ReviewEvent) -> CompleteTicket {
|
||||
CompleteTicket {
|
||||
ticket_id: self.ticket_id.clone(),
|
||||
operation_id: "evidence-ticket-completion".into(),
|
||||
item_revision: self.revision(),
|
||||
merge_request_ids: vec![EVIDENCE_MR.into()],
|
||||
requirement_approval_event_id: approval.event_id.clone(),
|
||||
auth: self.auth.clone(),
|
||||
now: evidence_time(),
|
||||
fn completion(&self, _approval: &ReviewEvent) -> ticket::TicketCompletion {
|
||||
ticket::TicketCompletion {
|
||||
operation_key: "evidence-ticket-completion".into(),
|
||||
expected_item_revision: self.revision(),
|
||||
expected_state: TicketWorkflowState::InProgress,
|
||||
reason: "Implementation judged complete independently of MR attestation".into(),
|
||||
references: Vec::new(),
|
||||
author: Some("fixture".into()),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -305,10 +303,10 @@ async fn done_merged_remote_ticket_uses_immutable_result_in_show_query_list_and_
|
||||
let merge = fixture.integrate(&approval);
|
||||
let revision = fixture.revision();
|
||||
let completion = fixture
|
||||
.store
|
||||
.complete_ticket(fixture.completion(&approval))
|
||||
.backend
|
||||
.complete(&fixture.ticket_id, fixture.completion(&approval))
|
||||
.unwrap();
|
||||
assert_eq!(completion.item_revision, revision);
|
||||
assert_eq!(ticket::ticket_item_revision(&completion), revision);
|
||||
assert!(
|
||||
fixture
|
||||
.api
|
||||
@@ -398,16 +396,13 @@ async fn latest_postmerge_approval_requires_exact_revision_and_stored_source_res
|
||||
);
|
||||
fixture.assert_filters(false, true, true).await;
|
||||
let mut outdated_completion = fixture.completion(&integration);
|
||||
outdated_completion.item_revision = integration.ticket_item_revision.clone();
|
||||
assert!(matches!(
|
||||
fixture.store.complete_ticket(outdated_completion),
|
||||
Err(merge_request::MergeRequestError::Conflict(_))
|
||||
));
|
||||
assert!(matches!(
|
||||
fixture.store.complete_ticket(fixture.completion(&integration)),
|
||||
Err(merge_request::MergeRequestError::NotReady(message))
|
||||
if message == merge_request::MergeRequestEvidenceError::ItemRevisionMismatch.as_str()
|
||||
));
|
||||
outdated_completion.expected_item_revision = integration.ticket_item_revision.clone();
|
||||
assert!(
|
||||
fixture
|
||||
.backend
|
||||
.complete(&fixture.ticket_id, outdated_completion)
|
||||
.is_err()
|
||||
);
|
||||
assert_eq!(fixture.show().await.state, "inprogress");
|
||||
|
||||
let latest = fixture.approve("latest-revision");
|
||||
@@ -425,11 +420,6 @@ async fn latest_postmerge_approval_requires_exact_revision_and_stored_source_res
|
||||
Some("Requirement approval integration")
|
||||
);
|
||||
fixture.assert_filters(true, false, false).await;
|
||||
assert!(matches!(
|
||||
fixture.store.complete_ticket(fixture.completion(&integration)),
|
||||
Err(merge_request::MergeRequestError::NotReady(message))
|
||||
if message == merge_request::MergeRequestEvidenceError::ApprovalNotEffective.as_str()
|
||||
));
|
||||
// A current revision alone is insufficient: simulate a stored attestation
|
||||
// against a different result set using typed serialized payloads in this
|
||||
// temporary fixture database, never a live Workspace database.
|
||||
@@ -477,11 +467,6 @@ async fn latest_postmerge_approval_requires_exact_revision_and_stored_source_res
|
||||
assert!(wrong.evidence.approved_current_subject);
|
||||
assert!(!wrong.evidence.complete_for_integration);
|
||||
fixture.assert_filters(false, true, true).await;
|
||||
assert!(matches!(
|
||||
fixture.store.complete_ticket(fixture.completion(&latest)),
|
||||
Err(merge_request::MergeRequestError::NotReady(message))
|
||||
if message == merge_request::MergeRequestEvidenceError::SourceSnapshotMismatch.as_str()
|
||||
));
|
||||
assert_eq!(fixture.show().await.state, "inprogress");
|
||||
|
||||
let repaired = fixture.approve("exact-snapshot");
|
||||
@@ -502,11 +487,10 @@ async fn latest_postmerge_approval_requires_exact_revision_and_stored_source_res
|
||||
assert_eq!(detail.source.revision_ref.as_deref(), Some(EVIDENCE_SOURCE));
|
||||
assert_eq!(detail.target.revision_ref.as_deref(), Some(EVIDENCE_TARGET));
|
||||
let completion = fixture
|
||||
.store
|
||||
.complete_ticket(fixture.completion(&repaired))
|
||||
.backend
|
||||
.complete(&fixture.ticket_id, fixture.completion(&repaired))
|
||||
.unwrap();
|
||||
assert_eq!(completion.item_revision, revision);
|
||||
assert_eq!(completion.requirement_approval_event_id, repaired.event_id);
|
||||
assert_eq!(ticket::ticket_item_revision(&completion), revision);
|
||||
assert_eq!(fixture.show().await.state, "done");
|
||||
fixture.assert_filters(true, false, false).await;
|
||||
}
|
||||
@@ -651,3 +635,187 @@ fn source_observation_diagnostics_preserve_provider_failure_codes() {
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn closing_ticket_with_open_unreviewed_mr_does_not_change_its_proof_or_state() {
|
||||
let fixture = EvidenceApiFixture::new().await;
|
||||
let before = fixture
|
||||
.store
|
||||
.get_by_id(TEST_WORKSPACE_ID, EVIDENCE_MR)
|
||||
.unwrap();
|
||||
fixture
|
||||
.backend
|
||||
.update_state(
|
||||
&fixture.ticket_id,
|
||||
ticket::TicketStateUpdate {
|
||||
operation_key: "close-with-open-mr".into(),
|
||||
expected_item_revision: fixture.revision(),
|
||||
expected_state: TicketWorkflowState::InProgress,
|
||||
state: TicketWorkflowState::Closed,
|
||||
reason: "The request was withdrawn; no integration or approval is asserted".into(),
|
||||
references: Vec::new(),
|
||||
author: Some("workspace-user".into()),
|
||||
},
|
||||
)
|
||||
.unwrap();
|
||||
let after = fixture
|
||||
.store
|
||||
.get_by_id(TEST_WORKSPACE_ID, EVIDENCE_MR)
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
serde_json::to_value(after).unwrap(),
|
||||
serde_json::to_value(before).unwrap()
|
||||
);
|
||||
let shown = fixture.show().await;
|
||||
assert_eq!(shown.state, "closed");
|
||||
assert!(!shown.evidence.approved_current_subject);
|
||||
assert_eq!(shown.merge_requests[0].state, "open");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn mrless_completion_is_a_ticket_judgment_not_approved_or_missing_merge_evidence() {
|
||||
let temp = tempfile::tempdir().unwrap();
|
||||
let api = test_api_with_remote_repository(temp.path()).await;
|
||||
let backend = browser_ticket_backend(&api).unwrap();
|
||||
let reference = backend
|
||||
.create(ticket::NewTicket::new("Investigation with no repository"))
|
||||
.unwrap();
|
||||
let ticket = backend.show(reference.id.clone().into()).unwrap();
|
||||
backend
|
||||
.complete(
|
||||
&reference.id,
|
||||
ticket::TicketCompletion {
|
||||
operation_key: "research-result".into(),
|
||||
expected_item_revision: ticket::ticket_item_revision(&ticket),
|
||||
expected_state: TicketWorkflowState::Planning,
|
||||
reason: "Answer recorded in the Ticket thread".into(),
|
||||
references: Vec::new(),
|
||||
author: Some("workspace-user".into()),
|
||||
},
|
||||
)
|
||||
.unwrap();
|
||||
let app = build_inner_router(api);
|
||||
let shown: TicketDetail = serde_json::from_value(
|
||||
get_json(
|
||||
app.clone(),
|
||||
&format!("/api/w/{TEST_WORKSPACE_ID}/tickets/{}", reference.id),
|
||||
)
|
||||
.await,
|
||||
)
|
||||
.unwrap();
|
||||
assert_eq!(shown.state, "done");
|
||||
assert!(!shown.evidence.has_merge_request);
|
||||
assert!(!shown.evidence.approved_current_subject);
|
||||
assert!(!shown.evidence.complete_for_integration);
|
||||
assert!(shown.evidence.missing.is_empty());
|
||||
assert!(shown.evidence.review_status.is_none());
|
||||
for filter in [
|
||||
json!({"attention":["missing_evidence"]}),
|
||||
json!({"evidence":["approved_review"]}),
|
||||
] {
|
||||
let query: TicketQueryResponse = serde_json::from_value(
|
||||
request_json(
|
||||
app.clone(),
|
||||
"POST",
|
||||
&format!("/api/w/{TEST_WORKSPACE_ID}/tickets/query"),
|
||||
Some(filter),
|
||||
StatusCode::OK,
|
||||
)
|
||||
.await,
|
||||
)
|
||||
.unwrap();
|
||||
assert!(query.items.is_empty());
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn all_public_state_and_close_routes_require_explicit_cas_and_operation_receipts() {
|
||||
let temp = tempfile::tempdir().unwrap();
|
||||
let api = test_api_with_remote_repository(temp.path()).await;
|
||||
let backend = browser_ticket_backend(&api).unwrap();
|
||||
let reference = backend
|
||||
.create(ticket::NewTicket::new("Transport decisions"))
|
||||
.unwrap();
|
||||
let before = backend.show(reference.id.clone().into()).unwrap();
|
||||
let app = build_inner_router(api);
|
||||
let path = format!("/api/w/{TEST_WORKSPACE_ID}/tickets/{}", reference.id);
|
||||
for suffix in ["workflow-state", "state-changes", "state-fields/state"] {
|
||||
request_json(
|
||||
app.clone(),
|
||||
"POST",
|
||||
&format!("{path}/{suffix}"),
|
||||
Some(json!({
|
||||
"from":"planning","to":"done","reason":"old unguarded request","body":""
|
||||
})),
|
||||
StatusCode::UNPROCESSABLE_ENTITY,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
request_json(
|
||||
app.clone(),
|
||||
"POST",
|
||||
&format!("{path}/workflow/close"),
|
||||
Some(json!("unguarded close")),
|
||||
StatusCode::UNPROCESSABLE_ENTITY,
|
||||
)
|
||||
.await;
|
||||
let close = json!({
|
||||
"operation_key":"guarded-close","expected_item_revision":ticket::ticket_item_revision(&before),
|
||||
"expected_state":"planning","reason":"No further work required","author":"spoofed-client-author"
|
||||
});
|
||||
let response = app
|
||||
.clone()
|
||||
.oneshot(
|
||||
Request::builder()
|
||||
.method("POST")
|
||||
.uri(format!("{path}/workflow/close"))
|
||||
.header(CONTENT_TYPE, "application/json")
|
||||
.body(Body::from(close.to_string()))
|
||||
.unwrap(),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(response.status(), StatusCode::NO_CONTENT);
|
||||
let closed = backend.show(reference.id.clone().into()).unwrap();
|
||||
assert_eq!(closed.meta.workflow_state, TicketWorkflowState::Closed);
|
||||
let receipt = closed.events.last().unwrap();
|
||||
assert_eq!(receipt.author.as_deref(), Some("workspace-user"));
|
||||
assert_eq!(receipt.from.as_deref(), Some("planning"));
|
||||
assert_eq!(
|
||||
receipt.attributes.get("operation_key"),
|
||||
Some(&"guarded-close".to_string())
|
||||
);
|
||||
let reopen = json!({
|
||||
"operation_key":"reopen","expected_item_revision":ticket::ticket_item_revision(&closed),
|
||||
"expected_state":"closed","state":"planning","reason":"A new question arrived"
|
||||
});
|
||||
let response = app
|
||||
.clone()
|
||||
.oneshot(
|
||||
Request::builder()
|
||||
.method("POST")
|
||||
.uri(format!("{path}/workflow-state"))
|
||||
.header(CONTENT_TYPE, "application/json")
|
||||
.body(Body::from(reopen.to_string()))
|
||||
.unwrap(),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(response.status(), StatusCode::NO_CONTENT);
|
||||
// Exact replay of the old close is a receipt lookup, not another close.
|
||||
let response = app
|
||||
.oneshot(
|
||||
Request::builder()
|
||||
.method("POST")
|
||||
.uri(format!("{path}/workflow/close"))
|
||||
.header(CONTENT_TYPE, "application/json")
|
||||
.body(Body::from(close.to_string()))
|
||||
.unwrap(),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(response.status(), StatusCode::NO_CONTENT);
|
||||
let reopened = backend.show(reference.id.into()).unwrap();
|
||||
assert_eq!(reopened.meta.workflow_state, TicketWorkflowState::Planning);
|
||||
assert_eq!(reopened.events.len(), closed.events.len() + 1);
|
||||
}
|
||||
|
||||
@@ -89,14 +89,7 @@ impl TicketItemCheckFindingCategory {
|
||||
}
|
||||
|
||||
pub fn item_revision(ticket: &ticket::Ticket) -> String {
|
||||
ticket
|
||||
.events
|
||||
.iter()
|
||||
.rev()
|
||||
.find(|event| matches!(event.kind.as_str(), "create" | "item_edit"))
|
||||
.and_then(|event| event.attributes.get("event_id").cloned())
|
||||
.or_else(|| ticket.meta.updated_at.clone())
|
||||
.unwrap_or_else(|| format!("{}:0", ticket.meta.id))
|
||||
ticket::ticket_item_revision(ticket)
|
||||
}
|
||||
|
||||
pub fn request(
|
||||
|
||||
Reference in New Issue
Block a user