From df390f02e985e7d7c57fd779baefced9cd3891c7 Mon Sep 17 00:00:00 2001 From: Hare Date: Thu, 8 Oct 2026 19:12:39 +0900 Subject: [PATCH] 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. --- crates/client/src/workspace_product.rs | 188 +- crates/merge-request/src/lib.rs | 226 +- crates/merge-request/tests/store.rs | 319 ++- crates/server-api/src/lib.rs | 112 +- crates/ticket/src/lib.rs | 2472 +++++++++++------ crates/ticket/src/tool.rs | 132 +- .../src/feature/builtin/merge_request.rs | 81 +- .../feature/builtin/resource_projection.rs | 55 +- crates/worker/src/feature/builtin/ticket.rs | 1067 ++++++- crates/worker/src/wip.rs | 58 +- crates/workspace-server/src/authority.rs | 79 +- crates/workspace-server/src/server.rs | 1321 ++++----- .../src/server/tests/ticket_evidence_tests.rs | 248 +- .../src/ticket_item_checker.rs | 9 +- docs/design/merge-request-wip-projection.md | 11 +- docs/design/ticket-evidence.md | 6 +- docs/report/t718-subworker-path-context.md | 15 + openapi/server-api.json | 461 ++- .../src/lib/generated/companion-api.ts | 2 +- .../src/lib/generated/repository-api.ts | 2 +- .../src/lib/generated/runtime-api.ts | 2 +- web/workspace/src/lib/generated/ticket-api.ts | 6 +- .../src/lib/generated/workdir-api.ts | 2 +- .../src/lib/generated/worker-launch-api.ts | 2 +- .../tickets/[ticketId]/+page.svelte | 12 +- 25 files changed, 4596 insertions(+), 2292 deletions(-) create mode 100644 docs/report/t718-subworker-path-context.md diff --git a/crates/client/src/workspace_product.rs b/crates/client/src/workspace_product.rs index c65d1b4a2..773d3e505 100644 --- a/crates/client/src/workspace_product.rs +++ b/crates/client/src/workspace_product.rs @@ -155,14 +155,40 @@ impl BackendWorkspaceProductClient { }) } + fn decision_from_state_change( + &self, + id: &TicketIdOrSlug, + change: &TicketStateChange, + ) -> Result { + let current = self.show_ticket(id)?; + let parse = |state: &str| { + ticket::TicketWorkflowState::parse(state).ok_or_else(|| { + BackendWorkspaceClientError::InvalidTarget(format!("invalid Ticket state {state}")) + }) + }; + Ok(ticket::TicketStateUpdate { + operation_key: uuid::Uuid::now_v7().to_string(), + expected_item_revision: ticket::ticket_item_revision(¤t), + expected_state: parse(&change.from)?, + state: parse(&change.to)?, + reason: if change.body.as_str().is_empty() { + change.reason.clone() + } else { + format!("{}\n\n{}", change.reason, change.body.as_str()) + }, + references: change.references.clone(), + author: change.author.clone(), + }) + } + pub fn set_ticket_workflow_state( &self, id: &TicketIdOrSlug, change: &TicketStateChange, ) -> Result<(), BackendWorkspaceClientError> { + let change = self.decision_from_state_change(id, change)?; let workspace_id = self.workspace_id.clone(); let id = ticket_reference(id); - let change = change.clone(); self.generated(move |client| async move { client .ticket_workflow_state_set( @@ -179,15 +205,23 @@ impl BackendWorkspaceProductClient { id: &TicketIdOrSlug, resolution: &MarkdownText, ) -> Result<(), BackendWorkspaceClientError> { + let current = self.show_ticket(id)?; + let request = ticket::TicketCompletion { + operation_key: uuid::Uuid::now_v7().to_string(), + expected_item_revision: ticket::ticket_item_revision(¤t), + expected_state: current.meta.workflow_state, + reason: resolution.as_str().to_string(), + references: Vec::new(), + author: None, + }; let workspace_id = self.workspace_id.clone(); let id = ticket_reference(id); - let resolution = resolution.clone(); self.generated(move |client| async move { client .ticket_close_record( workspace_id, id, - server_api::TicketCloseRecordRequest(resolution), + server_api::TicketCloseRecordRequest(request), ) .await }) @@ -633,6 +667,9 @@ impl TicketBackend for BackendWorkspaceProductClient { id: TicketIdOrSlug, change: TicketStateChange, ) -> ticket::Result<()> { + let change = self + .decision_from_state_change(&id, &change) + .map_err(ticket_client_error)?; let workspace_id = self.workspace_id.clone(); let id = ticket_reference(&id); self.generated(move |client| async move { @@ -672,6 +709,14 @@ impl TicketBackend for BackendWorkspaceProductClient { field: &str, change: TicketStateChange, ) -> ticket::Result<()> { + if field != "state" { + return Err(TicketError::Conflict( + "unsupported Ticket state field".into(), + )); + } + let change = self + .decision_from_state_change(&id, &change) + .map_err(ticket_client_error)?; let workspace_id = self.workspace_id.clone(); let id = ticket_reference(&id); let field = field.to_string(); @@ -697,6 +742,26 @@ impl TicketBackend for BackendWorkspaceProductClient { .map_err(ticket_client_error) } + fn update_state( + &self, + ticket: &str, + request: ticket::TicketStateUpdate, + ) -> ticket::Result { + let workspace_id = self.workspace_id.clone(); + let id = ticket.to_owned(); + self.generated(move |client| async move { + client + .ticket_state_update( + workspace_id, + id, + server_api::TicketStateUpdateRequest(request), + ) + .await + .map(|response| response.0) + }) + .map_err(ticket_client_error) + } + fn mark_ready(&self, id: TicketIdOrSlug, request: TicketMarkReady) -> ticket::Result { let workspace_id = self.workspace_id.clone(); let id = ticket_reference(&id); @@ -903,8 +968,12 @@ mod tests { } fn response_sequence_server( - responses: Vec<(&'static str, &'static str)>, + responses: Vec<(&str, &str)>, ) -> (String, mpsc::Receiver, thread::JoinHandle<()>) { + let responses = responses + .into_iter() + .map(|(status, body)| (status.to_owned(), body.to_owned())) + .collect::>(); let listener = TcpListener::bind("127.0.0.1:0").unwrap(); let address = listener.local_addr().unwrap(); let (sender, receiver) = mpsc::channel(); @@ -927,6 +996,117 @@ mod tests { (format!("http://{address}"), receiver, handle) } + #[test] + fn legacy_state_adapter_uses_snapshot_revision_without_overriding_stale_expected_state() { + let temp = tempfile::tempdir().unwrap(); + let backend = + ticket::SqliteTicketBackend::open(temp.path().join("fixture.db"), "workspace-a") + .unwrap(); + let reference = backend.create(NewTicket::new("Adapter snapshot")).unwrap(); + let snapshot = backend.show(reference.id.clone().into()).unwrap(); + let body = serde_json::to_string(&snapshot).unwrap(); + let (base_url, requests, handle) = + response_sequence_server(vec![("200 OK", &body), ("204 No Content", "")]); + let client = + BackendWorkspaceProductClient::new_with_access_token(base_url, "workspace-a", "token") + .unwrap(); + client + .set_ticket_workflow_state( + &reference.id.clone().into(), + &TicketStateChange::new("ready", "done", "Judgment", "Results in the thread"), + ) + .unwrap(); + assert!(requests.recv().unwrap().starts_with("GET ")); + let request = requests.recv().unwrap(); + let body: serde_json::Value = + serde_json::from_str(request.split("\r\n\r\n").nth(1).unwrap()).unwrap(); + assert_eq!( + body["expected_item_revision"], + ticket::ticket_item_revision(&snapshot) + ); + assert_eq!(body["expected_state"], "ready"); + assert_eq!(body["state"], "done"); + assert!(!body["operation_key"].as_str().unwrap().is_empty()); + handle.join().unwrap(); + } + + #[test] + fn legacy_close_adapter_forwards_snapshot_cas_and_resolution_as_judgment() { + let temp = tempfile::tempdir().unwrap(); + let backend = + ticket::SqliteTicketBackend::open(temp.path().join("fixture.db"), "workspace-a") + .unwrap(); + let reference = backend + .create(NewTicket::new("Close adapter snapshot")) + .unwrap(); + let snapshot = backend.show(reference.id.clone().into()).unwrap(); + let body = serde_json::to_string(&snapshot).unwrap(); + let (base_url, requests, handle) = + response_sequence_server(vec![("200 OK", &body), ("204 No Content", "")]); + let client = + BackendWorkspaceProductClient::new_with_access_token(base_url, "workspace-a", "token") + .unwrap(); + client + .close_ticket( + &reference.id.into(), + &MarkdownText::new("The request was withdrawn"), + ) + .unwrap(); + assert!(requests.recv().unwrap().starts_with("GET ")); + let request = requests.recv().unwrap(); + let body: serde_json::Value = + serde_json::from_str(request.split("\r\n\r\n").nth(1).unwrap()).unwrap(); + assert_eq!( + body["expected_item_revision"], + ticket::ticket_item_revision(&snapshot) + ); + assert_eq!(body["expected_state"], "planning"); + assert_eq!(body["reason"], "The request was withdrawn"); + assert!(!body["operation_key"].as_str().unwrap().is_empty()); + handle.join().unwrap(); + } + + #[test] + fn ticket_backend_completion_forwards_cas_judgment_and_backend_conflicts() { + let (base_url, request, handle) = one_response_server( + "409 Conflict", + r#"{"error":"ticket_conflict","message":"stale revision","diagnostics":[]}"#, + ); + let client = BackendWorkspaceProductClient::new_with_access_token( + base_url, + "workspace-a", + "test-token", + ) + .unwrap(); + let result = TicketBackend::complete( + &client, + "T-718", + ticket::TicketCompletion { + operation_key: "research-done".into(), + expected_item_revision: "ticket:1".into(), + expected_state: ticket::TicketWorkflowState::Planning, + reason: "Research findings recorded".into(), + references: Vec::new(), + author: None, + }, + ); + assert!(result.is_err()); + let request = request.recv().unwrap(); + assert!( + request.starts_with("POST /api/w/workspace-a/tickets/T-718/state-update "), + "{request}" + ); + let body: serde_json::Value = + serde_json::from_str(request.split("\r\n\r\n").nth(1).unwrap()).unwrap(); + assert_eq!(body["state"], "done"); + assert_eq!(body["operation_key"], "research-done"); + assert_eq!(body["expected_item_revision"], "ticket:1"); + assert_eq!(body["expected_state"], "planning"); + assert_eq!(body["reason"], "Research findings recorded"); + assert!(body.get("merge_request_ids").is_none()); + handle.join().unwrap(); + } + #[test] fn memory_document_uses_shared_workspace_scoped_response() { let body = r##"{"body_md":"# Memory\\n","created_at":"2026-09-01T00:00:00Z","updated_at":"2026-09-02T00:00:00Z","bytes":10,"record_source":"workspace-sqlite"}"##; diff --git a/crates/merge-request/src/lib.rs b/crates/merge-request/src/lib.rs index e9e95e79b..c0114b142 100644 --- a/crates/merge-request/src/lib.rs +++ b/crates/merge-request/src/lib.rs @@ -598,16 +598,8 @@ pub struct CompleteMergeRequest { pub auth: MergeRequestAuth, pub now: DateTime, } -#[derive(Debug, Clone)] -pub struct CompleteTicket { - pub ticket_id: String, - pub operation_id: String, - pub item_revision: String, - pub merge_request_ids: Vec, - pub requirement_approval_event_id: String, - pub auth: MergeRequestAuth, - pub now: DateTime, -} +/// Historical MR-owned Ticket completion payload, retained for reading existing events. +/// Canonical Ticket completion is owned by the Ticket domain. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, schemars::JsonSchema)] pub struct TicketCompletionEvent { pub operation_id: String, @@ -1332,164 +1324,6 @@ impl MergeRequestStore { Ok(event) } - pub fn complete_ticket( - &self, - mut input: CompleteTicket, - ) -> Result { - nonempty("operation_id", &input.operation_id)?; - nonempty("item_revision", &input.item_revision)?; - nonempty( - "requirement_approval_event_id", - &input.requirement_approval_event_id, - )?; - if input.merge_request_ids.is_empty() { - return Err(MergeRequestError::NotReady( - "Ticket completion requires at least one linked Merge Request result".into(), - )); - } - input.merge_request_ids.sort(); - if input - .merge_request_ids - .windows(2) - .any(|ids| ids[0] == ids[1]) - { - return Err(MergeRequestError::Validation( - "merge_request_ids must not contain duplicates".into(), - )); - } - let mut connection = self.lock()?; - let transaction = connection.transaction()?; - let existing: Option = transaction - .query_row( - "SELECT payload.value - FROM typed_ticket_event_attributes operation - JOIN typed_ticket_event_attributes payload - ON payload.workspace_id=operation.workspace_id - AND payload.ticket_id=operation.ticket_id - AND payload.event_index=operation.event_index - AND payload.key='ticket_completion' - WHERE operation.workspace_id=?1 AND operation.ticket_id=?2 - AND operation.key='operation_id' AND operation.value=?3", - params![input.auth.workspace_id, input.ticket_id, input.operation_id], - |row| row.get(0), - ) - .optional()?; - if let Some(payload) = existing { - let event: TicketCompletionEvent = json(&payload)?; - if event.item_revision == input.item_revision - && event.merge_request_ids == input.merge_request_ids - && event.requirement_approval_event_id == input.requirement_approval_event_id - && event.completed_by == input.auth.actor() - { - return Ok(event); - } - return Err(MergeRequestError::Conflict( - "Ticket completion operation fingerprint mismatch".into(), - )); - } - let current_revision = - current_ticket_revision(&transaction, &input.auth.workspace_id, &input.ticket_id)? - .ok_or(MergeRequestError::NotFound)?; - if current_revision != input.item_revision { - return Err(MergeRequestError::Conflict(format!( - "Ticket item revision changed from `{}` to `{current_revision}`; repeat requirement review before completion", - input.item_revision - ))); - } - let state: String = transaction.query_row( - "SELECT workflow_state FROM typed_tickets WHERE workspace_id=?1 AND ticket_id=?2", - params![input.auth.workspace_id, input.ticket_id], - |row| row.get(0), - )?; - if state != "inprogress" { - return Err(MergeRequestError::Conflict( - "Ticket must be inprogress".into(), - )); - } - let assignment_is_current: bool = transaction.query_row( - "SELECT EXISTS( - SELECT 1 FROM ticket_active_worker_assignments - WHERE workspace_id=?1 AND ticket_id=?2 AND assignment_id=?3 - )", - params![ - input.auth.workspace_id, - input.ticket_id, - input.auth.assignment_id - ], - |row| row.get(0), - )?; - if !assignment_is_current { - return Err(MergeRequestError::Unauthorized( - "Ticket completion assignment changed before commit".into(), - )); - } - let actual_ids = - linked_merge_request_ids(&transaction, &input.auth.workspace_id, &input.ticket_id)?; - if actual_ids != input.merge_request_ids { - return Err(MergeRequestError::Conflict( - "linked Merge Request set changed; refresh Ticket completion evidence".into(), - )); - } - let mut requests = Vec::with_capacity(actual_ids.len()); - let mut merged_subjects = Vec::with_capacity(actual_ids.len()); - for merge_request_id in &actual_ids { - let request = load_mr(&transaction, &input.auth.workspace_id, merge_request_id)? - .ok_or(MergeRequestError::NotFound)?; - request.integration_approval().map_err(|error| { - let message = format!("Merge Request `{merge_request_id}`: {}", error.as_str()); - if error == MergeRequestEvidenceError::MergeResultMissing { - MergeRequestError::Corrupt(message) - } else { - MergeRequestError::NotReady(message) - } - })?; - let merge = request - .merged_result() - .map_err(|error| MergeRequestError::Corrupt(error.as_str().into()))?; - merged_subjects.push(MergeRequestReviewSubject { - merge_request_id: merge_request_id.clone(), - subject_ref: merge.approved_source_ref.clone(), - }); - requests.push(request); - } - requirement_approval( - &requests, - &input.item_revision, - &merged_subjects, - Some(&input.requirement_approval_event_id), - ) - .map_err(|error| MergeRequestError::NotReady(error.as_str().into()))?; - let updated = transaction.execute( - "UPDATE typed_tickets - SET workflow_state='done',workflow_state_explicit=1,updated_at=?3 - WHERE workspace_id=?1 AND ticket_id=?2 AND workflow_state='inprogress'", - params![ - input.auth.workspace_id, - input.ticket_id, - input.now.to_rfc3339() - ], - )?; - if updated != 1 { - return Err(MergeRequestError::Conflict( - "Ticket state changed during completion".into(), - )); - } - // Terminal state ends unfinished work through the Workspace authority's - // lifecycle trigger. Keep the responsibility identity and its history. - let event = TicketCompletionEvent { - operation_id: input.operation_id, - ticket_id: input.ticket_id.clone(), - item_revision: input.item_revision, - merge_request_ids: input.merge_request_ids, - requirement_approval_event_id: input.requirement_approval_event_id, - completed_by: input.auth.actor(), - created_at: input.now, - }; - ticket_completion_event(&transaction, &input.auth.workspace_id, &event)?; - transaction.commit()?; - Ok(event) - } - pub fn repair_selector_from( &self, i: RepairSelectorFrom, @@ -1935,62 +1769,6 @@ fn ticket_event( Ok(()) } -fn ticket_completion_event( - transaction: &Transaction<'_>, - workspace_id: &str, - event: &TicketCompletionEvent, -) -> Result<(), MergeRequestError> { - let event_index: i64 = transaction.query_row( - "SELECT COALESCE(MAX(event_index),-1)+1 FROM typed_ticket_events WHERE workspace_id=?1 AND ticket_id=?2", - params![workspace_id, event.ticket_id], - |row| row.get(0), - )?; - transaction.execute( - "INSERT INTO typed_ticket_events(workspace_id,ticket_id,event_index,kind,author,at,from_state,to_state,heading,body) - VALUES(?1,?2,?3,'state_changed',?4,?5,'inprogress','done','Ticket requirements completed',?6)", - params![ - workspace_id, - event.ticket_id, - event_index, - format!( - "worker:{}:{}", - event.completed_by.runtime_id, event.completed_by.worker_id - ), - event.created_at.to_rfc3339(), - format!( - "Current Ticket revision `{}` was approved by Review `{}` and completed with Merge Requests: {}.", - event.item_revision, - event.requirement_approval_event_id, - event.merge_request_ids.join(", ") - ) - ], - )?; - for (key, value) in [ - ("operation_id", event.operation_id.clone()), - ("item_revision", event.item_revision.clone()), - ( - "requirement_approval_event_id", - event.requirement_approval_event_id.clone(), - ), - ( - "merge_request_ids", - serde_json::to_string(&event.merge_request_ids) - .map_err(|error| MergeRequestError::Operation(error.to_string()))?, - ), - ( - "ticket_completion", - serde_json::to_string(event) - .map_err(|error| MergeRequestError::Operation(error.to_string()))?, - ), - ] { - transaction.execute( - "INSERT INTO typed_ticket_event_attributes VALUES(?1,?2,?3,?4,?5)", - params![workspace_id, event.ticket_id, event_index, key, value], - )?; - } - Ok(()) -} - pub fn migrate(c: &Connection) -> Result<(), MergeRequestError> { match schema_state(c)? { SchemaState::Fresh => fresh(c), diff --git a/crates/merge-request/tests/store.rs b/crates/merge-request/tests/store.rs index 083f89e93..80b7dd1a9 100644 --- a/crates/merge-request/tests/store.rs +++ b/crates/merge-request/tests/store.rs @@ -166,7 +166,7 @@ fn review_submission_authorization_rejects_invalid_grants_before_side_effects() } #[test] -fn selectors_thread_and_completion_have_no_revision_or_commit_api() { +fn integration_preserves_selectors_ticket_state_and_assignment_and_replays_its_merge() { let (d, s) = fixture(); open(&s); let review = approve(&s, "opaque-source-ref", "token"); @@ -226,45 +226,14 @@ fn selectors_thread_and_completion_have_no_revision_or_commit_api() { }) .unwrap(); assert_eq!(replayed, merged); - let completed = s - .complete_ticket(CompleteTicket { - ticket_id: "T".into(), - operation_id: "ticket-op".into(), - item_revision: "t".into(), - merge_request_ids: vec!["MR".into()], - requirement_approval_event_id: merged.approval_event_id.clone(), - auth: auth(), - now: at(7), - }) - .unwrap(); - assert_eq!(completed.merge_request_ids, vec!["MR"]); let connection = Connection::open(d.path().join("db")).unwrap(); - let (state, assignment_exists): (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, "done"); - assert!(assignment_exists); - let active: bool = connection.query_row("SELECT EXISTS(SELECT 1 FROM ticket_active_worker_assignments WHERE workspace_id='W' AND ticket_id='T')", [], |row| row.get(0)).unwrap(); - assert!(!active); - let replayed_completion = s - .complete_ticket(CompleteTicket { - ticket_id: "T".into(), - operation_id: "ticket-op".into(), - item_revision: "t".into(), - merge_request_ids: vec!["MR".into()], - requirement_approval_event_id: merged.approval_event_id.clone(), - auth: auth(), - now: at(8), - }) - .unwrap(); - assert_eq!(replayed_completion, completed); + let (state, active): (String, bool) = connection.query_row( + "SELECT workflow_state, EXISTS(SELECT 1 FROM ticket_active_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"); + assert!(active); let json = serde_json::to_string(&mr).unwrap(); for banned in [ "revision_id", @@ -866,12 +835,12 @@ fn ticket_rescope_after_integration_accepts_fresh_requirement_attestation() { let (dir, store) = fixture(); open(&store); let integration_approval = approve(&store, "subject", "token-integration"); - store + let merge = store .complete(CompleteMergeRequest { merge_request_id: "MR".into(), ticket_id: "T".into(), operation_id: "merge".into(), - approval_event_id: integration_approval.event_id, + approval_event_id: integration_approval.event_id.clone(), current_subject_ref: "subject".into(), target_ref_before: "target-before".into(), target_ref_after: "target-after".into(), @@ -896,6 +865,26 @@ fn ticket_rescope_after_integration_accepts_fresh_requirement_attestation() { .unwrap(); drop(connection); + let snapshot = vec![MergeRequestReviewSubject { + merge_request_id: "MR".into(), + subject_ref: merge.approved_source_ref.clone(), + }]; + let before_refresh = store.list_for_ticket("W", "T").unwrap(); + assert_eq!( + requirement_approval( + &before_refresh, + "T:1", + &snapshot, + Some(&integration_approval.event_id), + ), + Err(MergeRequestEvidenceError::ItemRevisionMismatch) + ); + assert_eq!(before_refresh[0].merged_result().unwrap(), &merge); + assert_eq!( + before_refresh[0].integration_approval().unwrap(), + &integration_approval + ); + store .register_reviewer_child_session(RegisterReviewerChildSession { workspace_id: "W".into(), @@ -922,7 +911,7 @@ fn ticket_rescope_after_integration_accepts_fresh_requirement_attestation() { now: at(7), }) .unwrap(); - let requirement_approval = store + let requirement_review = store .submit_review(SubmitMergeRequestReview { merge_request_id: "MR".into(), ticket_id: "T".into(), @@ -934,17 +923,33 @@ fn ticket_rescope_after_integration_accepts_fresh_requirement_attestation() { now: at(8), }) .unwrap(); - store - .complete_ticket(CompleteTicket { - ticket_id: "T".into(), - operation_id: "ticket-complete-post-rescope".into(), - item_revision: "T:1".into(), - merge_request_ids: vec!["MR".into()], - requirement_approval_event_id: requirement_approval.event_id, - auth: auth(), - now: at(9), - }) - .unwrap(); + let refreshed = store.list_for_ticket("W", "T").unwrap(); + assert_eq!( + requirement_approval( + &refreshed, + "T:1", + &snapshot, + Some(&requirement_review.event_id), + ) + .unwrap(), + &requirement_review + ); + assert_eq!( + requirement_approval(&refreshed, "T:1", &snapshot, None).unwrap(), + &requirement_review + ); + assert_eq!(refreshed[0].merged_result().unwrap(), &merge); + assert_eq!( + refreshed[0].integration_approval().unwrap(), + &integration_approval + ); + let connection = Connection::open(dir.path().join("db")).unwrap(); + let merge_payload: String = connection.query_row( + "SELECT payload_json FROM merge_request_thread_events WHERE workspace_id='W' AND merge_request_id='MR' AND kind='merge'", + [], + |row| row.get(0), + ).unwrap(); + assert_eq!(merge_payload, serde_json::to_string(&merge).unwrap()); } #[test] @@ -980,19 +985,19 @@ fn review_request_rejects_a_snapshot_that_omits_a_linked_merge_request() { } #[test] -fn partial_integration_retains_ticket_and_assignment_until_guarded_ticket_completion() { +fn partial_and_full_integration_retain_ticket_state_and_active_assignment() { let (dir, store) = fixture(); open_for(&store, "MR", "R"); open_for(&store, "MR-2", "R2"); let first = approve_for(&store, "MR", "R", "subject-one", "token-one"); let second = approve_for(&store, "MR-2", "R2", "subject-two", "token-two"); - store + let first_merge = store .complete(CompleteMergeRequest { merge_request_id: "MR".into(), ticket_id: "T".into(), operation_id: "merge-one".into(), - approval_event_id: first.event_id, + approval_event_id: first.event_id.clone(), current_subject_ref: "subject-one".into(), target_ref_before: "target-one-before".into(), target_ref_after: "target-one-after".into(), @@ -1002,30 +1007,33 @@ fn partial_integration_retains_ticket_and_assignment_until_guarded_ticket_comple now: at(5), }) .unwrap(); - let connection = Connection::open(dir.path().join("db")).unwrap(); - 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"); - assert!(assigned); - assert!(matches!( - store.complete_ticket(CompleteTicket { - ticket_id: "T".into(), - operation_id: "ticket-complete".into(), - item_revision: "t".into(), - merge_request_ids: vec!["MR".into(), "MR-2".into()], - requirement_approval_event_id: second.event_id.clone(), - auth: auth(), - now: at(6), - }), - Err(MergeRequestError::NotReady(_)) - )); + let partial = store.get_by_id("W", "MR-2").unwrap(); + assert_eq!( + partial.integration_approval(), + Err(MergeRequestEvidenceError::NotMerged) + ); - store + let connection = Connection::open(dir.path().join("db")).unwrap(); + let assert_ticket_unchanged = || { + let (state, assigned, active, state_changes): (String, bool, bool, i64) = connection + .query_row( + "SELECT workflow_state, + EXISTS(SELECT 1 FROM ticket_current_worker_assignments WHERE workspace_id='W' AND ticket_id='T'), + EXISTS(SELECT 1 FROM ticket_active_worker_assignments WHERE workspace_id='W' AND ticket_id='T'), + (SELECT COUNT(*) FROM typed_ticket_events WHERE workspace_id='W' AND ticket_id='T' AND kind='state_changed') + FROM typed_tickets WHERE workspace_id='W' AND ticket_id='T'", + [], + |row| Ok((row.get(0)?, row.get(1)?, row.get(2)?, row.get(3)?)), + ) + .unwrap(); + assert_eq!(state, "inprogress"); + assert!(assigned); + assert!(active); + assert_eq!(state_changes, 0); + }; + assert_ticket_unchanged(); + + let second_merge = store .complete(CompleteMergeRequest { merge_request_id: "MR-2".into(), ticket_id: "T".into(), @@ -1040,69 +1048,84 @@ fn partial_integration_retains_ticket_and_assignment_until_guarded_ticket_comple now: at(7), }) .unwrap(); - assert!(matches!( - store.complete_ticket(CompleteTicket { - ticket_id: "T".into(), - operation_id: "ticket-complete".into(), - item_revision: "stale".into(), - merge_request_ids: vec!["MR".into(), "MR-2".into()], - requirement_approval_event_id: second.event_id.clone(), - auth: auth(), - now: at(8), - }), - Err(MergeRequestError::Conflict(_)) - )); - assert!(matches!( - store.complete_ticket(CompleteTicket { - ticket_id: "T".into(), - operation_id: "ticket-complete-missing-attestation".into(), - item_revision: "t".into(), - merge_request_ids: vec!["MR".into(), "MR-2".into()], - requirement_approval_event_id: "missing-review".into(), - auth: auth(), - now: at(8), - }), - Err(MergeRequestError::NotReady(_)) - )); - assert!(matches!( - store.complete_ticket(CompleteTicket { - ticket_id: "T".into(), - operation_id: "ticket-complete".into(), - item_revision: "t".into(), - merge_request_ids: vec!["MR".into()], - requirement_approval_event_id: second.event_id.clone(), - auth: auth(), - now: at(8), - }), - Err(MergeRequestError::Conflict(_)) - )); - let completed = store - .complete_ticket(CompleteTicket { - ticket_id: "T".into(), - operation_id: "ticket-complete".into(), - item_revision: "t".into(), - merge_request_ids: vec!["MR-2".into(), "MR".into()], - requirement_approval_event_id: second.event_id.clone(), - auth: auth(), - now: at(9), + assert_ticket_unchanged(); + let requests = store.list_for_ticket("W", "T").unwrap(); + let snapshot = requests + .iter() + .map(|request| MergeRequestReviewSubject { + merge_request_id: request.merge_request_id.clone(), + subject_ref: request.merged_result().unwrap().approved_source_ref.clone(), }) + .collect::>(); + assert_eq!( + requirement_approval(&requests, "t", &snapshot, Some(&second.event_id)).unwrap(), + &second + ); + let first_request = requests + .iter() + .find(|request| request.merge_request_id == "MR") .unwrap(); - let replay = store - .complete_ticket(CompleteTicket { - ticket_id: "T".into(), - operation_id: "ticket-complete".into(), - item_revision: "t".into(), - merge_request_ids: vec!["MR".into(), "MR-2".into()], - requirement_approval_event_id: second.event_id.clone(), - auth: auth(), - now: at(10), - }) + let second_request = requests + .iter() + .find(|request| request.merge_request_id == "MR-2") .unwrap(); - assert_eq!(replay, completed); + assert_eq!(first_request.merged_result().unwrap(), &first_merge); + assert_eq!(second_request.merged_result().unwrap(), &second_merge); + assert_eq!(first_request.integration_approval().unwrap(), &first); + assert_eq!(second_request.integration_approval().unwrap(), &second); } #[test] -fn persisted_merged_result_requires_matching_nonrevoked_requested_approval_before_completion() { +fn historical_ticket_completion_payload_remains_readable_and_unchanged_on_migration() { + let (dir, _store) = fixture(); + let connection = Connection::open(dir.path().join("db")).unwrap(); + let payload = r#"{ + "operation_id": "historical-operation", + "ticket_id": "T", + "item_revision": "historical-revision", + "merge_request_ids": ["MR-2", "MR"], + "requirement_approval_event_id": "historical-review", + "completed_by": {"runtime_id": "runtime", "worker_id": "coder"}, + "created_at": "2026-07-26T12:00:07Z" + }"#; + connection.execute_batch( + "UPDATE typed_tickets SET workflow_state='done' WHERE workspace_id='W' AND ticket_id='T'; + INSERT INTO typed_ticket_events VALUES('W','T',0,'state_changed','worker:runtime:coder','2026-07-26T12:00:07Z','inprogress','done','Ticket requirements completed','historical body');" + ).unwrap(); + connection + .execute( + "INSERT INTO typed_ticket_event_attributes VALUES('W','T',0,'ticket_completion',?1)", + [payload], + ) + .unwrap(); + + migrate(&connection).unwrap(); + + let (saved_payload, kind, from_state, to_state, heading, body, state): (String, String, String, String, String, String, String) = connection.query_row( + "SELECT attribute.value,event.kind,event.from_state,event.to_state,event.heading,event.body,ticket.workflow_state + FROM typed_ticket_event_attributes attribute + JOIN typed_ticket_events event USING(workspace_id,ticket_id,event_index) + JOIN typed_tickets ticket USING(workspace_id,ticket_id) + WHERE attribute.workspace_id='W' AND attribute.ticket_id='T' AND attribute.key='ticket_completion'", + [], + |row| Ok((row.get(0)?,row.get(1)?,row.get(2)?,row.get(3)?,row.get(4)?,row.get(5)?,row.get(6)?)), + ).unwrap(); + assert_eq!(saved_payload, payload); + assert_eq!(kind, "state_changed"); + assert_eq!(from_state, "inprogress"); + assert_eq!(to_state, "done"); + assert_eq!(heading, "Ticket requirements completed"); + assert_eq!(body, "historical body"); + assert_eq!(state, "done"); + let historical: TicketCompletionEvent = serde_json::from_str(&saved_payload).unwrap(); + assert_eq!( + serde_json::to_value(&historical).unwrap(), + serde_json::from_str::(payload).unwrap() + ); +} + +#[test] +fn persisted_integration_evidence_rejects_missing_mismatched_revoked_or_late_approval() { for (case, expected) in [ ( "merge-missing", @@ -1207,30 +1230,6 @@ fn persisted_merged_result_requires_matching_nonrevoked_requested_approval_befor } 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'", [], diff --git a/crates/server-api/src/lib.rs b/crates/server-api/src/lib.rs index 254c61a24..70f13d1f6 100644 --- a/crates/server-api/src/lib.rs +++ b/crates/server-api/src/lib.rs @@ -324,7 +324,6 @@ impl_openapi_schema!( SubmitMergeRequestReviewRequest, TextResponse, TicketCloseRecordRequest, - TicketCompletionEvent, TicketDependencyCheckResponse, TicketDetail, TicketDoctorResponse, @@ -350,6 +349,7 @@ impl_openapi_schema!( TicketRoleAssignmentsResponse, TicketShowRequest, TicketStateChangeRequest, + TicketStateUpdateRequest, TicketSummarySearchQuery, TicketThreadEventRequest, RuntimeManagementApiError, @@ -2917,7 +2917,7 @@ pub trait ServerApi { #[path] id: String, #[body] request: TicketThreadEventRequest, ) -> Result<(), RepositoryApiError>; - #[post("/api/w/{workspace_id}/tickets/{id}/state-changes", status = 204, error_status = 400, additional_error_statuses = [401, 403, 404, 409, 500], bearer_auth = true, browser_auth = true)] + #[post("/api/w/{workspace_id}/tickets/{id}/state-changes", status = 204, error_status = 400, additional_error_statuses = [401, 403, 404, 409, 422, 500], bearer_auth = true, browser_auth = true)] async fn ticket_state_change_add( &self, #[extension] context: ServerRequestContext, @@ -2933,7 +2933,7 @@ pub trait ServerApi { #[path] id: String, #[body] request: TicketIntakeSummaryRequest, ) -> Result<(), RepositoryApiError>; - #[post("/api/w/{workspace_id}/tickets/{id}/state-fields/{field}", status = 204, error_status = 400, additional_error_statuses = [401, 403, 404, 409, 500], bearer_auth = true, browser_auth = true)] + #[post("/api/w/{workspace_id}/tickets/{id}/state-fields/{field}", status = 204, error_status = 400, additional_error_statuses = [401, 403, 404, 409, 422, 500], bearer_auth = true, browser_auth = true)] async fn ticket_state_field_set( &self, #[extension] context: ServerRequestContext, @@ -2942,7 +2942,7 @@ pub trait ServerApi { #[path] field: String, #[body] request: TicketStateChangeRequest, ) -> Result<(), RepositoryApiError>; - #[post("/api/w/{workspace_id}/tickets/{id}/workflow-state", status = 204, error_status = 400, additional_error_statuses = [401, 403, 404, 409, 500], bearer_auth = true, browser_auth = true)] + #[post("/api/w/{workspace_id}/tickets/{id}/workflow-state", status = 204, error_status = 400, additional_error_statuses = [401, 403, 404, 409, 422, 500], bearer_auth = true, browser_auth = true)] async fn ticket_workflow_state_set( &self, #[extension] context: ServerRequestContext, @@ -2950,6 +2950,14 @@ pub trait ServerApi { #[path] id: String, #[body] request: TicketStateChangeRequest, ) -> Result<(), RepositoryApiError>; + #[post("/api/w/{workspace_id}/tickets/{id}/state-update", status = 200, error_status = 400, additional_error_statuses = [401, 403, 404, 409, 422, 500], bearer_auth = true, browser_auth = true)] + async fn ticket_state_update( + &self, + #[extension] context: ServerRequestContext, + #[path] workspace_id: String, + #[path] id: String, + #[body] request: TicketStateUpdateRequest, + ) -> Result; #[post("/api/w/{workspace_id}/tickets/{id}/workflow/mark-ready", status = 200, error_status = 400, additional_error_statuses = [401, 403, 404, 409, 500], bearer_auth = true, browser_auth = true)] async fn ticket_mark_ready_record( &self, @@ -3047,15 +3055,15 @@ pub trait ServerApi { #[path] id: String, #[body] request: CompleteMergeRequestRequest, ) -> Result; - #[post("/api/w/{workspace_id}/tickets/{id}/complete", status = 200, error_status = 400, additional_error_statuses = [401, 403, 404, 409, 500], openapi = false)] + #[post("/api/w/{workspace_id}/tickets/{id}/complete", status = 200, error_status = 400, additional_error_statuses = [401, 403, 404, 409, 422, 500], bearer_auth = true, browser_auth = true)] async fn ticket_complete( &self, #[extension] context: ServerRequestContext, #[path] workspace_id: String, #[path] id: String, #[body] request: CompleteTicketRequest, - ) -> Result; - #[post("/api/w/{workspace_id}/tickets/{id}/workflow/close", status = 204, error_status = 400, additional_error_statuses = [401, 403, 404, 409, 500], bearer_auth = true, browser_auth = true)] + ) -> Result; + #[post("/api/w/{workspace_id}/tickets/{id}/workflow/close", status = 204, error_status = 400, additional_error_statuses = [401, 403, 404, 409, 422, 500], bearer_auth = true, browser_auth = true)] async fn ticket_close_record( &self, #[extension] context: ServerRequestContext, @@ -3150,9 +3158,10 @@ pub trait ServerApi { #[body] request: CancelTicketImplementationRequest, ) -> Result; - #[post("/api/w/{workspace_id}/tickets/{id}/state", status = 200, error_status = 400, additional_error_statuses = [401, 403, 404, 409, 500], bearer_auth = true, browser_auth = true)] + #[post("/api/w/{workspace_id}/tickets/{id}/state", status = 200, error_status = 400, additional_error_statuses = [401, 403, 404, 409, 422, 500], bearer_auth = true, browser_auth = true)] async fn ticket_state_transition( &self, + #[extension] context: ServerRequestContext, #[path] workspace_id: String, #[path] id: String, #[body] request: BrowserTransitionTicketStateRequest, @@ -3161,6 +3170,7 @@ pub trait ServerApi { #[post("/api/w/{workspace_id}/tickets/{id}/ready", status = 200, error_status = 400, additional_error_statuses = [401, 403, 404, 409, 500], bearer_auth = true, browser_auth = true)] async fn ticket_ready( &self, + #[extension] context: ServerRequestContext, #[path] workspace_id: String, #[path] id: String, #[body] request: TicketMarkReadyRequest, @@ -3177,14 +3187,16 @@ pub trait ServerApi { #[post("/api/w/{workspace_id}/tickets/{id}/queue", status = 200, error_status = 400, additional_error_statuses = [401, 403, 404, 409, 500], bearer_auth = true, browser_auth = true)] async fn ticket_queue( &self, + #[extension] context: ServerRequestContext, #[path] workspace_id: String, #[path] id: String, #[body] request: BrowserQueueTicketRequest, ) -> Result; - #[post("/api/w/{workspace_id}/tickets/{id}/close", status = 200, error_status = 400, additional_error_statuses = [401, 403, 404, 409, 500], bearer_auth = true, browser_auth = true)] + #[post("/api/w/{workspace_id}/tickets/{id}/close", status = 200, error_status = 400, additional_error_statuses = [401, 403, 404, 409, 422, 500], bearer_auth = true, browser_auth = true)] async fn ticket_close( &self, + #[extension] context: ServerRequestContext, #[path] workspace_id: String, #[path] id: String, #[body] request: BrowserCloseTicketRequest, @@ -7647,9 +7659,10 @@ transparent_ticket_dto!( transparent_ticket_dto!(CreateTicketRecordRequest, ticket::NewTicket); transparent_ticket_dto!(EditTicketRecordItemRequest, ticket::TicketItemEdit); transparent_ticket_dto!(TicketThreadEventRequest, ticket::NewTicketEvent); -transparent_ticket_dto!(TicketStateChangeRequest, ticket::TicketStateChange); +transparent_ticket_dto!(TicketStateChangeRequest, ticket::TicketStateUpdate); +transparent_ticket_dto!(TicketStateUpdateRequest, ticket::TicketStateUpdate); transparent_ticket_dto!(TicketIntakeSummaryRequest, ticket::TicketIntakeSummary); -transparent_ticket_dto!(TicketCloseRecordRequest, ticket::MarkdownText); +transparent_ticket_dto!(TicketCloseRecordRequest, ticket::TicketCompletion); transparent_ticket_dto!(CreateTicketRelationRequest, ticket::NewTicketRelation); transparent_ticket_dto!( CreateTicketOrchestrationPlanRequest, @@ -7774,9 +7787,11 @@ pub struct BrowserEditTicketRequest { #[serde(deny_unknown_fields)] pub struct BrowserTransitionTicketStateRequest { pub state: BrowserTicketWorkflowState, - pub reason: Option, + pub operation_key: String, + pub expected_item_revision: String, + pub expected_state: BrowserTicketWorkflowState, + pub reason: String, pub body: Option, - pub author: Option, } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, JsonSchema)] @@ -7807,6 +7822,9 @@ pub struct BrowserQueueTicketRequest {} #[cfg_attr(feature = "typescript", derive(ts_rs::TS))] #[serde(deny_unknown_fields)] pub struct BrowserCloseTicketRequest { + pub operation_key: String, + pub expected_item_revision: String, + pub expected_state: BrowserTicketWorkflowState, pub resolution: String, } @@ -8185,22 +8203,12 @@ pub struct CompleteMergeRequestRequest { #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, JsonSchema)] #[serde(deny_unknown_fields)] pub struct CompleteTicketRequest { - pub operation_id: String, - pub item_revision: String, - pub merge_request_ids: Vec, - pub requirement_approval_event_id: String, -} - -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, JsonSchema)] -#[serde(deny_unknown_fields)] -pub struct TicketCompletionEvent { - pub operation_id: String, - pub ticket_id: String, - pub item_revision: String, - pub merge_request_ids: Vec, - pub requirement_approval_event_id: String, - pub completed_by: MergeRequestWorkerIdentity, - pub created_at: String, + pub operation_key: String, + pub expected_item_revision: String, + pub expected_state: ticket::TicketWorkflowState, + pub reason: String, + #[serde(default)] + pub references: Vec, } #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] @@ -11461,13 +11469,6 @@ mod tests { #[test] fn ticket_objective_and_merge_request_operations_are_in_server_api_metadata() { let operations = ServerApiMetadata::OPERATIONS; - assert_eq!( - operations - .iter() - .filter(|operation| operation.operation_id.starts_with("ticket_")) - .count(), - 38 - ); assert_eq!( operations .iter() @@ -11483,6 +11484,16 @@ mod tests { 11 ); for (operation_id, method, path) in [ + ( + "ticket_state_update", + HttpMethod::Post, + "/api/w/{workspace_id}/tickets/{id}/state-update", + ), + ( + "ticket_complete", + HttpMethod::Post, + "/api/w/{workspace_id}/tickets/{id}/complete", + ), ( "ticket_relation_query", HttpMethod::Post, @@ -12540,6 +12551,34 @@ mod tests { assert!(serde_json::from_value::(unknown).is_err()); } + #[test] + fn ticket_completion_wire_requires_cas_and_judgment_not_merge_evidence() { + let request = serde_json::json!({ + "operation_key": "investigation-done", + "expected_item_revision": "ticket:1", + "expected_state": "planning", + "reason": "Question answered in the thread" + }); + let parsed: CompleteTicketRequest = serde_json::from_value(request.clone()).unwrap(); + assert!(parsed.references.is_empty()); + for field in [ + "operation_key", + "expected_item_revision", + "expected_state", + "reason", + ] { + let mut missing = request.clone(); + missing.as_object_mut().unwrap().remove(field); + assert!( + serde_json::from_value::(missing).is_err(), + "{field}" + ); + } + let mut old = request; + old["requirement_approval_event_id"] = serde_json::json!("not-a-judgment"); + assert!(serde_json::from_value::(old).is_err()); + } + #[cfg(feature = "typescript")] #[test] fn generated_catalog_typescript_keeps_public_wrappers_and_nullability() { @@ -13388,7 +13427,6 @@ mod openapi_artifact_tests { "merge_request_review_revoke", "merge_request_review_submit", "merge_request_reviewer_child_register", - "ticket_complete", "runtime_resource_fetch", "worker_control_cancel", "worker_control_input", diff --git a/crates/ticket/src/lib.rs b/crates/ticket/src/lib.rs index 8f41eecc8..c2d86bff7 100644 --- a/crates/ticket/src/lib.rs +++ b/crates/ticket/src/lib.rs @@ -89,6 +89,8 @@ pub enum TicketError { InvalidReadWriteTargetCount(usize), #[error("ticket target authority is unavailable")] TargetAuthorityUnavailable, + #[error("stale ticket item revision: expected `{expected}`, found `{actual}`")] + StaleItemRevision { expected: String, actual: String }, #[error("stale ticket workflow state: expected `{expected}`, found `{actual}`")] StaleWorkflowState { expected: String, actual: String }, #[error("invalid ticket workflow transition `{from}` -> `{to}`")] @@ -282,18 +284,13 @@ impl TicketWorkflowState { from == Self::Planning && to == Self::Ready } - pub fn is_queue_transition(from: Self, to: Self) -> bool { - from == Self::Ready && to == Self::Queued + pub fn is_queue_transition(_from: Self, to: Self) -> bool { + to == Self::Queued } + /// Compatibility helper for internal callers; state is not a role phase path. pub fn is_role_transition(from: Self, to: Self) -> bool { - matches!( - (from, to), - (Self::Queued, Self::InProgress) - | (Self::InProgress, Self::Done) - | (Self::Ready, Self::Planning) - | (Self::Queued, Self::Planning) - ) + from != to } } @@ -473,6 +470,66 @@ impl TicketStateChange { } } +/// Audited completion independent of repositories, Git, or Merge Requests. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, schemars::JsonSchema)] +#[serde(deny_unknown_fields)] +pub struct TicketCompletion { + pub expected_item_revision: String, + pub expected_state: TicketWorkflowState, + pub operation_key: String, + pub reason: String, + #[serde(default)] + pub references: Vec, + #[serde(default)] + pub author: Option, +} + +/// External state mutation with item/state CAS and an immutable operation receipt. +/// Exact replay returns the current authoritative Ticket without a new event or +/// reapplying the recorded state, even after later edits or reopening. +/// Setting `state` to Closed also stores `reason` as the resolution; reopening +/// changes Ticket metadata only and never revives released work identities. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, schemars::JsonSchema)] +#[serde(deny_unknown_fields)] +pub struct TicketStateUpdate { + pub expected_item_revision: String, + pub expected_state: TicketWorkflowState, + pub state: TicketWorkflowState, + pub operation_key: String, + pub reason: String, + #[serde(default)] + pub references: Vec, + #[serde(default)] + pub author: Option, +} + +impl From for TicketStateUpdate { + fn from(request: TicketCompletion) -> Self { + Self { + expected_item_revision: request.expected_item_revision, + expected_state: request.expected_state, + state: TicketWorkflowState::Done, + operation_key: request.operation_key, + reason: request.reason, + references: request.references, + author: request.author, + } + } +} + +/// Canonical item revision shared by Ticket authority, item checkers, and Merge +/// Request review records. Only persisted item_edit event indexes advance it. +pub fn ticket_item_revision(ticket: &Ticket) -> String { + let index = ticket + .events + .iter() + .filter(|event| event.kind == TicketEventKind::Other("item_edit".to_owned())) + .filter_map(|event| event.attributes.get("event_sequence")?.parse::().ok()) + .max() + .unwrap_or(0); + format!("{}:{index}", ticket.meta.id) +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, schemars::JsonSchema)] pub struct TicketIntakeSummary { pub author: Option, @@ -585,7 +642,7 @@ pub enum TicketTargetsEdit { #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct ResolvedTicketTarget { pub repository_key: String, - pub ref_selector: String, + pub ref_selector: Option, pub access: TicketTargetAccess, } @@ -593,7 +650,8 @@ pub struct ResolvedTicketTarget { /// /// Ticket storage never infers repositories from cwd or repository paths. The /// Workspace Backend supplies this boundary from its authoritative repository -/// catalog. Backends without it fail closed for ready/queue transitions. +/// catalog, validating repository identity and selector syntax without Git +/// resolution. Empty targets are valid in every Ticket state. pub trait TicketTargetAuthority: Send + Sync { fn resolve_target( &self, @@ -645,12 +703,8 @@ fn resolve_ready_targets( targets: &[TicketTarget], ) -> Result> { validate_ticket_targets(targets)?; - let read_write_count = targets - .iter() - .filter(|target| target.access == TicketTargetAccess::ReadWrite) - .count(); - if read_write_count == 0 { - return Err(TicketError::InvalidReadWriteTargetCount(read_write_count)); + if targets.is_empty() { + return Ok(Vec::new()); } let authority = authority.ok_or(TicketError::TargetAuthorityUnavailable)?; let mut resolved = Vec::with_capacity(targets.len()); @@ -662,7 +716,9 @@ fn resolve_ready_targets( target.ref_selector.as_deref(), )?; validate_required_event_value("repository_key", &canonical.repository_key)?; - validate_required_event_value("ref_selector", &canonical.ref_selector)?; + if let Some(selector) = canonical.ref_selector.as_deref() { + validate_required_event_value("ref_selector", selector)?; + } canonical.access = target.access; if !repositories.insert(canonical.repository_key.clone()) { return Err(TicketError::DuplicateTargetRepository( @@ -679,7 +735,7 @@ fn canonical_ticket_targets(targets: &[ResolvedTicketTarget]) -> Vec Result<()> { - if current == TicketWorkflowState::Planning && to == TicketWorkflowState::Ready - || current == TicketWorkflowState::Planning && to == TicketWorkflowState::InProgress - || current == TicketWorkflowState::Ready && to == TicketWorkflowState::Queued - || current == TicketWorkflowState::Ready && to == TicketWorkflowState::InProgress - { - return Err(TicketError::InvalidWorkflowTransition { - from: current.as_str().to_owned(), - to: to.as_str().to_owned(), - }); - } - if !TicketWorkflowState::is_role_transition(current, to) { + if current == to { return Err(TicketError::InvalidWorkflowTransition { from: current.as_str().to_owned(), to: to.as_str().to_owned(), @@ -1260,72 +1318,14 @@ pub fn project_ticket_workspace_item( } pub fn ticket_queue_guard( - summary: &TicketSummary, + _summary: &TicketSummary, relation_blockers: &[TicketRelationBlocker], - orchestration_overlay: Option<&TicketWorkspaceStateOverlay>, + _orchestration_overlay: Option<&TicketWorkspaceStateOverlay>, ) -> TicketQueueGuard { - if orchestration_overlay.is_some() { - return TicketQueueGuard { - can_queue_for_orchestrator: false, - reason: Some( - "orchestration overlay already shows progress; duplicate queue is suppressed" - .to_string(), - ), - blocked_reason: None, - }; - } - if summary.workflow_state != TicketWorkflowState::Ready { - return TicketQueueGuard { - can_queue_for_orchestrator: false, - reason: Some(format!( - "Ticket state is {}; only ready Tickets can be queued for Orchestrator", - summary.workflow_state.as_str() - )), - blocked_reason: None, - }; - } - if relation_blockers - .iter() - .any(|blocker| blocker.blocking_ticket == summary.id) - { - return TicketQueueGuard { - can_queue_for_orchestrator: false, - reason: Some("Dependency cycle must be resolved before Queue".to_string()), - blocked_reason: Some(format!( - "Ticket {} is part of a dependency cycle", - summary.id - )), - }; - } - if relation_blockers - .iter() - .any(|blocker| blocker.blocking_state == TicketWorkflowState::Planning) - { - let blocked_reason = relation_blockers - .iter() - .filter(|blocker| blocker.blocking_state == TicketWorkflowState::Planning) - .map(|blocker| { - format!( - "{} ({})", - blocker.blocking_ticket, - blocker.blocking_state.as_str() - ) - }) - .collect::>() - .join(", "); - return TicketQueueGuard { - can_queue_for_orchestrator: false, - reason: Some("Dependencies must leave planning before Queue can proceed".to_string()), - blocked_reason: Some(blocked_reason), - }; - } TicketQueueGuard { can_queue_for_orchestrator: true, - reason: (!relation_blockers.is_empty()).then(|| { - "Ready dependencies will be queued atomically; active dependencies remain unchanged" - .to_string() - }), - blocked_reason: None, + reason: Some("Queue submits only the explicitly requested Ticket; state and dependencies are diagnostic context".to_owned()), + blocked_reason: (!relation_blockers.is_empty()).then(|| format_relation_blockers(relation_blockers)), } } @@ -1333,177 +1333,24 @@ fn derive_ticket_workspace_projection( summary: &TicketSummary, relation_blockers: &[TicketRelationBlocker], ) -> TicketWorkspaceProjection { - if !relation_blockers.is_empty() { - let active_blockers = relation_blockers - .iter() - .filter(|blocker| { - blocker.blocking_ticket == summary.id - || !relation_blocker_allows_ready_queue(blocker) - }) - .collect::>(); - if summary.workflow_state != TicketWorkflowState::Ready || !active_blockers.is_empty() { - let blockers_to_report = if active_blockers.is_empty() { - relation_blockers.iter().collect::>() - } else { - active_blockers - }; - let blockers = format_workspace_relation_blockers(&blockers_to_report); - let waiting_reason = format!("waiting for {blockers}"); - return TicketWorkspaceProjection { - kind: workspace_row_kind_for_state(summary.workflow_state), - priority: match summary.workflow_state { - TicketWorkflowState::Queued | TicketWorkflowState::InProgress => { - TicketWorkspaceActionPriority::ActiveWork - } - _ => TicketWorkspaceActionPriority::Background, - }, - next_action: Some(TicketWorkspaceNextAction::WaitForOrchestrator), - visible_state: summary.workflow_state.as_str().to_string(), - visible_overlay: None, - disabled_reason: Some(format!( - "Dependency context: {waiting_reason}. The Orchestrator decides whether work waits or starts in parallel." - )), - key_hint: Some(format!("Dependencies: {waiting_reason}")), - blocked_reason: Some(blockers), - queue_guard: TicketQueueGuard { - can_queue_for_orchestrator: false, - reason: Some(waiting_reason), - blocked_reason: None, - }, - }; - } - - let blockers = format_workspace_relation_blockers( - &relation_blockers - .iter() - .collect::>(), - ); - let mut queue_targets = vec![summary.id.clone()]; - queue_targets.extend( - relation_blockers - .iter() - .filter(|blocker| blocker.blocking_state == TicketWorkflowState::Ready) - .map(|blocker| blocker.blocking_ticket.clone()), - ); - queue_targets.sort(); - queue_targets.dedup(); - return TicketWorkspaceProjection { - kind: TicketWorkspaceRowKind::Ticket, - priority: TicketWorkspaceActionPriority::ReadyForQueue, - next_action: Some(TicketWorkspaceNextAction::QueueForOrchestrator), - visible_state: summary.workflow_state.as_str().to_string(), - visible_overlay: None, - disabled_reason: None, - key_hint: Some(format!( - "Queue targets: {}; active dependencies remain orchestration context ({blockers}).", - queue_targets.join(", ") - )), - blocked_reason: Some(blockers), - queue_guard: TicketQueueGuard { - can_queue_for_orchestrator: true, - reason: None, - blocked_reason: None, - }, - }; - } - - match summary.workflow_state { - TicketWorkflowState::Ready => TicketWorkspaceProjection { - kind: TicketWorkspaceRowKind::Ticket, - priority: TicketWorkspaceActionPriority::ReadyForQueue, - next_action: Some(TicketWorkspaceNextAction::QueueForOrchestrator), - visible_state: summary.workflow_state.as_str().to_string(), - visible_overlay: None, - disabled_reason: None, - key_hint: Some( - "Queue transitions ready -> queued and may notify Orchestrator".to_string(), - ), - blocked_reason: None, - queue_guard: TicketQueueGuard { - can_queue_for_orchestrator: true, - reason: None, - blocked_reason: None, - }, - }, - TicketWorkflowState::Queued => TicketWorkspaceProjection { - kind: TicketWorkspaceRowKind::ActiveWork, - priority: TicketWorkspaceActionPriority::ActiveWork, - next_action: Some(TicketWorkspaceNextAction::WaitForOrchestrator), - visible_state: summary.workflow_state.as_str().to_string(), - visible_overlay: None, - disabled_reason: Some("Ticket is queued for Orchestrator routing.".to_string()), - key_hint: None, - blocked_reason: None, - queue_guard: TicketQueueGuard { - can_queue_for_orchestrator: false, - reason: Some("Ticket is already queued for Orchestrator routing".to_string()), - blocked_reason: None, - }, - }, - TicketWorkflowState::InProgress => TicketWorkspaceProjection { - kind: TicketWorkspaceRowKind::ActiveWork, - priority: TicketWorkspaceActionPriority::ActiveWork, - next_action: Some(TicketWorkspaceNextAction::WaitForOrchestrator), - visible_state: summary.workflow_state.as_str().to_string(), - visible_overlay: None, - disabled_reason: Some("Ticket is already in progress.".to_string()), - key_hint: None, - blocked_reason: None, - queue_guard: TicketQueueGuard { - can_queue_for_orchestrator: false, - reason: Some("Ticket is already in progress".to_string()), - blocked_reason: None, - }, - }, - TicketWorkflowState::Done => TicketWorkspaceProjection { - kind: TicketWorkspaceRowKind::Review, - priority: TicketWorkspaceActionPriority::Background, - next_action: Some(TicketWorkspaceNextAction::Close), - visible_state: summary.workflow_state.as_str().to_string(), - visible_overlay: None, - disabled_reason: Some( - "state is done; close if a resolution is still missing.".to_string(), - ), - key_hint: None, - blocked_reason: None, - queue_guard: TicketQueueGuard { - can_queue_for_orchestrator: false, - reason: Some("Ticket is done; close or review instead of queueing".to_string()), - blocked_reason: None, - }, - }, - TicketWorkflowState::Planning => TicketWorkspaceProjection { - kind: TicketWorkspaceRowKind::Planning, - priority: TicketWorkspaceActionPriority::Background, - next_action: Some(TicketWorkspaceNextAction::Clarify), - visible_state: summary.workflow_state.as_str().to_string(), - visible_overlay: None, - disabled_reason: Some( - "Ticket is still in planning; mark it ready before queueing.".to_string(), - ), - key_hint: Some("Planning/Intake helpers can set state = ready".to_string()), - blocked_reason: None, - queue_guard: TicketQueueGuard { - can_queue_for_orchestrator: false, - reason: Some("Ticket is still in planning".to_string()), - blocked_reason: None, - }, - }, - TicketWorkflowState::Closed => TicketWorkspaceProjection { - kind: TicketWorkspaceRowKind::Review, - priority: TicketWorkspaceActionPriority::Background, - next_action: Some(TicketWorkspaceNextAction::WaitForOrchestrator), - visible_state: summary.workflow_state.as_str().to_string(), - visible_overlay: None, - disabled_reason: Some("Ticket is closed.".to_string()), - key_hint: None, - blocked_reason: None, - queue_guard: TicketQueueGuard { - can_queue_for_orchestrator: false, - reason: Some("Ticket is closed".to_string()), - blocked_reason: None, - }, + TicketWorkspaceProjection { + kind: workspace_row_kind_for_state(summary.workflow_state), + priority: if matches!( + summary.workflow_state, + TicketWorkflowState::Queued | TicketWorkflowState::InProgress + ) { + TicketWorkspaceActionPriority::ActiveWork + } else { + TicketWorkspaceActionPriority::ReadyForQueue }, + next_action: Some(TicketWorkspaceNextAction::QueueForOrchestrator), + visible_state: summary.workflow_state.to_string(), + visible_overlay: None, + disabled_reason: None, + key_hint: Some(format!("Queue only {} for Orchestrator", summary.id)), + blocked_reason: (!relation_blockers.is_empty()) + .then(|| format_relation_blockers(relation_blockers)), + queue_guard: ticket_queue_guard(summary, relation_blockers, None), } } @@ -1601,35 +1448,6 @@ fn compact_ticket_state_label(state: TicketWorkflowState) -> &'static str { } } -fn relation_blocker_allows_ready_queue(blocker: &TicketRelationBlocker) -> bool { - matches!( - blocker.blocking_state, - TicketWorkflowState::Ready | TicketWorkflowState::Queued | TicketWorkflowState::InProgress - ) -} - -fn format_workspace_relation_blockers(blockers: &[&TicketRelationBlocker]) -> String { - let shown_blockers = blockers.iter().take(3).count(); - let mut formatted = blockers - .iter() - .take(3) - .map(|blocker| { - format!( - "{} via {} (state: {})", - blocker.blocking_ticket, - blocker.reason_kind, - blocker.blocking_state.as_str() - ) - }) - .collect::>() - .join(", "); - let remaining_blockers = blockers.len().saturating_sub(shown_blockers); - if remaining_blockers > 0 { - formatted.push_str(&format!(" (+{remaining_blockers} more)")); - } - formatted -} - #[derive( Debug, Clone, @@ -1935,6 +1753,14 @@ pub trait TicketBackend { change: TicketStateChange, ) -> Result<()>; fn set_workflow_state(&self, id: TicketIdOrSlug, change: TicketStateChange) -> Result<()>; + fn update_state(&self, _ticket: &str, _request: TicketStateUpdate) -> Result { + Err(TicketError::Conflict( + "backend does not support audited state updates".to_owned(), + )) + } + fn complete(&self, ticket: &str, request: TicketCompletion) -> Result { + self.update_state(ticket, request.into()) + } fn mark_ready(&self, id: TicketIdOrSlug, request: TicketMarkReady) -> Result; fn queue_ready(&self, id: TicketIdOrSlug, queued_by: &str) -> Result; fn close(&self, id: TicketIdOrSlug, resolution: MarkdownText) -> Result<()>; @@ -2011,6 +1837,14 @@ pub enum TicketBackendOperation { id: TicketIdOrSlug, change: TicketStateChange, }, + UpdateState { + ticket: String, + request: TicketStateUpdate, + }, + Complete { + ticket: String, + request: TicketCompletion, + }, MarkReady { id: TicketIdOrSlug, request: TicketMarkReady, @@ -2116,6 +1950,12 @@ where backend.set_workflow_state(id, change)?; TicketBackendOperationResult::Unit } + TicketBackendOperation::UpdateState { ticket, request } => { + TicketBackendOperationResult::Ticket(backend.update_state(&ticket, request)?) + } + TicketBackendOperation::Complete { ticket, request } => { + TicketBackendOperationResult::Ticket(backend.complete(&ticket, request)?) + } TicketBackendOperation::MarkReady { id, request } => { TicketBackendOperationResult::Ticket(backend.mark_ready(id, request)?) } @@ -2512,6 +2352,203 @@ impl SqliteTicketBackend { finish_sqlite_transaction(&conn, op(&conn)) } + /// The server supplies these attributes from authenticated runtime context. + /// The live assignment lookup must stay inside the mutation's write transaction: + /// an earlier server precheck alone cannot fence a concurrent release. + fn require_active_source_assignment(&self, conn: &Connection, ticket_id: &str) -> Result<()> { + if self + .event_attributes + .get("source_actor_role") + .map(String::as_str) + != Some("coder") + && !self.event_attributes.contains_key("source_assignment_id") + { + return Ok(()); + } + let required = |key: &str| { + self.event_attributes + .get(key) + .filter(|value| !value.trim().is_empty()) + .ok_or_else(|| { + TicketError::Conflict(format!( + "Ticket mutation requires authenticated {key} for an active assignment" + )) + }) + }; + let runtime_id = required("source_runtime_id")?; + let worker_id = required("source_worker_id")?; + let assignment_id = required("source_assignment_id")?; + let active = conn.query_row( + "SELECT 1 FROM ticket_active_worker_assignments WHERE workspace_id = ?1 AND ticket_id = ?2 AND runtime_id = ?3 AND worker_id = ?4 AND assignment_id = ?5 LIMIT 1", + params![self.workspace_id, ticket_id, runtime_id, worker_id, assignment_id], + |_| Ok(()), + ).optional().map_err(sqlite_err)?.is_some(); + if !active { + return Err(TicketError::Conflict("Ticket mutation source assignment is no longer active for this exact runtime/worker/Ticket identity".to_owned())); + } + Ok(()) + } + + fn state_update_fingerprint( + &self, + ticket_id: &str, + request: &TicketStateUpdate, + body: &MarkdownText, + ) -> Result { + let source = [ + "source_runtime_id", + "source_worker_id", + "source_actor_role", + "source_assignment_id", + ] + .map(|key| self.event_attributes.get(key)); + let mut digest = Sha256::new(); + digest.update(b"ticket.state-update.v1\0"); + digest.update( + serde_json::to_vec(&(ticket_id, request, body, source)) + .map_err(|error| TicketError::Conflict(error.to_string()))?, + ); + Ok(digest + .finalize() + .iter() + .map(|byte| format!("{byte:02x}")) + .collect()) + } + + fn next_legacy_state_operation_key( + &self, + conn: &Connection, + ticket_id: &str, + ) -> Result { + let index: i64 = conn.query_row( + "SELECT COALESCE(MAX(event_index), -1) + 1 FROM typed_ticket_events WHERE workspace_id = ?1 AND ticket_id = ?2", + params![self.workspace_id, ticket_id], |row| row.get(0), + ).map_err(sqlite_err)?; + Ok(format!("legacy-state:{ticket_id}:{index}")) + } + + /// All callers supply a snapshot read inside this same write transaction. + /// Legacy adapters retain their Markdown bodies while sharing CAS, source + /// authorization, state/status/resolution writes, audit, and operation receipts. + fn apply_state_update( + &self, + conn: &Connection, + previous: Ticket, + request: TicketStateUpdate, + body: MarkdownText, + ) -> Result { + validate_required_event_value("operation_key", &request.operation_key)?; + validate_required_event_value("expected_item_revision", &request.expected_item_revision)?; + validate_state_change(&TicketStateChange { + from: request.expected_state.to_string(), + to: request.state.to_string(), + author: request.author.clone(), + reason: request.reason.clone(), + body: body.clone(), + references: request.references.clone(), + })?; + let ticket_id = &previous.meta.id; + let fingerprint = self.state_update_fingerprint(ticket_id, &request, &body)?; + if let Some(event) = previous + .events + .iter() + .find(|event| event.attributes.get("operation_key") == Some(&request.operation_key)) + { + if event + .attributes + .get("fingerprint_version") + .map(String::as_str) + != Some("state-update-v1") + || event.attributes.get("request_fingerprint") != Some(&fingerprint) + { + return Err(TicketError::OperationFingerprintMismatch { + operation_key: request.operation_key.clone(), + }); + } + // The immutable event is the receipt; replay returns current authority + // without appending an event or reapplying the receipt's old state. + return Ok(previous); + } + // Exact recorded replay is independent of today's live role/assignment. + // Parent transport authenticates identity before restoring receipt context. + self.require_active_source_assignment(conn, ticket_id)?; + let actual = ticket_item_revision(&previous); + if actual != request.expected_item_revision { + return Err(TicketError::StaleItemRevision { + expected: request.expected_item_revision.clone(), + actual, + }); + } + if previous.meta.workflow_state != request.expected_state { + return Err(TicketError::StaleWorkflowState { + expected: request.expected_state.to_string(), + actual: previous.meta.workflow_state.to_string(), + }); + } + let at = now_utc(); + let updated = conn.execute( + "UPDATE typed_tickets SET workflow_state = ?3, workflow_state_explicit = 1, status = CASE WHEN ?3 = 'closed' THEN 'closed' ELSE 'open' END, updated_at = ?4, resolution = CASE WHEN ?3 = 'closed' THEN ?5 ELSE resolution END WHERE workspace_id = ?1 AND ticket_id = ?2 AND workflow_state = ?6", + params![self.workspace_id, ticket_id, request.state.as_str(), at, body.as_str(), request.expected_state.as_str()], + ).map_err(sqlite_err)?; + if updated != 1 { + return Err(TicketError::Conflict( + "Ticket changed during state mutation".to_owned(), + )); + } + let kind = if request.state == TicketWorkflowState::Closed { + TicketEventKind::Close + } else { + TicketEventKind::StateChanged + }; + self.insert_event( + conn, + ticket_id, + &TicketEvent { + heading: Some(kind.heading()), + kind, + author: Some(request.author.clone().unwrap_or_else(default_author)), + at: Some(at), + status: Some( + if request.state == TicketWorkflowState::Closed { + "closed" + } else { + "open" + } + .to_owned(), + ), + from: Some(request.expected_state.to_string()), + to: Some(request.state.to_string()), + reason: Some(request.reason.clone()), + state_field: Some("state".to_owned()), + body, + references: request.references.clone(), + attributes: BTreeMap::from([ + ("operation_key".to_owned(), request.operation_key.clone()), + ("request_fingerprint".to_owned(), fingerprint), + ( + "fingerprint_version".to_owned(), + "state-update-v1".to_owned(), + ), + ( + "expected_item_revision".to_owned(), + request.expected_item_revision.clone(), + ), + ]), + }, + )?; + let result = self.load_ticket(conn, ticket_id)?; + if !result + .events + .iter() + .any(|event| event.attributes.get("operation_key") == Some(&request.operation_key)) + { + return Err(TicketError::Conflict( + "state update audit lost its operation key".to_owned(), + )); + } + Ok(result) + } + fn with_read(&self, op: impl FnOnce(&Connection) -> Result) -> Result { let conn = self.open_connection()?; op(&conn) @@ -3223,15 +3260,6 @@ impl TicketBackend for SqliteTicketBackend { } let ticket_id = self.resolve_ticket_id(conn, id)?; let previous = self.load_ticket(conn, &ticket_id)?; - if edit.targets.is_some() { - let current = previous.meta.workflow_state; - if current != TicketWorkflowState::Planning { - return Err(TicketError::Conflict(format!( - "ticket implementation target is locked after planning (current state: {})", - current.as_str() - ))); - } - } let now = now_utc(); let mut body_edit_audit = TicketBodyEditAudit::None; if let Some(title) = edit.title.as_ref() { @@ -3319,45 +3347,8 @@ impl TicketBackend for SqliteTicketBackend { let blockers = transitive_dependency_blockers(&ticket_id, &states, &relations)?; let summary = ticket_summary_from_meta(ticket.meta); let mut projection = project_ticket_workspace_item(&summary, &blockers, None); - let queue_tickets = if summary.workflow_state == TicketWorkflowState::Ready { - match dependency_queue_plan(&ticket_id, &states, &relations) { - Ok(queue_tickets) => { - let target_error = queue_tickets.iter().find_map(|candidate| { - self.load_ticket(conn, candidate) - .and_then(|ticket| { - resolve_ready_targets( - self.target_authority.as_ref(), - &self.workspace_id, - &ticket.meta.targets, - ) - }) - .err() - }); - if let Some(error) = target_error { - projection.queue_guard = TicketQueueGuard { - can_queue_for_orchestrator: false, - reason: Some( - "Queue dependency target validation failed".to_string(), - ), - blocked_reason: Some(error.to_string()), - }; - Vec::new() - } else { - queue_tickets - } - } - Err(error) => { - projection.queue_guard = TicketQueueGuard { - can_queue_for_orchestrator: false, - reason: Some("Queue dependency validation failed".to_string()), - blocked_reason: Some(error.to_string()), - }; - Vec::new() - } - } - } else { - Vec::new() - }; + projection.queue_guard = ticket_queue_guard(&summary, &blockers, None); + let queue_tickets = vec![ticket_id]; Ok(TicketDependencyCheck { ticket: summary, blockers, @@ -3430,14 +3421,18 @@ impl TicketBackend for SqliteTicketBackend { fn set_state_field( &self, id: TicketIdOrSlug, - _field: &str, + field: &str, change: TicketStateChange, ) -> Result<()> { + if field != "state" { + return Err(TicketError::Conflict(format!( + "unsupported Ticket state field: {field}" + ))); + } self.set_workflow_state(id, change) } fn set_workflow_state(&self, id: TicketIdOrSlug, change: TicketStateChange) -> Result<()> { - validate_state_change(&change)?; let from = TicketWorkflowState::parse(&change.from).ok_or_else(|| { TicketError::InvalidWorkflowTransition { from: change.from.clone(), @@ -3453,29 +3448,27 @@ impl TicketBackend for SqliteTicketBackend { validate_generic_state_change(from, to)?; self.with_write(|conn| { let ticket_id = self.resolve_ticket_id(conn, id)?; - let current = self.load_ticket(conn, &ticket_id)?.meta.workflow_state; - if current != from { - return Err(TicketError::StaleWorkflowState { - expected: from.as_str().to_owned(), - actual: current.as_str().to_owned(), - }); - } - if from == TicketWorkflowState::Queued && to == TicketWorkflowState::InProgress { - let ticket = self.load_ticket(conn, &ticket_id)?; - let blockers = ticket - .relations - .blockers - .into_iter() - .filter(|blocker| !relation_blocker_allows_queue(blocker)) - .collect::>(); - if !blockers.is_empty() { - return Err(TicketError::BlockingRelations(format_relation_blockers(&blockers))); - } - } - let at = now_utc(); - self.insert_event(conn, &ticket_id, &TicketEvent { kind: TicketEventKind::StateChanged, author: Some(change.author.clone().unwrap_or_else(default_author)), at: Some(at.clone()), status: None, from: Some(change.from), to: Some(change.to), reason: Some(change.reason), state_field: Some("state".to_string()), heading: Some(TicketEventKind::StateChanged.heading()), body: change.body, references: change.references, attributes: BTreeMap::new() })?; - conn.execute("UPDATE typed_tickets SET workflow_state = ?3, workflow_state_explicit = 1, updated_at = ?4, status = CASE WHEN ?3 = 'closed' THEN 'closed' ELSE status END WHERE workspace_id = ?1 AND ticket_id = ?2", params![self.workspace_id, ticket_id, to.as_str(), at]).map_err(sqlite_err)?; - Ok(()) + let previous = self.load_ticket(conn, &ticket_id)?; + let request = TicketStateUpdate { + expected_item_revision: ticket_item_revision(&previous), + expected_state: from, + state: to, + operation_key: self.next_legacy_state_operation_key(conn, &ticket_id)?, + reason: change.reason, + references: change.references, + author: Some(change.author.unwrap_or_else(default_author)), + }; + self.apply_state_update(conn, previous, request, change.body) + .map(|_| ()) + }) + } + + fn update_state(&self, ticket: &str, request: TicketStateUpdate) -> Result { + self.with_write(|conn| { + let ticket_id = self.resolve_ticket_id(conn, ticket.into())?; + let previous = self.load_ticket(conn, &ticket_id)?; + let body = MarkdownText::new(&request.reason); + self.apply_state_update(conn, previous, request, body) }) } @@ -3487,6 +3480,7 @@ impl TicketBackend for SqliteTicketBackend { if validate_mark_ready_replay(&ticket, &request)? { return Ok(ticket); } + self.require_active_source_assignment(conn, &ticket_id)?; if ticket.meta.workflow_state != TicketWorkflowState::Planning { return Err(TicketError::StaleWorkflowState { expected: TicketWorkflowState::Planning.as_str().to_owned(), @@ -3506,7 +3500,7 @@ impl TicketBackend for SqliteTicketBackend { format!( "{}@{} ({})", target.repository_key, - target.ref_selector, + target.ref_selector.as_deref().unwrap_or("(unspecified)"), target.access.as_str() ) }) @@ -3590,99 +3584,67 @@ impl TicketBackend for SqliteTicketBackend { validate_required_event_value("queued_by", queued_by)?; self.with_write(|conn| { let requested_ticket = self.resolve_ticket_id(conn, id)?; - let states = self.state_index(conn)?; - let relations = self.all_relations(conn)?; - let queued_tickets = dependency_queue_plan(&requested_ticket, &states, &relations)?; - if let Some(json) = self - .event_attributes - .get("queue_orchestrator_assignments") - { - let assignments = serde_json::from_str::>(json).map_err( - |error| { - TicketError::Conflict(format!( - "invalid Queue assignment fence: {error}" - )) - }, - )?; - let planned = assignments.keys().cloned().collect::>(); - let actual = queued_tickets.iter().cloned().collect::>(); - if planned != actual || assignments.values().any(|value| value.trim().is_empty()) { - return Err(TicketError::Conflict( - "Queue dependency plan changed after assignment validation".to_string(), - )); + self.require_active_source_assignment(conn, &requested_ticket)?; + let previous = self.load_ticket(conn, &requested_ticket)?; + let mut attributes = BTreeMap::from([ + ("queue_root_ticket".to_owned(), requested_ticket.clone()), + ("queued_by".to_owned(), queued_by.to_owned()), + ]); + if let Some(json) = self.event_attributes.get("queue_orchestrator_assignments") { + let assignments = serde_json::from_str::>(json) + .map_err(|error| TicketError::Conflict(format!("invalid Queue assignment fence: {error}")))?; + if assignments.len() != 1 || assignments.get(&requested_ticket).is_none_or(|value| value.trim().is_empty()) { + return Err(TicketError::Conflict("Queue assignment must name only the explicit Ticket".to_owned())); } + attributes.insert("orchestrator_assignment_id".to_owned(), assignments[&requested_ticket].clone()); } - - let mut resolved_targets = Vec::with_capacity(queued_tickets.len()); - for ticket_id in &queued_tickets { - let ticket = self.load_ticket(conn, ticket_id)?; - let targets = resolve_ready_targets( - self.target_authority.as_ref(), - &self.workspace_id, - &ticket.meta.targets, - )?; - resolved_targets.push((ticket_id.clone(), targets)); - } - let at = now_utc(); - for (ticket_id, targets) in resolved_targets { - let updated = conn.execute( - "UPDATE typed_tickets SET workflow_state = 'queued', workflow_state_explicit = 1, queued_by = ?3, queued_at = ?4, updated_at = ?4 WHERE workspace_id = ?1 AND ticket_id = ?2 AND workflow_state = 'ready'", - params![self.workspace_id, ticket_id, queued_by, at], - ).map_err(sqlite_err)?; - if updated != 1 { - return Err(TicketError::Conflict(format!( - "Ticket {ticket_id} changed while the dependency queue plan was being applied" - ))); - } - self.replace_ticket_targets( - conn, - &ticket_id, - &canonical_ticket_targets(&targets), - )?; - let mut attributes = BTreeMap::from([ - ("queued_by".to_owned(), queued_by.to_owned()), - ("queued_at".to_owned(), at.clone()), - ("targets".to_owned(), encoded_resolved_targets(&targets)?), - ("queue_root_ticket".to_owned(), requested_ticket.clone()), - ]); - if let Some(assignment_id) = self - .event_attributes - .get("queue_orchestrator_assignments") - .and_then(|json| serde_json::from_str::>(json).ok()) - .and_then(|assignments| assignments.get(&ticket_id).cloned()) - { - attributes.insert("orchestrator_assignment_id".to_owned(), assignment_id); - } - self.insert_event(conn, &ticket_id, &TicketEvent { - kind: TicketEventKind::StateChanged, - author: Some(queued_by.to_string()), - at: Some(at.clone()), - status: None, - from: Some("ready".to_string()), - to: Some("queued".to_string()), - reason: Some("queued".to_string()), - state_field: Some("state".to_string()), - heading: Some(TicketEventKind::StateChanged.heading()), - body: MarkdownText::new(format!("Queued for Orchestrator by {queued_by}.")), - references: Vec::new(), - attributes, - })?; - } - + attributes.insert("queued_at".to_owned(), at.clone()); + conn.execute( + "UPDATE typed_tickets SET workflow_state = 'queued', status = 'open', workflow_state_explicit = 1, queued_by = ?3, queued_at = ?4, updated_at = ?4 WHERE workspace_id = ?1 AND ticket_id = ?2", + params![self.workspace_id, requested_ticket, queued_by, at], + ).map_err(sqlite_err)?; + self.insert_event(conn, &requested_ticket, &TicketEvent { + kind: TicketEventKind::StateChanged, + author: Some(queued_by.to_owned()), + at: Some(at), + status: Some("open".to_owned()), + from: Some(previous.meta.workflow_state.to_string()), + to: Some("queued".to_owned()), + reason: Some("queued".to_owned()), + state_field: Some("state".to_owned()), + heading: Some(TicketEventKind::StateChanged.heading()), + body: MarkdownText::new(format!("Queued for Orchestrator by {queued_by}.")), + references: Vec::new(), + attributes, + })?; Ok(TicketQueueOutcome { + queued_tickets: vec![requested_ticket.clone()], requested_ticket, - queued_tickets, }) }) } fn close(&self, id: TicketIdOrSlug, resolution: MarkdownText) -> Result<()> { + if resolution.as_str().trim().is_empty() { + return Err(TicketError::Conflict( + "Ticket close requires a non-empty resolution".to_owned(), + )); + } self.with_write(|conn| { let ticket_id = self.resolve_ticket_id(conn, id)?; - let at = now_utc(); - conn.execute("UPDATE typed_tickets SET status = 'closed', workflow_state = 'closed', workflow_state_explicit = 1, updated_at = ?3, resolution = ?4 WHERE workspace_id = ?1 AND ticket_id = ?2", params![self.workspace_id, ticket_id, at, resolution.as_str()]).map_err(sqlite_err)?; - self.insert_event(conn, &ticket_id, &TicketEvent { kind: TicketEventKind::Close, author: Some(default_author()), at: Some(at), status: Some("closed".to_string()), from: None, to: Some("closed".to_string()), reason: None, state_field: Some("state".to_string()), heading: Some(TicketEventKind::Close.heading()), body: resolution, references: Vec::new(), attributes: BTreeMap::new() }) + let previous = self.load_ticket(conn, &ticket_id)?; + let request = TicketStateUpdate { + expected_item_revision: ticket_item_revision(&previous), + expected_state: previous.meta.workflow_state, + state: TicketWorkflowState::Closed, + operation_key: self.next_legacy_state_operation_key(conn, &ticket_id)?, + reason: "closed".to_owned(), + references: Vec::new(), + author: Some(default_author()), + }; + self.apply_state_update(conn, previous, request, resolution) + .map(|_| ()) }) } @@ -4046,151 +4008,6 @@ fn transitive_dependency_blockers( Ok(blockers.into_values().collect()) } -fn dependency_queue_plan( - requested_ticket: &str, - states: &HashMap, - relations: &[TicketRelation], -) -> Result> { - let requested_state = states - .get(requested_ticket) - .copied() - .ok_or_else(|| TicketError::NotFound(requested_ticket.to_owned()))?; - if requested_state != TicketWorkflowState::Ready { - return Err(TicketError::StaleWorkflowState { - expected: TicketWorkflowState::Ready.as_str().to_owned(), - actual: requested_state.as_str().to_owned(), - }); - } - - let mut prerequisites = BTreeMap::>::new(); - for relation in relations { - match relation.kind { - TicketRelationKind::DependsOn => { - prerequisites - .entry(relation.ticket_id.clone()) - .or_default() - .insert(relation.target.clone()); - } - TicketRelationKind::Blocks => { - prerequisites - .entry(relation.target.clone()) - .or_default() - .insert(relation.ticket_id.clone()); - } - TicketRelationKind::Related - | TicketRelationKind::Supersedes - | TicketRelationKind::DuplicateOf => {} - } - } - - fn visit( - ticket: &str, - states: &HashMap, - prerequisites: &BTreeMap>, - marks: &mut BTreeMap, - stack: &mut Vec, - ordered: &mut Vec, - ) -> Result<()> { - let state = states - .get(ticket) - .copied() - .ok_or_else(|| TicketError::NotFound(ticket.to_owned()))?; - if ticket_state_resolved(state) { - return Ok(()); - } - match marks.get(ticket).copied() { - Some(2) => return Ok(()), - Some(1) => { - let start = stack.iter().position(|item| item == ticket).unwrap_or(0); - let mut cycle = stack[start..].to_vec(); - cycle.push(ticket.to_owned()); - return Err(TicketError::Conflict(format!( - "ticket dependency cycle detected: {}", - cycle.join(" -> ") - ))); - } - _ => {} - } - - marks.insert(ticket.to_owned(), 1); - stack.push(ticket.to_owned()); - if let Some(dependencies) = prerequisites.get(ticket) { - for dependency in dependencies { - visit(dependency, states, prerequisites, marks, stack, ordered)?; - } - } - stack.pop(); - marks.insert(ticket.to_owned(), 2); - ordered.push(ticket.to_owned()); - Ok(()) - } - - let mut ordered = Vec::new(); - visit( - requested_ticket, - states, - &prerequisites, - &mut BTreeMap::new(), - &mut Vec::new(), - &mut ordered, - )?; - - fn collect_ready( - ticket: &str, - requested_ticket: &str, - states: &HashMap, - prerequisites: &BTreeMap>, - collected: &mut BTreeSet, - ready: &mut Vec, - ) -> Result<()> { - if !collected.insert(ticket.to_owned()) { - return Ok(()); - } - let state = states - .get(ticket) - .copied() - .ok_or_else(|| TicketError::NotFound(ticket.to_owned()))?; - if ticket != requested_ticket && state == TicketWorkflowState::Planning { - return Err(TicketError::BlockingRelations(format!( - "dependency {ticket} is still planning" - ))); - } - if matches!( - state, - TicketWorkflowState::Done | TicketWorkflowState::Closed - ) { - return Ok(()); - } - if let Some(dependencies) = prerequisites.get(ticket) { - for dependency in dependencies { - collect_ready( - dependency, - requested_ticket, - states, - prerequisites, - collected, - ready, - )?; - } - } - if state == TicketWorkflowState::Ready { - ready.push(ticket.to_owned()); - } - Ok(()) - } - - let mut ready = Vec::new(); - collect_ready( - requested_ticket, - requested_ticket, - states, - &prerequisites, - &mut BTreeSet::new(), - &mut ready, - )?; - Ok(ready) -} - fn relation_view_from_records( meta: &TicketMeta, records: &[TicketRelation], @@ -4273,13 +4090,6 @@ fn relation_view_from_records( view } -fn relation_blocker_allows_queue(blocker: &TicketRelationBlocker) -> bool { - matches!( - blocker.blocking_state, - TicketWorkflowState::Queued | TicketWorkflowState::InProgress - ) -} - fn format_relation_blockers(blockers: &[TicketRelationBlocker]) -> String { blockers .iter() @@ -4492,6 +4302,10 @@ fn validate_state_change(change: &TicketStateChange) -> Result<()> { if let Some(author) = change.author.as_deref() { validate_required_event_value("author", author)?; } + for reference in &change.references { + validate_required_event_value("reference_kind", &reference.kind)?; + validate_required_event_value("reference_target", &reference.target)?; + } if change.body.as_str().len() > MAX_INTAKE_SUMMARY_BODY_BYTES { return Err(TicketError::Conflict(format!( "state_changed body exceeds {MAX_INTAKE_SUMMARY_BODY_BYTES} bytes" @@ -4569,7 +4383,7 @@ mod tests { } Ok(ResolvedTicketTarget { repository_key: repository_key.to_owned(), - ref_selector: ref_selector.unwrap_or("develop").to_owned(), + ref_selector: Some(ref_selector.unwrap_or("develop").to_owned()), access: TicketTargetAccess::ReadOnly, }) } @@ -4837,24 +4651,6 @@ mod tests { } } - #[test] - fn dependency_queue_plan_orders_transitive_ready_dependencies() { - let states = HashMap::from([ - ("root".to_owned(), TicketWorkflowState::Ready), - ("middle".to_owned(), TicketWorkflowState::Ready), - ("leaf".to_owned(), TicketWorkflowState::Ready), - ]); - let relations = [ - dependency_relation("root", "middle"), - dependency_relation("middle", "leaf"), - ]; - - assert_eq!( - dependency_queue_plan("root", &states, &relations).unwrap(), - vec!["leaf", "middle", "root"] - ); - } - #[test] fn transitive_blockers_project_internal_dependency_cycle_as_blocking() { let states = HashMap::from([ @@ -4876,68 +4672,17 @@ mod tests { let mut summary = summary; summary.id = "root".to_string(); assert!( - !project_ticket_workspace_item(&summary, &blockers, None) + project_ticket_workspace_item(&summary, &blockers, None) .queue_guard .can_queue_for_orchestrator ); - assert!(matches!( - dependency_queue_plan("root", &states, &relations), - Err(TicketError::Conflict(_)) - )); - } - - #[test] - fn dependency_queue_plan_stops_at_resolved_dependency() { - let states = HashMap::from([ - ("root".to_owned(), TicketWorkflowState::Ready), - ("done".to_owned(), TicketWorkflowState::Done), - ("planning".to_owned(), TicketWorkflowState::Planning), - ]); - let relations = [ - dependency_relation("root", "done"), - dependency_relation("done", "planning"), - dependency_relation("done", "root"), - ]; - - assert_eq!( - dependency_queue_plan("root", &states, &relations).unwrap(), - vec!["root"] + assert!( + ticket_queue_guard(&summary, &blockers, None) + .blocked_reason + .is_some() ); } - #[test] - fn dependency_queue_plan_rejects_transitive_planning_dependency() { - let states = HashMap::from([ - ("root".to_owned(), TicketWorkflowState::Ready), - ("middle".to_owned(), TicketWorkflowState::Queued), - ("leaf".to_owned(), TicketWorkflowState::Planning), - ]); - let relations = [ - dependency_relation("root", "middle"), - dependency_relation("middle", "leaf"), - ]; - - let error = dependency_queue_plan("root", &states, &relations).unwrap_err(); - assert!(matches!(error, TicketError::BlockingRelations(_))); - assert!(error.to_string().contains("leaf")); - } - - #[test] - fn dependency_queue_plan_reports_cycle_path() { - let states = HashMap::from([ - ("root".to_owned(), TicketWorkflowState::Ready), - ("middle".to_owned(), TicketWorkflowState::Ready), - ]); - let relations = [ - dependency_relation("root", "middle"), - dependency_relation("middle", "root"), - ]; - - let error = dependency_queue_plan("root", &states, &relations).unwrap_err(); - assert!(matches!(error, TicketError::Conflict(_))); - assert!(error.to_string().contains("root -> middle -> root")); - } - #[test] fn workspace_projection_queues_ready_ticket_for_orchestrator() { let summary = summary_with_state(TicketWorkflowState::Ready); @@ -4957,7 +4702,7 @@ mod tests { } #[test] - fn workspace_projection_blocks_ready_ticket_with_planning_dependency() { + fn workspace_projection_queues_ready_ticket_with_planning_dependency() { let summary = summary_with_state(TicketWorkflowState::Ready); let blockers = [blocker_with_state(TicketWorkflowState::Planning)]; let projection = project_ticket_workspace_item(&summary, &blockers, None); @@ -4965,11 +4710,11 @@ mod tests { assert_eq!(projection.kind, TicketWorkspaceRowKind::Ticket); assert_eq!( projection.next_action, - Some(TicketWorkspaceNextAction::WaitForOrchestrator) + Some(TicketWorkspaceNextAction::QueueForOrchestrator) ); - assert!(!projection.queue_guard.can_queue_for_orchestrator); + assert!(projection.queue_guard.can_queue_for_orchestrator); assert!(projection.blocked_reason.is_some()); - assert!(projection.disabled_reason.is_some()); + assert!(projection.disabled_reason.is_none()); } #[test] @@ -4989,12 +4734,12 @@ mod tests { .key_hint .as_deref() .unwrap_or_default() - .contains("orchestration context") + .contains("Queue only") ); } #[test] - fn workspace_projection_overlay_suppresses_duplicate_queue() { + fn workspace_projection_overlay_retains_context_without_queue_gate() { let summary = summary_with_state(TicketWorkflowState::Ready); let overlay = TicketWorkspaceStateOverlay { source: "orchestration".to_string(), @@ -5008,25 +4753,25 @@ mod tests { Some(TicketWorkspaceNextAction::WaitForOrchestrator) ); assert_eq!(projection.visible_state, "ready→prog"); - assert!(!projection.queue_guard.can_queue_for_orchestrator); + assert!(projection.queue_guard.can_queue_for_orchestrator); assert!(projection.visible_overlay.is_some()); } #[test] - fn generic_state_change_rejects_manual_start_bypass() { + fn generic_state_change_allows_direct_start_without_phase_path() { assert!( validate_generic_state_change( TicketWorkflowState::Ready, TicketWorkflowState::InProgress ) - .is_err() + .is_ok() ); assert!( validate_generic_state_change( TicketWorkflowState::Planning, TicketWorkflowState::InProgress ) - .is_err() + .is_ok() ); } @@ -5070,7 +4815,7 @@ mod tests { TicketWorkflowState::Queued, TicketWorkflowState::Planning )); - assert!(!TicketWorkflowState::is_role_transition( + assert!(TicketWorkflowState::is_role_transition( TicketWorkflowState::Planning, TicketWorkflowState::Queued )); @@ -5128,7 +4873,7 @@ mod tests { } #[test] - fn sqlite_dependency_check_blocks_transitive_planning_dependency() { + fn sqlite_dependency_check_reports_transitive_planning_dependency() { let temp = TempDir::new().unwrap(); let backend = SqliteTicketBackend::open(temp.path().join("tickets.db"), "workspace-test") .unwrap() @@ -5162,8 +4907,8 @@ mod tests { let check = backend .dependency_check(TicketIdOrSlug::Id(root.id)) .unwrap(); - assert!(!check.queue_guard.can_queue_for_orchestrator); - assert!(check.queue_tickets.is_empty()); + assert!(check.queue_guard.can_queue_for_orchestrator); + assert_eq!(check.queue_tickets, vec![check.ticket.id.clone()]); assert!(check.blockers.iter().any(|blocker| { blocker.blocking_ticket == leaf.id && blocker.blocking_state == TicketWorkflowState::Planning @@ -5186,42 +4931,28 @@ mod tests { && blocker.blocking_state == TicketWorkflowState::Planning })); assert!( - !project_ticket_workspace_item(&item.summary, &item.relation_blockers, None) + project_ticket_workspace_item(&item.summary, &item.relation_blockers, None) .queue_guard .can_queue_for_orchestrator ); } #[test] - fn dependency_checks_fail_closed_without_target_authority() { - let sqlite_temp = TempDir::new().unwrap(); - let sqlite = - SqliteTicketBackend::open(sqlite_temp.path().join("tickets.db"), "workspace-test") - .unwrap(); - let mut sqlite_input = NewTicket::new("SQLite ready Ticket"); - sqlite_input.workflow_state = Some(TicketWorkflowState::Ready); - sqlite_input.targets = vec![ticket_target( - "main", - Some("develop"), - TicketTargetAccess::ReadWrite, - )]; - let sqlite_ticket = sqlite.create(sqlite_input).unwrap(); - let sqlite_check = sqlite - .dependency_check(TicketIdOrSlug::Id(sqlite_ticket.id.clone())) - .unwrap(); - assert!(!sqlite_check.queue_guard.can_queue_for_orchestrator); - assert!( - sqlite_check - .queue_guard - .blocked_reason - .as_deref() - .unwrap_or_default() - .contains("target authority is unavailable") + fn queue_does_not_require_target_authority() { + let tmp = TempDir::new().unwrap(); + let backend = + SqliteTicketBackend::open(tmp.path().join("tickets.db"), "workspace-test").unwrap(); + let ticket = backend.create(NewTicket::new("No Git or targets")).unwrap(); + let check = backend.dependency_check(ticket.id.clone().into()).unwrap(); + assert!(check.queue_guard.can_queue_for_orchestrator); + assert_eq!(check.queue_tickets, vec![ticket.id.clone()]); + assert_eq!( + backend + .queue_ready(ticket.id.clone().into(), "actor") + .unwrap() + .queued_tickets, + vec![ticket.id] ); - assert!(matches!( - sqlite.queue_ready(TicketIdOrSlug::Id(sqlite_ticket.id), "test"), - Err(TicketError::TargetAuthorityUnavailable) - )); } #[test] @@ -5278,7 +5009,7 @@ mod tests { } #[test] - fn sqlite_queue_cycle_diagnostic_leaves_all_tickets_ready() { + fn sqlite_queue_cycle_diagnostic_queues_only_explicit_ticket() { let temp = TempDir::new().unwrap(); let backend = SqliteTicketBackend::open(temp.path().join("tickets.db"), "workspace-test") .unwrap() @@ -5316,7 +5047,7 @@ mod tests { let check = backend .dependency_check(TicketIdOrSlug::Id(first.id.clone())) .unwrap(); - assert!(!check.queue_guard.can_queue_for_orchestrator); + assert!(check.queue_guard.can_queue_for_orchestrator); assert!( check .queue_guard @@ -5338,16 +5069,19 @@ mod tests { .find(|item| item.summary.id == check.ticket.id) .unwrap(); assert!( - !project_ticket_workspace_item(&item.summary, &item.relation_blockers, None) + project_ticket_workspace_item(&item.summary, &item.relation_blockers, None) .queue_guard .can_queue_for_orchestrator ); - let error = backend - .queue_ready(TicketIdOrSlug::Id(first.id.clone()), "orchestrator") - .unwrap_err(); - assert!(matches!(error, TicketError::Conflict(_))); - assert!(error.to_string().contains(" -> ")); - for ticket_id in [first.id, second.id, third.id] { + let outcome = backend + .queue_ready(first.id.clone().into(), "orchestrator") + .unwrap(); + assert_eq!(outcome.queued_tickets, vec![first.id.clone()]); + assert_eq!( + backend.show(first.id.into()).unwrap().meta.workflow_state, + TicketWorkflowState::Queued + ); + for ticket_id in [second.id, third.id] { assert_eq!( backend .show(TicketIdOrSlug::Id(ticket_id)) @@ -5360,7 +5094,7 @@ mod tests { } #[test] - fn sqlite_queue_target_failure_rolls_back_entire_ready_closure() { + fn sqlite_queue_ignores_dependency_targets() { let temp = TempDir::new().unwrap(); let backend = SqliteTicketBackend::open(temp.path().join("tickets.db"), "workspace-test") .unwrap() @@ -5388,20 +5122,16 @@ mod tests { let check = backend .dependency_check(TicketIdOrSlug::Id(root.id.clone())) .unwrap(); - assert!(!check.queue_guard.can_queue_for_orchestrator); - assert!( - check - .queue_guard - .blocked_reason - .as_deref() - .unwrap_or_default() - .contains("unknown") + assert!(check.queue_guard.can_queue_for_orchestrator); + let outcome = backend + .queue_ready(root.id.clone().into(), "orchestrator") + .unwrap(); + assert_eq!(outcome.queued_tickets, vec![root.id.clone()]); + assert_eq!( + backend.show(root.id.into()).unwrap().meta.workflow_state, + TicketWorkflowState::Queued ); - let error = backend - .queue_ready(TicketIdOrSlug::Id(root.id.clone()), "orchestrator") - .unwrap_err(); - assert!(matches!(error, TicketError::UnknownTargetRepository(_))); - for ticket_id in [dependency.id, root.id] { + for ticket_id in [dependency.id] { assert_eq!( backend .show(TicketIdOrSlug::Id(ticket_id)) @@ -5414,7 +5144,7 @@ mod tests { } #[test] - fn sqlite_queue_atomically_queues_ready_dependency_closure() { + fn sqlite_queue_queues_only_explicit_ticket_not_ready_dependency() { let tmp = TempDir::new().unwrap(); let backend = SqliteTicketBackend::open(tmp.path().join("workspace.db"), "workspace-test") .unwrap() @@ -5475,10 +5205,7 @@ mod tests { ) .unwrap(); assert_eq!(outcome.requested_ticket, implementation.id); - assert_eq!( - outcome.queued_tickets, - vec![dependency.id.clone(), implementation.id.clone()] - ); + assert_eq!(outcome.queued_tickets, vec![implementation.id.clone()]); let queued = backend .show(TicketIdOrSlug::Id(implementation.id.clone())) .unwrap(); @@ -5488,7 +5215,7 @@ mod tests { assert_eq!(queued.meta.workflow_state, TicketWorkflowState::Queued); assert_eq!( queued_dependency.meta.workflow_state, - TicketWorkflowState::Queued + TicketWorkflowState::Ready ); assert_eq!(queued.meta.queued_by.as_deref(), Some("orchestrator")); assert_eq!(queued.relations.blockers.len(), 1); @@ -5871,6 +5598,1102 @@ mod tests { ); } + fn assignment_source(role: &str) -> BTreeMap { + BTreeMap::from([ + ("source_runtime_id".to_owned(), "runtime-a".to_owned()), + ("source_worker_id".to_owned(), "worker-a".to_owned()), + ("source_actor_role".to_owned(), role.to_owned()), + ("source_assignment_id".to_owned(), "assignment-a".to_owned()), + ]) + } + + fn active_assignment_fixture(backend: &SqliteTicketBackend, ticket_id: &str) { + let conn = backend.open_connection().unwrap(); + conn.execute_batch( + "CREATE TABLE test_worker_assignments ( + workspace_id TEXT NOT NULL, ticket_id TEXT NOT NULL, runtime_id TEXT NOT NULL, + worker_id TEXT NOT NULL, assignment_id TEXT NOT NULL, status TEXT NOT NULL + ); + CREATE VIEW ticket_active_worker_assignments AS + SELECT workspace_id, ticket_id, runtime_id, worker_id, assignment_id + FROM test_worker_assignments WHERE status = 'active';", + ) + .unwrap(); + conn.execute("INSERT INTO test_worker_assignments VALUES (?1, ?2, 'runtime-a', 'worker-a', 'assignment-a', 'active')", params![backend.workspace_id(), ticket_id]).unwrap(); + } + + fn release_assignment_fixture(backend: &SqliteTicketBackend) { + backend + .open_connection() + .unwrap() + .execute("UPDATE test_worker_assignments SET status = 'released'", []) + .unwrap(); + } + + #[test] + fn item_revision_uses_canonical_max_item_edit_index_not_edit_count_or_content_hash() { + let tmp = TempDir::new().unwrap(); + let backend = backend(&tmp); + let (reference, ticket) = backend + .create_with_snapshot(NewTicket::new("Revision format")) + .unwrap(); + assert_eq!(ticket_item_revision(&ticket), format!("{}:0", reference.id)); + backend + .add_event( + reference.id.clone().into(), + NewTicketEvent::new(TicketEventKind::Comment, "before edit"), + ) + .unwrap(); + let first = backend + .edit_item( + reference.id.clone().into(), + TicketItemEdit { + title: Some("Edited".to_owned()), + ..Default::default() + }, + ) + .unwrap(); + assert_eq!(ticket_item_revision(&first), format!("{}:2", reference.id)); + backend + .add_event( + reference.id.clone().into(), + NewTicketEvent::new(TicketEventKind::Comment, "between edits"), + ) + .unwrap(); + let second = backend + .edit_item( + reference.id.clone().into(), + TicketItemEdit { + body: Some(MarkdownText::new("Changed body")), + ..Default::default() + }, + ) + .unwrap(); + assert_eq!(ticket_item_revision(&second), format!("{}:4", reference.id)); + let mut reordered = second; + reordered.events.reverse(); + assert_eq!( + ticket_item_revision(&reordered), + format!("{}:4", reference.id) + ); + } + + #[test] + fn assignment_fence_accepts_exact_live_source_inside_state_and_close_transaction() { + let tmp = TempDir::new().unwrap(); + let backend = backend(&tmp); + let (reference, ticket) = backend + .create_with_snapshot(NewTicket::new("Active source")) + .unwrap(); + active_assignment_fixture(&backend, &reference.id); + let source = backend.clone().with_event_attributes(assignment_source("coder")) + .with_mutation_hook(Arc::new(|conn, _| { + assert!(!conn.is_autocommit()); + let count: i64 = conn.query_row("SELECT COUNT(*) FROM ticket_active_worker_assignments WHERE runtime_id = 'runtime-a' AND worker_id = 'worker-a' AND assignment_id = 'assignment-a'", [], |row| row.get(0)).map_err(sqlite_err)?; + assert_eq!(count, 1); + Ok(()) + })); + let completed = source + .complete(&reference.id, completion_request(&ticket, "complete")) + .unwrap(); + source + .set_workflow_state( + reference.id.clone().into(), + TicketStateChange::new("done", "planning", "reopen", "reopen"), + ) + .unwrap(); + source + .close( + reference.id.clone().into(), + MarkdownText::new("Closed by active coder"), + ) + .unwrap(); + let closed = source.show(reference.id.clone().into()).unwrap(); + assert_eq!(closed.meta.workflow_state, TicketWorkflowState::Closed); + assert_eq!( + completed + .events + .last() + .unwrap() + .attributes + .get("source_assignment_id") + .map(String::as_str), + Some("assignment-a") + ); + } + + #[test] + fn assignment_fence_rejects_every_mismatched_identity_without_mutation() { + let tmp = TempDir::new().unwrap(); + let backend = backend(&tmp); + let (reference, ticket) = backend + .create_with_snapshot(NewTicket::new("Exact identity")) + .unwrap(); + active_assignment_fixture(&backend, &reference.id); + for key in [ + "source_runtime_id", + "source_worker_id", + "source_assignment_id", + ] { + let mut attrs = assignment_source("coder"); + attrs.insert(key.to_owned(), "other".to_owned()); + let source = backend.clone().with_event_attributes(attrs); + assert!( + source + .complete(&reference.id, completion_request(&ticket, "complete")) + .is_err() + ); + assert!( + source + .set_workflow_state( + reference.id.clone().into(), + TicketStateChange::new("planning", "done", "finished", "finished") + ) + .is_err() + ); + assert!( + source + .close(reference.id.clone().into(), MarkdownText::new("finished")) + .is_err() + ); + assert_eq!(backend.show(reference.id.clone().into()).unwrap(), ticket); + } + let (other, other_ticket) = backend + .create_with_snapshot(NewTicket::new("Different Ticket")) + .unwrap(); + let source = backend + .clone() + .with_event_attributes(assignment_source("coder")); + assert!( + source + .complete(&other.id, completion_request(&other_ticket, "complete")) + .is_err() + ); + assert_eq!(backend.show(other.id.into()).unwrap(), other_ticket); + let other_workspace = + SqliteTicketBackend::open(backend.db_path(), "other-workspace").unwrap(); + let (other, other_ticket) = other_workspace + .create_with_snapshot(NewTicket::new("Different Workspace")) + .unwrap(); + let source = other_workspace + .clone() + .with_event_attributes(assignment_source("coder")); + assert!( + source + .complete(&other.id, completion_request(&other_ticket, "complete")) + .is_err() + ); + assert_eq!(other_workspace.show(other.id.into()).unwrap(), other_ticket); + } + + #[test] + fn assignment_fence_rejects_missing_identity_or_active_view_and_non_coder_assignment_sources() { + let tmp = TempDir::new().unwrap(); + let backend = backend(&tmp); + let (reference, ticket) = backend + .create_with_snapshot(NewTicket::new("Missing identity")) + .unwrap(); + for key in [ + "source_runtime_id", + "source_worker_id", + "source_assignment_id", + ] { + let mut attrs = assignment_source("coder"); + attrs.remove(key); + let source = backend.clone().with_event_attributes(attrs); + assert!( + source + .complete(&reference.id, completion_request(&ticket, "complete")) + .is_err() + ); + } + let source = backend + .clone() + .with_event_attributes(assignment_source("coder")); + assert!( + source + .complete(&reference.id, completion_request(&ticket, "complete")) + .is_err() + ); + active_assignment_fixture(&backend, &reference.id); + release_assignment_fixture(&backend); + let source = backend + .clone() + .with_event_attributes(assignment_source("orchestrator")); + assert!( + source + .complete(&reference.id, completion_request(&ticket, "complete")) + .is_err() + ); + assert_eq!(backend.show(reference.id.into()).unwrap(), ticket); + } + + #[test] + fn released_assignment_cannot_mutate_reopened_ticket_but_can_exactly_replay() { + let tmp = TempDir::new().unwrap(); + let backend = backend(&tmp); + let (reference, ticket) = backend + .create_with_snapshot(NewTicket::new("Released source")) + .unwrap(); + active_assignment_fixture(&backend, &reference.id); + let source = backend + .clone() + .with_event_attributes(assignment_source("coder")); + let request = completion_request(&ticket, "complete"); + let completed = source.complete(&reference.id, request.clone()).unwrap(); + release_assignment_fixture(&backend); + let mut orchestrator_attrs = assignment_source("orchestrator"); + orchestrator_attrs.remove("source_assignment_id"); + let orchestrator = backend.clone().with_event_attributes(orchestrator_attrs); + let reopened = orchestrator + .update_state( + &reference.id, + TicketStateUpdate { + state: TicketWorkflowState::Planning, + author: Some("registered-orchestrator".to_owned()), + ..completion_request(&completed, "reopen").into() + }, + ) + .unwrap(); + assert_eq!( + source.complete(&reference.id, request.clone()).unwrap(), + reopened + ); + assert_eq!( + &reopened.events[..completed.events.len()], + completed.events.as_slice() + ); + assert!( + source + .complete(&reference.id, completion_request(&reopened, "new-complete")) + .is_err() + ); + assert!( + source + .set_workflow_state( + reference.id.clone().into(), + TicketStateChange::new("planning", "done", "finished", "finished") + ) + .is_err() + ); + assert!( + source + .close(reference.id.clone().into(), MarkdownText::new("finished")) + .is_err() + ); + let close = TicketStateUpdate { + state: TicketWorkflowState::Closed, + ..completion_request(&reopened, "close").into() + }; + assert!(source.update_state(&reference.id, close).is_err()); + assert_eq!(backend.show(reference.id.into()).unwrap(), reopened); + let conn = backend.open_connection().unwrap(); + let status: String = conn + .query_row("SELECT status FROM test_worker_assignments", [], |row| { + row.get(0) + }) + .unwrap(); + assert_eq!(status, "released"); + } + + #[test] + fn assignment_source_actor_is_bound_to_replay_fingerprint() { + let tmp = TempDir::new().unwrap(); + let backend = backend(&tmp); + let (reference, ticket) = backend + .create_with_snapshot(NewTicket::new("Actor fingerprint")) + .unwrap(); + active_assignment_fixture(&backend, &reference.id); + let source = backend + .clone() + .with_event_attributes(assignment_source("coder")); + let request = completion_request(&ticket, "complete"); + let completed = source.complete(&reference.id, request.clone()).unwrap(); + release_assignment_fixture(&backend); + for key in [ + "source_runtime_id", + "source_worker_id", + "source_assignment_id", + "source_actor_role", + ] { + let mut attrs = assignment_source("coder"); + attrs.insert(key.to_owned(), "different-source".to_owned()); + let other_source = backend.clone().with_event_attributes(attrs); + assert!(matches!( + other_source.complete(&reference.id, request.clone()), + Err(TicketError::OperationFingerprintMismatch { .. }) + )); + } + assert_eq!(backend.show(reference.id.into()).unwrap(), completed); + } + + #[test] + fn legacy_state_and_close_share_transactional_audit_and_operation_receipts() { + let tmp = TempDir::new().unwrap(); + let backend = backend(&tmp); + let (reference, ticket) = backend + .create_with_snapshot(NewTicket::new("Legacy boundary")) + .unwrap(); + let body = MarkdownText::new("## Custom implementation report\n\nPreserved body.\n"); + let references = vec![TicketReference { + kind: "report".to_owned(), + target: "test-results".to_owned(), + }]; + let mut change = TicketStateChange::new("planning", "done", "finished", body.clone()); + change.references = references.clone(); + change.author = Some("legacy-author".to_owned()); + backend + .set_workflow_state(reference.id.clone().into(), change) + .unwrap(); + let done = backend.show(reference.id.clone().into()).unwrap(); + let event = done.events.last().unwrap(); + assert_eq!(event.body, body); + assert_eq!(event.references, references); + assert_eq!(event.author.as_deref(), Some("legacy-author")); + assert_eq!(event.from.as_deref(), Some("planning")); + assert_eq!(event.reason.as_deref(), Some("finished")); + assert_eq!( + event.attributes.get("expected_item_revision"), + Some(&ticket_item_revision(&ticket)) + ); + assert_eq!( + event + .attributes + .get("fingerprint_version") + .map(String::as_str), + Some("state-update-v1") + ); + let request = TicketStateUpdate { + expected_item_revision: ticket_item_revision(&ticket), + expected_state: TicketWorkflowState::Planning, + state: TicketWorkflowState::Done, + operation_key: event.attributes["operation_key"].clone(), + reason: "finished".to_owned(), + references, + author: Some("legacy-author".to_owned()), + }; + let replay = backend + .with_write(|conn| { + let snapshot = backend.load_ticket(conn, &reference.id)?; + backend.apply_state_update(conn, snapshot, request.clone(), body.clone()) + }) + .unwrap(); + assert_eq!(replay, done); + let resolution = + MarkdownText::new(format!("## Resolution\n\n{}\n", "details ".repeat(200))); + backend + .close(reference.id.clone().into(), resolution.clone()) + .unwrap(); + let closed = backend.show(reference.id.clone().into()).unwrap(); + let close = closed.events.last().unwrap(); + assert_eq!(close.kind, TicketEventKind::Close); + assert_eq!(close.from.as_deref(), Some("done")); + assert_eq!(close.to.as_deref(), Some("closed")); + assert_eq!(close.reason.as_deref(), Some("closed")); + assert_eq!(close.body, resolution); + assert_eq!(closed.resolution, Some(resolution)); + assert_eq!( + close.attributes.get("expected_item_revision"), + Some(&ticket_item_revision(&done)) + ); + assert_ne!( + close.attributes["operation_key"], + event.attributes["operation_key"] + ); + let conn = backend.open_connection().unwrap(); + let receipt_attributes = backend + .load_event_attributes( + &conn, + &reference.id, + close.attributes["event_sequence"].parse().unwrap(), + ) + .unwrap(); + assert_eq!(receipt_attributes.len(), 4); + let replay = backend + .with_write(|conn| { + let current = backend.load_ticket(conn, &reference.id)?; + backend.apply_state_update(conn, current, request, body) + }) + .unwrap(); + assert_eq!(replay, closed); + assert_eq!(&replay.events[..done.events.len()], done.events.as_slice()); + } + + #[test] + fn legacy_state_and_close_hooks_observe_new_state_and_rollback_common_boundary() { + let tmp = TempDir::new().unwrap(); + let backend = backend(&tmp); + let (reference, ticket) = backend + .create_with_snapshot(NewTicket::new("Legacy rollback")) + .unwrap(); + let failing = backend.clone().with_mutation_hook(Arc::new(|conn, event| { + assert!(!conn.is_autocommit()); + let (current, audited): (String, String) = conn.query_row("SELECT t.workflow_state, e.to_state FROM typed_tickets t JOIN typed_ticket_events e ON e.workspace_id = t.workspace_id AND e.ticket_id = t.ticket_id WHERE t.workspace_id = ?1 AND t.ticket_id = ?2 AND e.event_index = ?3", params![event.workspace_id, event.ticket_id, event.event_index], |row| Ok((row.get(0)?, row.get(1)?))).map_err(sqlite_err)?; + assert_eq!(current, audited); + Err(TicketError::Conflict("reject mutation".to_owned())) + })); + assert!( + failing + .set_workflow_state( + reference.id.clone().into(), + TicketStateChange::new("planning", "done", "finished", "custom body") + ) + .is_err() + ); + assert_eq!(backend.show(reference.id.clone().into()).unwrap(), ticket); + assert!( + failing + .close(reference.id.clone().into(), MarkdownText::new("Resolution")) + .is_err() + ); + assert_eq!(backend.show(reference.id.clone().into()).unwrap(), ticket); + let count: i64 = backend + .open_connection() + .unwrap() + .query_row( + "SELECT COUNT(*) FROM typed_ticket_event_attributes WHERE key = 'operation_key'", + [], + |row| row.get(0), + ) + .unwrap(); + assert_eq!(count, 0); + } + + #[test] + fn authenticated_receipt_context_replays_after_live_role_and_assignment_clearance() { + let tmp = TempDir::new().unwrap(); + let backend = backend(&tmp); + let (reference, ticket) = backend + .create_with_snapshot(NewTicket::new("Role clearance replay")) + .unwrap(); + active_assignment_fixture(&backend, &reference.id); + let source = backend + .clone() + .with_event_attributes(assignment_source("coder")); + let request = completion_request(&ticket, "complete"); + let completed = source.complete(&reference.id, request.clone()).unwrap(); + release_assignment_fixture(&backend); + let mut current_context = assignment_source("reader"); + current_context.remove("source_assignment_id"); + // Parent authenticates unchanged runtime/worker/author and operation before + // restoring only receipt role/assignment metadata from the recorded audit. + let receipt = completed.events.last().unwrap(); + for key in ["source_actor_role", "source_assignment_id"] { + current_context.insert(key.to_owned(), receipt.attributes[key].clone()); + } + let replay_source = backend.clone().with_event_attributes(current_context); + assert_eq!( + replay_source.complete(&reference.id, request).unwrap(), + completed + ); + assert!( + replay_source + .complete( + &reference.id, + completion_request(&completed, "new-complete") + ) + .is_err() + ); + assert_eq!(backend.show(reference.id.into()).unwrap(), completed); + } + + #[test] + fn common_state_mutation_preserves_actual_historical_ticket_completion_events() { + let tmp = TempDir::new().unwrap(); + let backend = backend(&tmp); + let reference = backend + .create(NewTicket::new("Historical MR completion")) + .unwrap(); + let historical = backend.clone().with_event_attributes(BTreeMap::from([ + ( + "operation_key".to_owned(), + "historical-mr-completion".to_owned(), + ), + ("merge_request_id".to_owned(), "MR-existing".to_owned()), + ])); + let mut event = NewTicketEvent::new( + TicketEventKind::Other("ticket_completion".to_owned()), + "Historical Merge Request completion", + ); + event.references.push(TicketReference { + kind: "merge_request".to_owned(), + target: "MR-existing".to_owned(), + }); + historical + .add_event(reference.id.clone().into(), event) + .unwrap(); + let previous = backend.show(reference.id.clone().into()).unwrap(); + backend + .set_workflow_state( + reference.id.clone().into(), + TicketStateChange::new("planning", "done", "completed", "completed"), + ) + .unwrap(); + backend + .close(reference.id.clone().into(), MarkdownText::new("Resolved")) + .unwrap(); + let current = backend.show(reference.id.into()).unwrap(); + assert_eq!( + ¤t.events[..previous.events.len()], + previous.events.as_slice() + ); + assert_eq!( + current.events[1].kind, + TicketEventKind::Other("ticket_completion".to_owned()) + ); + } + + fn completion_request(ticket: &Ticket, key: &str) -> TicketCompletion { + TicketCompletion { + expected_item_revision: ticket_item_revision(ticket), + expected_state: ticket.meta.workflow_state, + operation_key: key.to_owned(), + reason: "Acceptance criteria met".to_owned(), + references: Vec::new(), + author: Some("tester".to_owned()), + } + } + + #[test] + fn completion_from_all_states_needs_no_git_targets_or_merge_request() { + let tmp = TempDir::new().unwrap(); + let backend = + SqliteTicketBackend::open(tmp.path().join("tickets.db"), "workspace-test").unwrap(); + for state in [ + TicketWorkflowState::Planning, + TicketWorkflowState::Ready, + TicketWorkflowState::Queued, + TicketWorkflowState::InProgress, + TicketWorkflowState::Done, + TicketWorkflowState::Closed, + ] { + let mut input = NewTicket::new(format!("Complete {state}")); + input.workflow_state = Some(state); + let (reference, ticket) = backend.create_with_snapshot(input).unwrap(); + let completed = backend + .complete(&reference.id, completion_request(&ticket, "complete")) + .unwrap(); + assert_eq!(completed.meta.workflow_state, TicketWorkflowState::Done); + assert_eq!(completed.meta.status, ExtensibleTicketStatus::Open); + assert_eq!(completed.meta.targets, ticket.meta.targets); + assert_eq!(completed.events.len(), ticket.events.len() + 1); + let event = completed.events.last().unwrap(); + assert_eq!(event.author.as_deref(), Some("tester")); + assert_eq!(event.from.as_deref(), Some(state.as_str())); + assert_eq!(event.to.as_deref(), Some("done")); + assert!(event.references.is_empty()); + assert_eq!(backend.show(reference.id.into()).unwrap(), completed); + } + } + + #[test] + fn completion_cas_rejects_stale_item_or_state_without_mutation() { + let tmp = TempDir::new().unwrap(); + let backend = backend(&tmp); + let (reference, ticket) = backend.create_with_snapshot(NewTicket::new("CAS")).unwrap(); + let request = completion_request(&ticket, "complete"); + let edited = backend + .edit_item( + reference.id.clone().into(), + TicketItemEdit { + title: Some("Edited".to_owned()), + ..Default::default() + }, + ) + .unwrap(); + assert!(matches!( + backend.complete(&reference.id, request), + Err(TicketError::StaleItemRevision { .. }) + )); + assert_eq!(backend.show(reference.id.clone().into()).unwrap(), edited); + let mut request = completion_request(&edited, "complete"); + request.expected_state = TicketWorkflowState::Ready; + assert!(matches!( + backend.complete(&reference.id, request), + Err(TicketError::StaleWorkflowState { .. }) + )); + assert_eq!(backend.show(reference.id.into()).unwrap(), edited); + } + + #[test] + fn completion_exact_replay_survives_later_edits_reopen_and_backend_restart() { + let tmp = TempDir::new().unwrap(); + let db = tmp.path().join("tickets.db"); + let backend = SqliteTicketBackend::open(&db, "workspace-test").unwrap(); + let (reference, ticket) = backend + .create_with_snapshot(NewTicket::new("Replay")) + .unwrap(); + let mut request = completion_request(&ticket, "complete"); + request.references = vec![TicketReference { + kind: "test_report".to_owned(), + target: "optional-evidence".to_owned(), + }]; + let completed = backend.complete("T-1", request.clone()).unwrap(); + assert_eq!( + completed.events.last().unwrap().references, + request.references + ); + let reopened = backend + .update_state( + &reference.id, + TicketStateUpdate { + state: TicketWorkflowState::Planning, + operation_key: "reopen".to_owned(), + ..completion_request(&completed, "unused").into() + }, + ) + .unwrap(); + let edited = backend + .edit_item( + reference.id.clone().into(), + TicketItemEdit { + body: Some(MarkdownText::new("New body")), + ..Default::default() + }, + ) + .unwrap(); + let backend = SqliteTicketBackend::open(&db, "workspace-test").unwrap(); + let replayed = backend.complete(&reference.id, request.clone()).unwrap(); + assert_eq!(replayed, edited); + assert_eq!(replayed.meta.workflow_state, TicketWorkflowState::Planning); + assert_eq!(replayed.document.body.as_str(), "New body"); + assert_eq!( + &replayed.events[..completed.events.len()], + completed.events.as_slice() + ); + let original_receipt = replayed + .events + .iter() + .find(|event| event.attributes.get("operation_key") == Some(&request.operation_key)) + .unwrap(); + assert_eq!(original_receipt, completed.events.last().unwrap()); + assert_eq!(replayed.events.len(), edited.events.len()); + assert_eq!(backend.show(reference.id.into()).unwrap(), edited); + assert_eq!(reopened.meta.workflow_state, TicketWorkflowState::Planning); + } + + #[test] + fn state_operation_receipts_store_constant_metadata_without_ticket_snapshots() { + let tmp = TempDir::new().unwrap(); + let backend = backend(&tmp); + let mut input = NewTicket::new("Linear receipt storage"); + input.body = MarkdownText::new("Large authoritative item body. ".repeat(2000)); + let (reference, mut current) = backend.create_with_snapshot(input).unwrap(); + let first_request = completion_request(¤t, "complete-0"); + let operations = 16; + for index in 0..operations { + let request = completion_request(¤t, &format!("complete-{index}")); + current = backend.complete(&reference.id, request).unwrap(); + } + let conn = backend.open_connection().unwrap(); + let (count, bytes): (i64, i64) = conn.query_row( + "SELECT COUNT(*), COALESCE(SUM(length(value)), 0) FROM typed_ticket_event_attributes WHERE workspace_id = ?1 AND ticket_id = ?2", + params![backend.workspace_id(), reference.id], |row| Ok((row.get(0)?, row.get(1)?)), + ).unwrap(); + assert_eq!(count, operations * 4); + assert!(bytes < operations * 256); + let original_receipt = current.events[1].clone(); + let replayed = backend.complete(&reference.id, first_request).unwrap(); + assert_eq!(replayed, current); + assert_eq!(replayed.events[1], original_receipt); + assert_eq!(replayed.events.len(), operations as usize + 1); + let count_after: i64 = conn.query_row( + "SELECT COUNT(*) FROM typed_ticket_event_attributes WHERE workspace_id = ?1 AND ticket_id = ?2", + params![backend.workspace_id(), reference.id], |row| row.get(0), + ).unwrap(); + assert_eq!(count_after, count); + } + + #[test] + fn completion_fingerprint_binds_every_request_field_and_operation_kind() { + let tmp = TempDir::new().unwrap(); + let backend = backend(&tmp); + let (reference, ticket) = backend + .create_with_snapshot(NewTicket::new("Fingerprint")) + .unwrap(); + let request = completion_request(&ticket, "complete"); + let completed = backend.complete(&reference.id, request.clone()).unwrap(); + let variants = [ + TicketCompletion { + reason: "Changed reason".to_owned(), + ..request.clone() + }, + TicketCompletion { + author: Some("other".to_owned()), + ..request.clone() + }, + TicketCompletion { + author: None, + ..request.clone() + }, + TicketCompletion { + expected_item_revision: "other-revision".to_owned(), + ..request.clone() + }, + TicketCompletion { + expected_state: TicketWorkflowState::Ready, + ..request.clone() + }, + TicketCompletion { + references: vec![TicketReference { + kind: "report".to_owned(), + target: "different".to_owned(), + }], + ..request.clone() + }, + ]; + for changed in variants { + assert!(matches!( + backend.complete(&reference.id, changed), + Err(TicketError::OperationFingerprintMismatch { .. }) + )); + } + let close = TicketStateUpdate { + state: TicketWorkflowState::Closed, + ..request.into() + }; + assert!(matches!( + backend.update_state(&reference.id, close), + Err(TicketError::OperationFingerprintMismatch { .. }) + )); + assert_eq!(backend.show(reference.id.into()).unwrap(), completed); + } + + #[test] + fn audited_close_reopen_preserves_resolution_history_and_resource_identity() { + let tmp = TempDir::new().unwrap(); + let backend = backend(&tmp); + let (reference, ticket) = backend + .create_with_snapshot(NewTicket::new("Close reopen")) + .unwrap(); + let close = TicketStateUpdate { + state: TicketWorkflowState::Closed, + ..completion_request(&ticket, "close").into() + }; + let closed = backend.update_state(&reference.id, close.clone()).unwrap(); + assert_eq!(closed.meta.status, ExtensibleTicketStatus::Closed); + assert_eq!(closed.resolution, Some(MarkdownText::new(&close.reason))); + assert_eq!(closed.events.last().unwrap().kind, TicketEventKind::Close); + assert_eq!(closed.events.last().unwrap().author, close.author); + let reopen = TicketStateUpdate { + state: TicketWorkflowState::Planning, + ..completion_request(&closed, "reopen").into() + }; + let reopened = backend.update_state(&reference.id, reopen.clone()).unwrap(); + assert_eq!(reopened.meta.status, ExtensibleTicketStatus::Open); + assert_eq!(reopened.meta.resource_key, closed.meta.resource_key); + assert_eq!(reopened.resolution, closed.resolution); + assert_eq!( + &reopened.events[..closed.events.len()], + closed.events.as_slice() + ); + let replayed = backend.update_state(&reference.id, close).unwrap(); + assert_eq!(replayed, reopened); + assert_eq!(replayed.meta.workflow_state, TicketWorkflowState::Planning); + assert_eq!( + &replayed.events[..closed.events.len()], + closed.events.as_slice() + ); + assert_eq!( + backend.update_state(&reference.id, reopen).unwrap(), + reopened + ); + assert_eq!(backend.show(reference.id.into()).unwrap(), reopened); + } + + #[test] + fn completion_mutation_hook_failure_rolls_back_state_event_and_replay() { + let tmp = TempDir::new().unwrap(); + let backend = backend(&tmp); + let (reference, ticket) = backend + .create_with_snapshot(NewTicket::new("Rollback")) + .unwrap(); + let request = completion_request(&ticket, "complete"); + let failing = backend.clone().with_mutation_hook(Arc::new(|_, _| { + Err(TicketError::Conflict("reject audit".to_owned())) + })); + assert!(failing.complete(&reference.id, request.clone()).is_err()); + assert_eq!(backend.show(reference.id.clone().into()).unwrap(), ticket); + let completed = backend.complete(&reference.id, request.clone()).unwrap(); + assert_eq!(backend.complete(&reference.id, request).unwrap(), completed); + } + + #[test] + fn completion_concurrent_exact_replay_appends_one_event() { + let tmp = TempDir::new().unwrap(); + let backend = backend(&tmp); + let (reference, ticket) = backend + .create_with_snapshot(NewTicket::new("Concurrent")) + .unwrap(); + let request = completion_request(&ticket, "complete"); + let barrier = Arc::new(std::sync::Barrier::new(2)); + let handles = (0..2) + .map(|_| { + let backend = backend.clone(); + let reference = reference.clone(); + let request = request.clone(); + let barrier = barrier.clone(); + std::thread::spawn(move || { + barrier.wait(); + backend.complete(&reference.id, request).unwrap() + }) + }) + .collect::>(); + let results = handles + .into_iter() + .map(|handle| handle.join().unwrap()) + .collect::>(); + assert_eq!(results[0], results[1]); + assert_eq!(results[0].events.len(), ticket.events.len() + 1); + assert_eq!(backend.show(reference.id.into()).unwrap(), results[0]); + } + + #[test] + fn item_revision_tracks_target_edits_and_aba_but_not_comments_or_state() { + let tmp = TempDir::new().unwrap(); + let backend = backend(&tmp); + let (reference, ticket) = backend + .create_with_snapshot(NewTicket::new("Revision")) + .unwrap(); + let revision = ticket_item_revision(&ticket); + backend + .add_event( + reference.id.clone().into(), + NewTicketEvent::new(TicketEventKind::Comment, "comment"), + ) + .unwrap(); + let commented = backend.show(reference.id.clone().into()).unwrap(); + assert_eq!(ticket_item_revision(&commented), revision); + let completed = backend + .complete(&reference.id, completion_request(&ticket, "complete")) + .unwrap(); + assert_eq!(ticket_item_revision(&completed), revision); + backend + .edit_item( + reference.id.clone().into(), + TicketItemEdit { + targets: Some(TicketTargetsEdit::Set { + targets: vec![write_target("main")], + }), + ..Default::default() + }, + ) + .unwrap(); + let restored = backend + .edit_item( + reference.id.into(), + TicketItemEdit { + targets: Some(TicketTargetsEdit::Clear), + ..Default::default() + }, + ) + .unwrap(); + assert_ne!(ticket_item_revision(&restored), revision); + } + + #[test] + fn completion_rejects_invalid_requests_without_recording_operation_key() { + let tmp = TempDir::new().unwrap(); + let backend = backend(&tmp); + let (reference, ticket) = backend + .create_with_snapshot(NewTicket::new("Validation")) + .unwrap(); + let request = completion_request(&ticket, "complete"); + for invalid in [ + TicketCompletion { + operation_key: " ".to_owned(), + ..request.clone() + }, + TicketCompletion { + expected_item_revision: "".to_owned(), + ..request.clone() + }, + TicketCompletion { + reason: "".to_owned(), + ..request.clone() + }, + TicketCompletion { + reason: " \t ".to_owned(), + ..request.clone() + }, + TicketCompletion { + references: vec![TicketReference { + kind: " \t ".to_owned(), + target: "report".to_owned(), + }], + ..request.clone() + }, + TicketCompletion { + references: vec![TicketReference { + kind: "report".to_owned(), + target: " \t ".to_owned(), + }], + ..request.clone() + }, + TicketCompletion { + reason: "x".repeat(MAX_STATE_CHANGE_REASON_BYTES + 1), + ..request.clone() + }, + TicketCompletion { + author: Some("".to_owned()), + ..request.clone() + }, + TicketCompletion { + references: vec![TicketReference { + kind: "".to_owned(), + target: "report".to_owned(), + }], + ..request.clone() + }, + TicketCompletion { + references: vec![TicketReference { + kind: "report".to_owned(), + target: "".to_owned(), + }], + ..request.clone() + }, + ] { + assert!(matches!( + backend.complete(&reference.id, invalid), + Err(TicketError::Conflict(_)) + )); + assert_eq!(backend.show(reference.id.clone().into()).unwrap(), ticket); + } + assert!(backend.complete(&reference.id, request).is_ok()); + } + + #[test] + fn legacy_state_and_close_reject_blank_reasons_resolution_and_references() { + let tmp = TempDir::new().unwrap(); + let backend = backend(&tmp); + let (reference, ticket) = backend + .create_with_snapshot(NewTicket::new("Legacy validation")) + .unwrap(); + assert!(matches!( + backend.set_workflow_state( + reference.id.clone().into(), + TicketStateChange::new("planning", "done", " \t ", "finished") + ), + Err(TicketError::Conflict(_)) + )); + let mut change = TicketStateChange::new("planning", "done", "finished", "finished"); + change.references.push(TicketReference { + kind: "report".to_owned(), + target: " \t ".to_owned(), + }); + assert!(matches!( + backend.set_workflow_state(reference.id.clone().into(), change), + Err(TicketError::Conflict(_)) + )); + assert!(matches!( + backend.close(reference.id.clone().into(), MarkdownText::new(" \n\t ")), + Err(TicketError::Conflict(_)) + )); + assert_eq!(backend.show(reference.id.into()).unwrap(), ticket); + } + + #[test] + fn completion_and_state_update_dispatch_round_trip_the_shared_api() { + let tmp = TempDir::new().unwrap(); + let backend = crate::tool::TicketToolBackend::new(backend(&tmp)); + let (reference, ticket) = backend + .create_with_snapshot(NewTicket::new("Dispatch")) + .unwrap(); + let operation = TicketBackendOperation::Complete { + ticket: "T-1".to_owned(), + request: completion_request(&ticket, "complete"), + }; + let encoded = serde_json::to_string(&operation).unwrap(); + let operation = serde_json::from_str(&encoded).unwrap(); + let TicketBackendOperationResult::Ticket(completed) = + execute_ticket_backend_operation(&backend, operation).unwrap() + else { + panic!("expected Ticket") + }; + assert_eq!(completed.meta.workflow_state, TicketWorkflowState::Done); + let operation = TicketBackendOperation::UpdateState { + ticket: reference.id, + request: TicketStateUpdate { + state: TicketWorkflowState::Closed, + ..completion_request(&completed, "close").into() + }, + }; + let encoded = serde_json::to_string(&operation).unwrap(); + let operation = serde_json::from_str(&encoded).unwrap(); + let TicketBackendOperationResult::Ticket(closed) = + execute_ticket_backend_operation(&backend, operation).unwrap() + else { + panic!("expected Ticket") + }; + assert_eq!(closed.meta.workflow_state, TicketWorkflowState::Closed); + } + + #[test] + fn queue_assignment_fence_names_only_the_explicit_ticket() { + let tmp = TempDir::new().unwrap(); + let backend = backend(&tmp); + let (reference, ticket) = backend + .create_with_snapshot(NewTicket::new("Queue fence")) + .unwrap(); + let other = backend.create(NewTicket::new("Other")).unwrap(); + let assignments = BTreeMap::from([ + (reference.id.clone(), "assignment".to_owned()), + (other.id, "other-assignment".to_owned()), + ]); + let fenced = backend.clone().with_event_attributes(BTreeMap::from([( + "queue_orchestrator_assignments".to_owned(), + serde_json::to_string(&assignments).unwrap(), + )])); + assert!( + fenced + .queue_ready(reference.id.clone().into(), "actor") + .is_err() + ); + assert_eq!(backend.show(reference.id.clone().into()).unwrap(), ticket); + let assignments = BTreeMap::from([(reference.id.clone(), "assignment".to_owned())]); + let fenced = backend.with_event_attributes(BTreeMap::from([( + "queue_orchestrator_assignments".to_owned(), + serde_json::to_string(&assignments).unwrap(), + )])); + assert_eq!( + fenced + .queue_ready(reference.id.clone().into(), "actor") + .unwrap() + .queued_tickets, + vec![reference.id.clone()] + ); + let current = fenced.show(reference.id.into()).unwrap(); + assert_eq!( + current + .events + .last() + .unwrap() + .attributes + .get("orchestrator_assignment_id") + .map(String::as_str), + Some("assignment") + ); + } + + #[test] + fn state_field_rejects_removed_phase_path_without_mutation() { + let tmp = TempDir::new().unwrap(); + let backend = backend(&tmp); + let (reference, ticket) = backend + .create_with_snapshot(NewTicket::new("No phase")) + .unwrap(); + assert!( + backend + .set_state_field( + reference.id.clone().into(), + "phase", + TicketStateChange::new("planning", "done", "finished", "finished") + ) + .is_err() + ); + assert_eq!(backend.show(reference.id.into()).unwrap(), ticket); + } + #[test] fn state_defaults_and_queue_transition_round_trip() { let tmp = TempDir::new().unwrap(); @@ -5913,36 +6736,36 @@ mod tests { assert_eq!(event.from.as_deref(), Some("ready")); assert_eq!(event.to.as_deref(), Some("queued")); assert_eq!(event.reason.as_deref(), Some("queued")); - let event_targets = serde_json::from_str::>( - event.attributes.get("targets").expect("canonical targets"), - ) - .unwrap(); - assert_eq!(event_targets.len(), 1); - assert_eq!(event_targets[0].repository_key, "main"); - assert_eq!(event_targets[0].access, TicketTargetAccess::ReadWrite); + assert!(!event.attributes.contains_key("targets")); + assert_eq!(queued.meta.targets[0].repository_key, "main"); assert!(!event.attributes.contains_key("repository_id")); assert!(!event.attributes.contains_key("ref_selector")); } #[test] - fn workflow_queue_rejects_non_ready_ticket_without_mutation() { + fn queue_accepts_all_states_without_targets() { let tmp = TempDir::new().unwrap(); let backend = backend(&tmp); - let ticket = backend.create(NewTicket::new("Planning Ticket")).unwrap(); - - assert!(matches!( - backend.queue_ready(TicketIdOrSlug::Id(ticket.id.clone()), "workspace-panel"), - Err(TicketError::StaleWorkflowState { .. }) - )); - let record = backend.show(TicketIdOrSlug::Id(ticket.id)).unwrap(); - assert_eq!(record.meta.workflow_state, TicketWorkflowState::Planning); - assert!(record.meta.queued_by.is_none()); - assert!( - !record - .events - .iter() - .any(|event| event.kind == TicketEventKind::StateChanged) - ); + for state in [ + TicketWorkflowState::Planning, + TicketWorkflowState::Ready, + TicketWorkflowState::Queued, + TicketWorkflowState::InProgress, + TicketWorkflowState::Done, + TicketWorkflowState::Closed, + ] { + let mut input = NewTicket::new(format!("Queue {state}")); + input.workflow_state = Some(state); + let ticket = backend.create(input).unwrap(); + let outcome = backend + .queue_ready(ticket.id.clone().into(), "actor") + .unwrap(); + assert_eq!(outcome.queued_tickets, vec![ticket.id.clone()]); + let current = backend.show(ticket.id.into()).unwrap(); + assert_eq!(current.meta.workflow_state, TicketWorkflowState::Queued); + assert_eq!(current.meta.status, ExtensibleTicketStatus::Open); + assert_eq!(current.events.last().unwrap().from, Some(state.to_string())); + } } #[test] @@ -6027,7 +6850,7 @@ mod tests { } #[test] - fn target_validation_rejects_duplicates_and_invalid_write_layouts_without_state_change() { + fn target_validation_rejects_duplicates_but_allows_read_only_targets() { let tmp = TempDir::new().unwrap(); let backend = backend(&tmp); @@ -6044,7 +6867,7 @@ mod tests { let mut no_write = NewTicket::new("No write target"); no_write.targets = vec![ticket_target("docs", None, TicketTargetAccess::ReadOnly)]; let ticket = backend.create(no_write).unwrap(); - let error = backend + let ready = backend .mark_ready( TicketIdOrSlug::Id(ticket.id.clone()), TicketMarkReady { @@ -6054,10 +6877,10 @@ mod tests { intake_summary: None, }, ) - .unwrap_err(); - assert!(matches!(error, TicketError::InvalidReadWriteTargetCount(0))); + .unwrap(); + assert_eq!(ready.meta.targets[0].access, TicketTargetAccess::ReadOnly); let unchanged = backend.show(TicketIdOrSlug::Id(ticket.id)).unwrap(); - assert_eq!(unchanged.meta.workflow_state, TicketWorkflowState::Planning); + assert_eq!(unchanged.meta.workflow_state, TicketWorkflowState::Ready); let mut multiple_writes = NewTicket::new("Multiple write targets"); multiple_writes.targets = vec![write_target("main"), write_target("docs")]; @@ -6086,38 +6909,46 @@ mod tests { } #[test] - fn targets_are_locked_after_planning_and_queue_revalidates_every_target() { + fn targets_can_be_edited_and_cleared_in_every_state() { let tmp = TempDir::new().unwrap(); let backend = backend(&tmp); - let mut ready_input = NewTicket::new("Ready with unknown reference target"); - ready_input.workflow_state = Some(TicketWorkflowState::Ready); - ready_input.targets = vec![ - ticket_target("main", Some("develop"), TicketTargetAccess::ReadWrite), - ticket_target("unknown", Some("main"), TicketTargetAccess::ReadOnly), - ]; - let ticket = backend.create(ready_input).unwrap(); - - let edit_error = backend - .edit_item( - TicketIdOrSlug::Id(ticket.id.clone()), - TicketItemEdit { - targets: Some(TicketTargetsEdit::Clear), - ..Default::default() - }, - ) - .unwrap_err(); - assert!(edit_error.to_string().contains("locked after planning")); - - let queue_error = backend - .queue_ready(TicketIdOrSlug::Id(ticket.id.clone()), "orchestrator") - .unwrap_err(); - assert!(matches!( - queue_error, - TicketError::UnknownTargetRepository(repository) if repository == "unknown" - )); - let unchanged = backend.show(TicketIdOrSlug::Id(ticket.id)).unwrap(); - assert_eq!(unchanged.meta.workflow_state, TicketWorkflowState::Ready); - assert!(unchanged.meta.queued_at.is_none()); + for state in [ + TicketWorkflowState::Planning, + TicketWorkflowState::Ready, + TicketWorkflowState::Queued, + TicketWorkflowState::InProgress, + TicketWorkflowState::Done, + TicketWorkflowState::Closed, + ] { + let mut input = NewTicket::new(format!("Targets {state}")); + input.workflow_state = Some(state); + let ticket = backend.create(input).unwrap(); + let set = backend + .edit_item( + ticket.id.clone().into(), + TicketItemEdit { + targets: Some(TicketTargetsEdit::Set { + targets: vec![write_target("main")], + }), + ..Default::default() + }, + ) + .unwrap(); + assert_eq!(set.meta.workflow_state, state); + assert_eq!(set.meta.targets, vec![write_target("main")]); + let cleared = backend + .edit_item( + ticket.id.into(), + TicketItemEdit { + targets: Some(TicketTargetsEdit::Clear), + ..Default::default() + }, + ) + .unwrap(); + assert_eq!(cleared.meta.workflow_state, state); + assert!(cleared.meta.targets.is_empty()); + assert_eq!(cleared.meta.resource_key, set.meta.resource_key); + } } #[test] @@ -6136,12 +6967,15 @@ mod tests { let record = backend.show(TicketIdOrSlug::Id(ticket.id)).unwrap(); assert_eq!(record.meta.status, ExtensibleTicketStatus::Closed); assert_eq!(record.meta.workflow_state, TicketWorkflowState::Closed); - assert!( - record - .events - .iter() - .any(|event| event.kind == TicketEventKind::Close) + let close = record.events.last().unwrap(); + assert_eq!(close.kind, TicketEventKind::Close); + assert_eq!(close.from.as_deref(), Some("planning")); + assert_eq!(close.to.as_deref(), Some("closed")); + assert_eq!( + close.attributes.get("expected_item_revision"), + Some(&format!("{}:0", record.meta.id)) ); + assert!(close.attributes.contains_key("operation_key")); } #[test] @@ -6256,7 +7090,7 @@ mod tests { } #[test] - fn queue_rejects_planning_dependency_and_incoming_blocker_without_mutation() { + fn queue_retains_planning_dependency_and_incoming_blocker_diagnostics() { let tmp = TempDir::new().unwrap(); let backend = backend(&tmp); let mut blocked_input = NewTicket::new("Blocked Ready"); @@ -6274,14 +7108,14 @@ mod tests { }, ) .unwrap(); - let error = backend + let outcome = backend .queue_ready(TicketIdOrSlug::Id(blocked.id.clone()), "test") - .unwrap_err(); - assert!(matches!(error, TicketError::BlockingRelations(_))); + .unwrap(); + assert_eq!(outcome.queued_tickets.len(), 1); let unchanged = backend .show(TicketIdOrSlug::Id(blocked.id.clone())) .unwrap(); - assert_eq!(unchanged.meta.workflow_state, TicketWorkflowState::Ready); + assert_eq!(unchanged.meta.workflow_state, TicketWorkflowState::Queued); assert_eq!(unchanged.relations.blockers.len(), 1); assert_eq!( unchanged.relations.blockers[0].blocking_ticket, @@ -6303,16 +7137,16 @@ mod tests { }, ) .unwrap(); - let error = backend + let outcome = backend .queue_ready(TicketIdOrSlug::Id(incoming.id.clone()), "test") - .unwrap_err(); - assert!(matches!(error, TicketError::BlockingRelations(_))); + .unwrap(); + assert_eq!(outcome.queued_tickets.len(), 1); let unchanged_incoming = backend .show(TicketIdOrSlug::Id(incoming.id.clone())) .unwrap(); assert_eq!( unchanged_incoming.meta.workflow_state, - TicketWorkflowState::Ready + TicketWorkflowState::Queued ); assert_eq!(unchanged_incoming.relations.blockers.len(), 1); assert_eq!( diff --git a/crates/ticket/src/tool.rs b/crates/ticket/src/tool.rs index c33f4817c..7b5a2f310 100644 --- a/crates/ticket/src/tool.rs +++ b/crates/ticket/src/tool.rs @@ -134,18 +134,15 @@ const DECISION_DESCRIPTION: &str = "Append a typed Ticket decision event. `body` const IMPLEMENTATION_REPORT_DESCRIPTION: &str = "Append a typed Ticket implementation_report event. `body` is Markdown."; const MARK_READY_DESCRIPTION: &str = "Mark a planning Ticket ready through the typed Ticket backend. \ -The backend atomically validates and normalizes every persisted repository target, requires at least one \ -read_write target, records one typed state_changed event, and transitions planning -> ready. `reason` is optional."; +Targets are optional; provided targets are validated against the Workspace catalog without Git resolution. \ +Records one typed state_changed event and transitions planning -> ready. `reason` is optional."; const INTAKE_READY_DESCRIPTION: &str = "Record a bounded intake summary and mark a planning Ticket ready. \ -The backend applies the same targets validation and lock as TicketMarkReady and commits the summary, \ +The backend applies the same optional catalog targets validation as TicketMarkReady and commits the summary, \ state_changed event, effective targets, and planning -> ready transition atomically."; -const QUEUE_DESCRIPTION: &str = "Queue a ready Ticket for Orchestrator routing through the typed \ -Ticket backend. The backend rejects transitive planning dependencies and cycles, atomically queues the \ -requested Ticket plus every transitive ready dependency, and leaves queued or in-progress dependencies unchanged."; -const WORKFLOW_STATE_DESCRIPTION: &str = "Transition Ticket `state` through the typed \ -Ticket backend with a bounded `state_changed` event. Treat `queued -> inprogress` \ -as the implementation acceptance step: implementation side effects should happen only after that \ -transition is accepted and recorded. Orchestrator may return `ready` or `queued` Tickets to `planning` only with a concrete missing decision/information reason."; +const QUEUE_DESCRIPTION: &str = "Queue only the explicitly requested Ticket for Orchestrator routing through the typed \ +Ticket backend, regardless of state or dependencies. Dependency diagnostics are retained; no dependencies are auto-queued."; +const WORKFLOW_STATE_DESCRIPTION: &str = "Change Ticket state with a bounded audited state_changed event. \ +State and dependency metadata do not impose an implementation phase path. Reopening does not revive released work identities."; const CLOSE_DESCRIPTION: &str = "Close a Ticket with a Markdown resolution through the typed Ticket \ backend. The backend sets the Ticket state to closed and appends a close event."; const RELATION_RECORD_DESCRIPTION: &str = "Record a forward typed Ticket-to-Ticket relation as durable \ @@ -321,6 +318,18 @@ impl TicketBackend for TicketToolBackend { self.backend.set_workflow_state(id, change) } + fn update_state( + &self, + ticket: &str, + request: crate::TicketStateUpdate, + ) -> TicketResult { + self.backend.update_state(ticket, request) + } + + fn complete(&self, ticket: &str, request: crate::TicketCompletion) -> TicketResult { + self.backend.complete(ticket, request) + } + fn mark_ready(&self, id: TicketIdOrSlug, request: TicketMarkReady) -> TicketResult { self.backend.mark_ready(id, request) } @@ -1953,7 +1962,7 @@ mod tests { ) -> crate::Result { Ok(crate::ResolvedTicketTarget { repository_key: repository_key.to_owned(), - ref_selector: ref_selector.unwrap_or("develop").to_owned(), + ref_selector: Some(ref_selector.unwrap_or("develop").to_owned()), access: crate::TicketTargetAccess::ReadOnly, }) } @@ -2061,15 +2070,21 @@ mod tests { } #[test] - fn state_tool_description_explains_queued_acceptance() { + fn state_tool_description_explains_phase_independence() { let temp = TempDir::new().unwrap(); let definition = ticket_tools(backend(&temp)) .into_iter() .find(|definition| definition().0.name == "TicketWorkflowState") .expect("state tool exists"); let (meta, _) = definition(); - assert!(meta.description.contains("queued -> inprogress")); - assert!(meta.description.contains("implementation side effects")); + assert!( + meta.description + .contains("do not impose an implementation phase path") + ); + assert!( + meta.description + .contains("does not revive released work identities") + ); } #[test] @@ -2668,10 +2683,10 @@ mod tests { ) .await .unwrap(); - assert!(queued.summary.contains("2 ticket(s)")); + assert!(queued.summary.contains("1 ticket(s)")); let queued_content = queued.content.unwrap(); assert!(queued_content.contains(&target_key)); - assert!(queued_content.contains(&dependency_key)); + assert!(!queued_content.contains(&dependency_key)); assert!(!queued_content.contains(&target.id)); assert!(!queued_content.contains(&dependency.id)); @@ -2931,79 +2946,24 @@ mod tests { } #[tokio::test] - async fn ticket_workflow_tool_rejects_disallowed_transition_graph_edges() { + async fn ticket_workflow_tool_allows_direct_state_changes_without_phase_path() { let temp = TempDir::new().unwrap(); let backend = backend(&temp); let workflow = tool_by_name(backend.clone(), "TicketWorkflowState"); - - let mut ready_input = NewTicket::new("Ready Bypass"); - ready_input.workflow_state = Some(TicketWorkflowState::Ready); - let ready = backend.create(ready_input).unwrap(); - let ready_error = workflow - .execute( - &json!({ - "ticket": ready.id, - "from": "ready", - "to": "inprogress", - "reason": "bypass_queue", - "body": "Should not bypass Queue.\n" - }) - .to_string(), - Default::default(), - ) - .await - .unwrap_err(); - assert!( - ready_error - .to_string() - .contains("invalid ticket workflow transition") - ); - - let mut done_input = NewTicket::new("Backward Bypass"); - done_input.workflow_state = Some(TicketWorkflowState::Done); - let done = backend.create(done_input).unwrap(); - let backward_error = workflow - .execute( - &json!({ - "ticket": done.id, - "from": "done", - "to": "planning", - "reason": "backwards", - "body": "Should not move backwards.\n" - }) - .to_string(), - Default::default(), - ) - .await - .unwrap_err(); - assert!( - backward_error - .to_string() - .contains("invalid ticket workflow transition") - ); - - let mut queued_input = NewTicket::new("Skip Bypass"); - queued_input.workflow_state = Some(TicketWorkflowState::Queued); - let queued = backend.create(queued_input).unwrap(); - let skip_error = workflow - .execute( - &json!({ - "ticket": queued.id, - "from": "queued", - "to": "done", - "reason": "skip_inprogress", - "body": "Should not skip inprogress.\n" - }) - .to_string(), - Default::default(), - ) - .await - .unwrap_err(); - assert!( - skip_error - .to_string() - .contains("invalid ticket workflow transition") - ); + for (from, to) in [ + (TicketWorkflowState::Ready, TicketWorkflowState::InProgress), + (TicketWorkflowState::Done, TicketWorkflowState::Planning), + (TicketWorkflowState::Queued, TicketWorkflowState::Done), + ] { + let mut input = NewTicket::new(format!("{from} to {to}")); + input.workflow_state = Some(from); + let ticket = backend.create(input).unwrap(); + workflow.execute(&json!({"ticket": ticket.id, "from": from.as_str(), "to": to.as_str(), "reason": "explicit_update", "body": "State metadata updated."}).to_string(), Default::default()).await.unwrap(); + assert_eq!( + backend.show(ticket.id.into()).unwrap().meta.workflow_state, + to + ); + } } #[tokio::test] diff --git a/crates/worker/src/feature/builtin/merge_request.rs b/crates/worker/src/feature/builtin/merge_request.rs index b1d1e786c..28ad9418d 100644 --- a/crates/worker/src/feature/builtin/merge_request.rs +++ b/crates/worker/src/feature/builtin/merge_request.rs @@ -41,13 +41,12 @@ fn workflow_instruction() -> FeatureInstructionDeclaration { .expect("static Merge Request workflow instruction declaration is valid") } -const ALL_KINDS: [Kind; 6] = [ +const ALL_KINDS: [Kind; 5] = [ Kind::Show, Kind::Open, Kind::Review, Kind::Readiness, Kind::Complete, - Kind::CompleteTicket, ]; #[derive(Clone, Copy, Debug, Eq, PartialEq)] @@ -56,7 +55,6 @@ enum Kind { Readiness, Open, Complete, - CompleteTicket, Review, } #[derive(Clone)] @@ -98,14 +96,6 @@ struct CompleteMergeRequestInput { resolution: MergeResolutionInput, } #[derive(Debug, Deserialize, JsonSchema)] -struct CompleteTicketInput { - ticket: String, - operation_id: String, - item_revision: String, - merge_request_ids: Vec, - requirement_approval_event_id: String, -} -#[derive(Debug, Deserialize, JsonSchema)] #[serde(rename_all = "snake_case")] enum MergeStrategyInput { FastForward, @@ -150,7 +140,7 @@ impl Kind { Self::Open => config.open, Self::Review => config.review, Self::Readiness => config.readiness_check, - Self::Complete | Self::CompleteTicket => config.complete, + Self::Complete => config.complete, } } @@ -160,7 +150,6 @@ impl Kind { Self::Readiness => "CheckMergeRequestReadiness", Self::Open => "OpenMergeRequest", Self::Complete => "CompleteMergeRequest", - Self::CompleteTicket => "CompleteTicket", Self::Review => "ReviewMergeRequest", } } @@ -170,16 +159,12 @@ impl Kind { Self::Readiness => json!(schemars::schema_for!(MergeRequestInput)), Self::Open => json!(schemars::schema_for!(OpenMergeRequestInput)), Self::Complete => json!(schemars::schema_for!(CompleteMergeRequestInput)), - Self::CompleteTicket => json!(schemars::schema_for!(CompleteTicketInput)), Self::Review => json!(schemars::schema_for!(ReviewMergeRequestInput)), } } fn mutating(self) -> bool { - matches!( - self, - Self::Open | Self::Review | Self::Complete | Self::CompleteTicket - ) + matches!(self, Self::Open | Self::Review | Self::Complete) } } #[async_trait] @@ -240,23 +225,6 @@ impl Tool for MergeRequestTool { ), ) } - Kind::CompleteTicket => { - let v: CompleteTicketInput = parse(input)?; - nonempty_named("ticket", &v.ticket)?; - ( - WorkspaceRequestMethod::Post, - format!( - "/api/w/{ws}/tickets/{}/complete", - encode_path_segment(&v.ticket) - ), - Some(json!({ - "operation_id": v.operation_id, - "item_revision": v.item_revision, - "merge_request_ids": v.merge_request_ids, - "requirement_approval_event_id": v.requirement_approval_event_id, - })), - ) - } Kind::Review => { let v: ReviewMergeRequestInput = parse(input)?; let ctx = self.client.reviewer_context().ok_or_else(|| { @@ -499,11 +467,6 @@ fn native_merge_request_operation(kind: Kind) -> NativeMergeRequestOperation { surface: NativeMergeRequestSurface::Item, identity_field: Some("merge_request_id"), }, - Kind::CompleteTicket => NativeMergeRequestOperation { - operation: "complete_ticket", - surface: NativeMergeRequestSurface::Collection, - identity_field: None, - }, } } @@ -584,8 +547,8 @@ pub fn mount_workspace_http_merge_request_wip( let collection_descriptor = merge_request_descriptor( &collection_tools, - "Open Merge Requests and complete Tickets through existing scoped authority", - "Only collection-scoped Merge Request Feature operations enabled for this Worker are published. Opening retains repository and selector inputs; Ticket completion retains its full guarded result-set inputs.", + "Open Merge Requests through existing scoped authority", + "Only collection-scoped Merge Request Feature operations enabled for this Worker are published. Opening retains repository and selector inputs; Ticket completion is a separate Ticket operation.", )?; registry.mount(WipProjection { route: collection_route.clone(), @@ -617,7 +580,7 @@ pub fn mount_workspace_http_merge_request_wip( let item_descriptor = merge_request_descriptor( &item_tools, "Inspect and operate on the Merge Request bound to this object route", - "The subject Merge Request identity comes exclusively from the target route. Repository selectors, exact review capability/candidate, Ticket result snapshot, approval, target refs, strategy, and resolution remain ordinary Backend-validated preconditions.", + "The subject Merge Request identity comes exclusively from the target route. Repository selectors, exact review capability/candidate, approval, target refs, strategy, and resolution remain ordinary Backend-validated preconditions.", )?; registry.mount_dynamic(WipDynamicMount { collection_route: collection_route.clone(), @@ -665,18 +628,6 @@ impl WipOperationHandler for MergeRequestCollectionWipHandler { .get(operation) .ok_or_else(merge_request_operation_not_found)?; let input = native_merge_request_input(arguments, None, tool.projection.identity_field)?; - let affected_merge_requests = if tool.kind == Kind::CompleteTicket { - input - .get("merge_request_ids") - .and_then(Value::as_array) - .into_iter() - .flatten() - .filter_map(Value::as_str) - .map(ToOwned::to_owned) - .collect::>() - } else { - Vec::new() - }; let output = execute_native_merge_request_tool(tool, &self.permissions, input, None, context) .await?; @@ -699,18 +650,6 @@ impl WipOperationHandler for MergeRequestCollectionWipHandler { ); } } - Kind::CompleteTicket => { - for reference in affected_merge_requests { - if is_merge_request_route_reference(&reference) { - record_merge_request_observation( - &self.revisions, - &reference, - &response, - true, - ); - } - } - } _ => {} } Ok(WipOperationOutput::native( @@ -1148,9 +1087,6 @@ pub fn description(n: &str) -> Option<&'static str> { "CompleteMergeRequest" => Some( "Record Orchestrator-owned integration for the explicitly addressed Merge Request without completing the Ticket or releasing its assignment.", ), - "CompleteTicket" => Some( - "Complete the Ticket only after reviewing its current item revision and exact linked Merge Request result set; this ends unfinished work atomically while retaining responsibility and assignment history.", - ), "ReviewMergeRequest" => Some( "Submit the injected Reviewer capability result for its captured exact source ref; source movement cancels it, while target-only movement does not.", ), @@ -1220,7 +1156,6 @@ mod tests { "CheckMergeRequestReadiness", "OpenMergeRequest", "CompleteMergeRequest", - "CompleteTicket", "ReviewMergeRequest", ] { assert!(description(name).is_some(), "missing operation {name}"); @@ -1369,8 +1304,7 @@ mod tests { [ "ShowMergeRequest", "CheckMergeRequestReadiness", - "CompleteMergeRequest", - "CompleteTicket" + "CompleteMergeRequest" ] ); assert_eq!(install(coder).1, [FEATURE_PROMPT_REF]); @@ -1427,7 +1361,6 @@ mod tests { (NativeMergeRequestSurface::Item, "read"), (NativeMergeRequestSurface::Item, "check_readiness"), (NativeMergeRequestSurface::Item, "complete"), - (NativeMergeRequestSurface::Collection, "complete_ticket"), ] ); diff --git a/crates/worker/src/feature/builtin/resource_projection.rs b/crates/worker/src/feature/builtin/resource_projection.rs index fb002b876..a3367826c 100644 --- a/crates/worker/src/feature/builtin/resource_projection.rs +++ b/crates/worker/src/feature/builtin/resource_projection.rs @@ -176,6 +176,8 @@ struct ModelMergeRequest { #[derive(Debug, Serialize)] struct ModelTicketEvidence { + /// MR evidence is inapplicable without a linked MR, independent of Ticket decisions. + integration_applicable: bool, has_merge_request: bool, has_current_subject_ref: bool, has_review_request: bool, @@ -535,8 +537,10 @@ fn project_merge_request(value: &Value) -> Result { fn project_evidence(value: &Value) -> Result { let evidence = object(value, "Ticket evidence")?; - Ok(ModelTicketEvidence { - has_merge_request: bool_field(evidence, "has_merge_request")?, + let has_merge_request = bool_field(evidence, "has_merge_request")?; + let projected = ModelTicketEvidence { + integration_applicable: has_merge_request, + has_merge_request, has_current_subject_ref: bool_field(evidence, "has_current_subject_ref")?, has_review_request: bool_field(evidence, "has_review_request")?, has_commit: bool_field(evidence, "has_commit")?, @@ -546,7 +550,20 @@ fn project_evidence(value: &Value) -> Result { unresolved_request_changes: bool_field(evidence, "unresolved_request_changes")?, complete_for_integration: bool_field(evidence, "complete_for_integration")?, missing: string_array(evidence, "missing")?, - }) + }; + if !has_merge_request { + // Empty MR sets are neither missing evidence nor vacuous approval. + return Ok(ModelTicketEvidence { + review_status: None, + approved_current_subject: false, + review_after_rescope: false, + unresolved_request_changes: false, + complete_for_integration: false, + missing: Vec::new(), + ..projected + }); + } + Ok(projected) } fn project_actions(value: &Value) -> Result { @@ -677,6 +694,38 @@ mod tests { use super::*; use serde_json::json; + #[test] + fn mrless_evidence_is_inapplicable_not_missing_or_vacuously_approved() { + for state in [ + "planning", + "ready", + "queued", + "inprogress", + "done", + "closed", + ] { + let projected = project_ticket_detail(json!({ + "resource_key": "T-718", "item_revision": "rev-1", "title": "No Git required", "body": "Decision", + "state": state, "events": [{"sequence": 1, "kind": "completed", "body": "No Git needed"}], + "linked_objectives": [], "assignments": [], "implementation_reports": [], + "merge_requests": [], "merge_request": null, + "evidence": { + "has_merge_request": false, "has_current_subject_ref": false, "has_review_request": false, "has_commit": false, + "review_status": "approved", "approved_current_subject": true, "review_after_rescope": true, + "unresolved_request_changes": false, "complete_for_integration": true, "missing": ["merge_request"] + } + })).unwrap(); + let projected = serde_json::to_value(projected).unwrap(); + assert_eq!(projected["state"], state); + assert_eq!(projected["thread"][0]["kind"], "completed"); + assert_eq!(projected["evidence"]["integration_applicable"], false); + assert_eq!(projected["evidence"]["review_status"], Value::Null); + assert_eq!(projected["evidence"]["approved_current_subject"], false); + assert_eq!(projected["evidence"]["complete_for_integration"], false); + assert_eq!(projected["evidence"]["missing"], json!([])); + } + } + #[test] fn objective_projection_exposes_only_resource_references() { let projected = project_objective_detail(json!({ diff --git a/crates/worker/src/feature/builtin/ticket.rs b/crates/worker/src/feature/builtin/ticket.rs index 34fa64ba7..0a0883918 100644 --- a/crates/worker/src/feature/builtin/ticket.rs +++ b/crates/worker/src/feature/builtin/ticket.rs @@ -285,6 +285,227 @@ fn workspace_ticket_read_definition( }) } +#[derive(Debug, Deserialize, Serialize, JsonSchema)] +#[serde(deny_unknown_fields)] +struct WorkspaceCompleteTicketInput { + /// Ticket reference. Prefer `T-*`. + ticket: String, + /// Stable key for exact replay of this completion decision. + operation_key: String, + /// Current item revision from ShowTicket. + expected_item_revision: String, + expected_state: TicketWorkflowState, + /// Required explanation of the completion decision; MR approval is not a prerequisite. + reason: String, + /// Optional typed supporting Ticket references, independent of MR integration evidence. + #[serde(default)] + references: Vec, +} + +#[derive(Debug, Deserialize, Serialize, JsonSchema)] +#[serde(deny_unknown_fields)] +struct WorkspaceTicketStateUpdateInput { + /// Ticket reference. Prefer `T-*`. + ticket: String, + operation_key: String, + /// Current item revision from ShowTicket. + expected_item_revision: String, + expected_state: TicketWorkflowState, + /// Progress-display state only: this does not start Workers or integrate Merge Requests. + state: TicketWorkflowState, + /// Required explanation for the state decision. + reason: String, + #[serde(default)] + references: Vec, +} + +#[derive(Debug, Deserialize, Serialize, JsonSchema)] +#[serde(deny_unknown_fields)] +struct WorkspaceTicketCloseInput { + /// Ticket reference. Prefer `T-*`; WIP binds this from the item route. + ticket: String, + /// Stable key for exact replay of this close decision. + operation_key: String, + /// Current item revision from ShowTicket. + expected_item_revision: String, + expected_state: TicketWorkflowState, + /// Required explanation for closing this Ticket. + resolution: String, + #[serde(default)] + references: Vec, +} + +const TICKET_CLOSE_DESCRIPTION: &str = "Close a Ticket with resolution, item/state CAS, and replay key. No Merge Request or enforced state sequence is required. Actor identity is transport-bound; this does not integrate Merge Requests or expand Workdir grants."; +const COMPLETE_TICKET_DESCRIPTION: &str = "Record a Ticket completion decision with reason, item/state CAS, and replay key. Merge Requests and approvals are not prerequisites. The Backend authorizes the assigned Worker or registered Workspace Orchestrator; roles supplied by the model never grant authority."; +const TICKET_STATE_UPDATE_DESCRIPTION: &str = "Update a Ticket progress-display state with reason, item/state CAS, and replay key. States need not follow an enforced sequence. This does not start Workers, queue dependencies, integrate Merge Requests, or enlarge live Workdir grants; the Backend validates actor authority."; + +#[derive(Clone, Copy)] +enum WorkspaceTicketDecisionKind { + Complete, + UpdateState, + Close, +} + +impl WorkspaceTicketDecisionKind { + fn name(self) -> &'static str { + match self { + Self::Complete => "CompleteTicket", + Self::UpdateState => "TicketWorkflowState", + Self::Close => "TicketClose", + } + } + + fn description(self) -> &'static str { + match self { + Self::Complete => COMPLETE_TICKET_DESCRIPTION, + Self::UpdateState => TICKET_STATE_UPDATE_DESCRIPTION, + Self::Close => TICKET_CLOSE_DESCRIPTION, + } + } + + fn schema(self) -> Value { + match self { + Self::Complete => json!(schemars::schema_for!(WorkspaceCompleteTicketInput)), + Self::UpdateState => json!(schemars::schema_for!(WorkspaceTicketStateUpdateInput)), + Self::Close => json!(schemars::schema_for!(WorkspaceTicketCloseInput)), + } + } +} + +#[derive(Clone)] +struct WorkspaceTicketDecisionTool { + client: Arc, + kind: WorkspaceTicketDecisionKind, +} + +fn validate_ticket_decision( + ticket: &str, + operation_key: &str, + revision: &str, + reason: &str, +) -> Result<(), ToolError> { + for (name, value) in [ + ("ticket", ticket), + ("operation_key", operation_key), + ("expected_item_revision", revision), + ("reason", reason), + ] { + if value.trim().is_empty() { + return Err(ToolError::InvalidArgument(format!( + "{name} must not be empty" + ))); + } + } + Ok(()) +} + +#[async_trait] +impl Tool for WorkspaceTicketDecisionTool { + async fn execute(&self, input: &str, _: ToolExecutionContext) -> Result { + let workspace_id = self.client.workspace_id().ok_or_else(|| { + ToolError::InvalidArgument("Ticket decisions require Workspace identity".into()) + })?; + let (ticket, endpoint, body) = match self.kind { + WorkspaceTicketDecisionKind::Complete => { + let value: WorkspaceCompleteTicketInput = serde_json::from_str(input) + .map_err(|error| ToolError::InvalidArgument(error.to_string()))?; + validate_ticket_decision( + &value.ticket, + &value.operation_key, + &value.expected_item_revision, + &value.reason, + )?; + let mut body = serde_json::to_value(&value) + .map_err(|error| ToolError::Internal(error.to_string()))?; + body.as_object_mut() + .expect("typed Ticket input is an object") + .remove("ticket"); + (value.ticket, "complete", body) + } + WorkspaceTicketDecisionKind::Close => { + let value: WorkspaceTicketCloseInput = serde_json::from_str(input) + .map_err(|error| ToolError::InvalidArgument(error.to_string()))?; + validate_ticket_decision( + &value.ticket, + &value.operation_key, + &value.expected_item_revision, + &value.resolution, + )?; + let body = json!({ + "operation_key": value.operation_key, + "expected_item_revision": value.expected_item_revision, + "expected_state": value.expected_state, + "state": TicketWorkflowState::Closed, + "reason": value.resolution, + "references": value.references, + }); + (value.ticket, "state-update", body) + } + WorkspaceTicketDecisionKind::UpdateState => { + let value: WorkspaceTicketStateUpdateInput = serde_json::from_str(input) + .map_err(|error| ToolError::InvalidArgument(error.to_string()))?; + validate_ticket_decision( + &value.ticket, + &value.operation_key, + &value.expected_item_revision, + &value.reason, + )?; + let mut body = serde_json::to_value(&value) + .map_err(|error| ToolError::Internal(error.to_string()))?; + body.as_object_mut() + .expect("typed Ticket input is an object") + .remove("ticket"); + // Actor identity is transport-bound, never a model-visible author/role input. + (value.ticket, "state-update", body) + } + }; + let ticket: Ticket = WorkspaceHttpTicketBackend::request( + self.client.clone(), + WorkspaceRequestMethod::Post, + format!( + "/api/w/{workspace_id}/tickets/{}/{endpoint}", + WorkspaceHttpTicketBackend::ticket_path(&TicketIdOrSlug::from(ticket.as_str())) + ), + Some(body), + ) + .map_err(|error| ToolError::ExecutionFailed(error.to_string()))?; + Ok(ToolOutput { + summary: self.kind.name().into(), + content: Some( + serde_json::to_string(&ticket) + .map_err(|error| ToolError::Internal(error.to_string()))?, + ), + attachments: Vec::new(), + }) + } +} + +fn workspace_ticket_decision_definition( + client: Arc, + kind: WorkspaceTicketDecisionKind, +) -> ToolDefinition { + Arc::new(move || { + ( + ToolMeta::new(kind.name()) + .description(kind.description()) + .input_schema(kind.schema()), + Arc::new(WorkspaceTicketDecisionTool { + client: client.clone(), + kind, + }) as Arc, + ) + }) +} + +fn workspace_ticket_tool_description(name: &str) -> String { + match name { + "CompleteTicket" => COMPLETE_TICKET_DESCRIPTION.into(), + "TicketWorkflowState" => TICKET_STATE_UPDATE_DESCRIPTION.into(), + "TicketClose" => TICKET_CLOSE_DESCRIPTION.into(), + _ => ticket_tool_description(name, None), + } +} + const FEATURE_ID: &str = "ticket"; const FEATURE_NAME: &str = "Ticket tools"; const FEATURE_DESCRIPTION: &str = @@ -411,11 +632,16 @@ impl TicketFeatureAccess { .iter() .copied() .filter(|name| self.allows_tool(name)) + .chain((self.authoring || self.workflow || self.thread).then_some("CompleteTicket")) .collect() } fn allows_tool(self, name: &str) -> bool { + // Publishing a decision tool is not an authority grant. The injected + // Workspace transport lets the Backend validate the current assignment + // or author/orchestrator permission on every decision, in both modes. READ_ONLY_TOOL_NAMES.contains(&name) + || (name == "TicketWorkflowState" && (self.authoring || self.thread || self.workflow)) || (self.authoring && AUTHORING_TOOL_NAMES.contains(&name)) || (self.thread && THREAD_TOOL_NAMES.contains(&name)) || (self.intake && INTAKE_TOOL_NAMES.contains(&name)) @@ -448,9 +674,11 @@ const WORKSPACE_AUTHORING_TOOL_NAMES: &[&str] = &[ "TicketComment", "TicketMarkReady", "TicketQueue", + "TicketWorkflowState", "TicketClose", "TicketRelationRecord", "TicketRelationRemove", + "CompleteTicket", ]; #[cfg(test)] @@ -465,6 +693,7 @@ const WORKFLOW_TOOL_NAMES: &[&str] = &[ "TicketRelationRemove", "TicketOrchestrationPlanRecord", "TicketOrchestrationPlanQuery", + "CompleteTicket", ]; const WORKFLOW_ADDITIONAL_TOOL_NAMES: &[&str] = &[ @@ -518,7 +747,7 @@ impl FeatureModule for TicketFeature { for name in enabled_tool_names { descriptor = descriptor.with_tool(ToolDeclaration::new( name, - ticket_tool_description(name, None), + workspace_ticket_tool_description(name), )); } descriptor @@ -744,6 +973,49 @@ impl WorkspaceHttpTicketBackend { Ok(TicketBackendOperationResult::Unit) } + fn legacy_state_request( + client: Arc, + base: &str, + id: &TicketIdOrSlug, + change: TicketStateChange, + ) -> TicketResult { + // Preserve the caller's original `from`: refreshing it from the snapshot + // would turn a stale transition into a successful write. + let expected_state = TicketWorkflowState::parse(&change.from).ok_or_else(|| { + TicketError::Conflict(format!("invalid Ticket from state: {}", change.from)) + })?; + let state = TicketWorkflowState::parse(&change.to).ok_or_else(|| { + TicketError::Conflict(format!("invalid Ticket to state: {}", change.to)) + })?; + let reason = [change.reason.as_str(), change.body.as_str()] + .into_iter() + .map(str::trim) + .filter(|part| !part.is_empty()) + .collect::>() + .join("\n\n"); + if reason.is_empty() { + return Err(TicketError::Conflict( + "Ticket state decision requires a reason".into(), + )); + } + let snapshot: Ticket = Self::request_unprojected( + client, + WorkspaceRequestMethod::Get, + format!("{base}/{}/record", Self::ticket_path(id)), + None, + )?; + serde_json::to_value(ticket::TicketStateUpdate { + operation_key: uuid::Uuid::now_v7().to_string(), + expected_item_revision: ticket::ticket_item_revision(&snapshot), + expected_state, + state, + reason: reason.into(), + references: change.references, + author: change.author, + }) + .map_err(|error| TicketError::Conflict(format!("serialize Ticket state decision: {error}"))) + } + fn invoke_client( client: Arc, workspace_id: String, @@ -836,14 +1108,15 @@ impl WorkspaceHttpTicketBackend { TicketError::Conflict(format!("serialize Ticket event: {error}")) })?), ), - TicketBackendOperation::AddStateChanged { id, change } => Self::request_unit( - client, - WorkspaceRequestMethod::Post, - format!("{base}/{}/state-changes", Self::ticket_path(&id)), - Some(serde_json::to_value(change).map_err(|error| { - TicketError::Conflict(format!("serialize Ticket state change: {error}")) - })?), - ), + TicketBackendOperation::AddStateChanged { id, change } => { + let body = Self::legacy_state_request(client.clone(), &base, &id, change)?; + Self::request_unit( + client, + WorkspaceRequestMethod::Post, + format!("{base}/{}/state-changes", Self::ticket_path(&id)), + Some(body), + ) + } TicketBackendOperation::AddIntakeSummary { id, summary } => Self::request_unit( client, WorkspaceRequestMethod::Post, @@ -852,26 +1125,57 @@ impl WorkspaceHttpTicketBackend { TicketError::Conflict(format!("serialize Ticket intake summary: {error}")) })?), ), - TicketBackendOperation::SetStateField { id, field, change } => Self::request_unit( + TicketBackendOperation::SetStateField { id, field, change } => { + if field != "workflow_state" && field != "state" { + return Err(TicketError::Conflict(format!( + "unsupported Ticket state field: {field}" + ))); + } + let body = Self::legacy_state_request(client.clone(), &base, &id, change)?; + Self::request_unit( + client, + WorkspaceRequestMethod::Post, + format!( + "{base}/{}/state-fields/{}", + Self::ticket_path(&id), + Self::ticket_path(&TicketIdOrSlug::Query(field)) + ), + Some(body), + ) + } + TicketBackendOperation::SetWorkflowState { id, change } => { + let body = Self::legacy_state_request(client.clone(), &base, &id, change)?; + Self::request_unit( + client, + WorkspaceRequestMethod::Post, + format!("{base}/{}/workflow-state", Self::ticket_path(&id)), + Some(body), + ) + } + TicketBackendOperation::UpdateState { ticket, request } => Self::request( client, WorkspaceRequestMethod::Post, format!( - "{base}/{}/state-fields/{}", - Self::ticket_path(&id), - Self::ticket_path(&TicketIdOrSlug::Query(field)) + "{base}/{}/state-update", + Self::ticket_path(&TicketIdOrSlug::from(ticket.as_str())) ), - Some(serde_json::to_value(change).map_err(|error| { - TicketError::Conflict(format!("serialize Ticket state field change: {error}")) + Some(serde_json::to_value(request).map_err(|error| { + TicketError::Conflict(format!("serialize Ticket state update: {error}")) })?), - ), - TicketBackendOperation::SetWorkflowState { id, change } => Self::request_unit( + ) + .map(TicketBackendOperationResult::Ticket), + TicketBackendOperation::Complete { ticket, request } => Self::request( client, WorkspaceRequestMethod::Post, - format!("{base}/{}/workflow-state", Self::ticket_path(&id)), - Some(serde_json::to_value(change).map_err(|error| { - TicketError::Conflict(format!("serialize Ticket workflow change: {error}")) + format!( + "{base}/{}/complete", + Self::ticket_path(&TicketIdOrSlug::from(ticket.as_str())) + ), + Some(serde_json::to_value(request).map_err(|error| { + TicketError::Conflict(format!("serialize Ticket completion: {error}")) })?), - ), + ) + .map(TicketBackendOperationResult::Ticket), TicketBackendOperation::MarkReady { id, request } => Self::request( client, WorkspaceRequestMethod::Post, @@ -888,14 +1192,36 @@ impl WorkspaceHttpTicketBackend { None, ) .map(TicketBackendOperationResult::QueueOutcome), - TicketBackendOperation::Close { id, resolution } => Self::request_unit( - client, - WorkspaceRequestMethod::Post, - format!("{base}/{}/workflow/close", Self::ticket_path(&id)), - Some(serde_json::to_value(resolution).map_err(|error| { + TicketBackendOperation::Close { id, resolution } => { + if resolution.as_str().trim().is_empty() { + return Err(TicketError::Conflict( + "Ticket close requires a resolution".into(), + )); + } + let snapshot: Ticket = Self::request_unprojected( + client.clone(), + WorkspaceRequestMethod::Get, + format!("{base}/{}/record", Self::ticket_path(&id)), + None, + )?; + let body = serde_json::to_value(ticket::TicketCompletion { + operation_key: uuid::Uuid::now_v7().to_string(), + expected_item_revision: ticket::ticket_item_revision(&snapshot), + expected_state: snapshot.meta.workflow_state, + reason: resolution.as_str().to_owned(), + references: Vec::new(), + author: None, + }) + .map_err(|error| { TicketError::Conflict(format!("serialize Ticket close: {error}")) - })?), - ), + })?; + Self::request_unit( + client, + WorkspaceRequestMethod::Post, + format!("{base}/{}/workflow/close", Self::ticket_path(&id)), + Some(body), + ) + } TicketBackendOperation::AddTicketRelation { id, relation } => { let source_resource_key = Self::resolve_ticket_resource_key(client.clone(), &base, &id)?; @@ -1114,6 +1440,20 @@ impl TicketBackend for WorkspaceHttpTicketBackend { } } + fn update_state( + &self, + ticket: &str, + request: ticket::TicketStateUpdate, + ) -> TicketResult { + expect_ticket_result!( + self.invoke(TicketBackendOperation::UpdateState { + ticket: ticket.to_string(), + request + }), + TicketBackendOperationResult::Ticket + ) + } + fn mark_ready( &self, id: TicketIdOrSlug, @@ -1292,6 +1632,12 @@ fn native_ticket_operation(tool_name: &str) -> Option { identity_field: Some("ticket"), mutating: true, }, + "CompleteTicket" => NativeTicketOperation { + operation: "complete", + surface: NativeTicketSurface::Item, + identity_field: Some("ticket"), + mutating: true, + }, "TicketClose" => NativeTicketOperation { operation: "close", surface: NativeTicketSurface::Item, @@ -1348,7 +1694,7 @@ pub(crate) fn enabled_ticket_definitions( ) -> Vec { let backend = TicketToolBackend::new(WorkspaceHttpTicketBackend::new(client.clone())); let allowed = access.tool_names(); - ticket_tools(backend) + let mut definitions: Vec = ticket_tools(backend) .into_iter() .filter_map(|definition| { let (meta, _) = definition(); @@ -1362,10 +1708,25 @@ pub(crate) fn enabled_ticket_definitions( "ShowTicket" => { workspace_ticket_read_definition(client.clone(), WorkspaceTicketReadKind::Show) } + "TicketClose" => workspace_ticket_decision_definition( + client.clone(), + WorkspaceTicketDecisionKind::Close, + ), + "TicketWorkflowState" => workspace_ticket_decision_definition( + client.clone(), + WorkspaceTicketDecisionKind::UpdateState, + ), _ => definition, }) }) - .collect() + .collect(); + if allowed.contains(&"CompleteTicket") { + definitions.push(workspace_ticket_decision_definition( + client, + WorkspaceTicketDecisionKind::Complete, + )); + } + definitions } fn native_ticket_tools( @@ -2016,6 +2377,637 @@ mod tests { use std::net::TcpListener; use std::thread; + #[derive(Debug)] + struct DecisionClient { + requests: Mutex>, + response: crate::worker::WorkspaceResponse, + } + + impl WorkspaceClient for DecisionClient { + fn workspace_id(&self) -> Option<&str> { + Some("workspace") + } + fn kind(&self) -> &str { + "test-decisions" + } + fn is_available(&self) -> bool { + true + } + fn execute( + &self, + request: WorkspaceRequest, + ) -> Result { + let mut response = self.response.clone(); + // Legacy adapters read a snapshot before the mutation. Let tests + // inject mutation rejection independently of that successful read. + if request.method == WorkspaceRequestMethod::Get { + response.status = 200; + } + self.requests.lock().unwrap().push(request); + Ok(response) + } + } + + fn decision_client(status: u16, state: TicketWorkflowState) -> Arc { + let temp = tempfile::tempdir().unwrap(); + let db = + ticket::SqliteTicketBackend::open(temp.path().join("tickets.db"), "workspace").unwrap(); + let created = db.create(NewTicket::new("Decision")).unwrap(); + let mut record = db.show(TicketIdOrSlug::Id(created.id)).unwrap(); + record.meta.resource_key = Some("T-718".into()); + record.meta.workflow_state = state; + Arc::new(DecisionClient { + requests: Mutex::new(Vec::new()), + response: crate::worker::WorkspaceResponse { + status, + body: serde_json::to_string(&record).unwrap(), + }, + }) + } + + fn decision_input() -> Value { + json!({ + "ticket": "T-718", "operation_key": "decision-1", + "expected_item_revision": "revision-1", "expected_state": "planning", + "reason": "No repository changes are needed" + }) + } + + fn close_input() -> Value { + let mut input = decision_input(); + let reason = input.as_object_mut().unwrap().remove("reason").unwrap(); + input["resolution"] = reason; + input + } + + fn legacy_change() -> TicketStateChange { + serde_json::from_value(json!({ + "from": "planning", "to": "closed", "reason": "Decision", + "body": "Supporting explanation", "references": [], "author": "caller" + })) + .unwrap() + } + + #[test] + fn legacy_ticket_state_adapters_send_strong_cas_without_refreshing_stale_from() { + for endpoint in [ + "state-changes", + "workflow-state", + "state-fields/workflow_state", + "state-fields/state", + ] { + // The observed state is ready, deliberately different from change.from. + let client = decision_client(200, TicketWorkflowState::Ready); + let snapshot: Ticket = serde_json::from_str(&client.response.body).unwrap(); + let backend = WorkspaceHttpTicketBackend::new(client.clone()); + let id = TicketIdOrSlug::from("T-718"); + match endpoint { + "state-changes" => backend.add_state_changed(id, legacy_change()), + "workflow-state" => backend.set_workflow_state(id, legacy_change()), + "state-fields/workflow_state" => { + backend.set_state_field(id, "workflow_state", legacy_change()) + } + _ => backend.set_state_field(id, "state", legacy_change()), + } + .unwrap(); + let requests = client.requests.lock().unwrap(); + assert_eq!(requests.len(), 2); + assert_eq!(requests[0].method, WorkspaceRequestMethod::Get); + assert_eq!(requests[0].path, "/api/w/workspace/tickets/T-718/record"); + assert_eq!(requests[1].method, WorkspaceRequestMethod::Post); + assert_eq!( + requests[1].path, + format!("/api/w/workspace/tickets/T-718/{endpoint}") + ); + let body: Value = serde_json::from_str(requests[1].body.as_deref().unwrap()).unwrap(); + let request: ticket::TicketStateUpdate = serde_json::from_value(body.clone()).unwrap(); + assert!(uuid::Uuid::parse_str(&request.operation_key).is_ok()); + assert_eq!( + request.expected_item_revision, + ticket::ticket_item_revision(&snapshot) + ); + assert_eq!(request.expected_state, TicketWorkflowState::Planning); + assert_eq!(request.state, TicketWorkflowState::Closed); + assert_eq!(request.reason, "Decision\n\nSupporting explanation"); + assert_eq!(body["author"], "caller"); + assert_eq!(body["references"], json!([])); + assert!(body.get("from").is_none()); + assert!(body.get("to").is_none()); + } + } + + #[test] + fn legacy_ticket_state_adapter_parses_each_original_display_state() { + let client = decision_client(200, TicketWorkflowState::Closed); + let backend = WorkspaceHttpTicketBackend::new(client.clone()); + for state in [ + TicketWorkflowState::Planning, + TicketWorkflowState::Ready, + TicketWorkflowState::Queued, + TicketWorkflowState::InProgress, + TicketWorkflowState::Done, + TicketWorkflowState::Closed, + ] { + let mut change = legacy_change(); + change.from = state.as_str().into(); + change.to = state.as_str().into(); + backend + .set_workflow_state(TicketIdOrSlug::from("T-718"), change) + .unwrap(); + let requests = client.requests.lock().unwrap(); + let request: ticket::TicketStateUpdate = + serde_json::from_str(requests.last().unwrap().body.as_deref().unwrap()).unwrap(); + assert_eq!(request.expected_state, state); + assert_eq!(request.state, state); + } + } + + #[test] + fn legacy_ticket_adapters_reject_invalid_fields_states_and_empty_reasons_before_dispatch() { + let client = decision_client(200, TicketWorkflowState::Planning); + let backend = WorkspaceHttpTicketBackend::new(client.clone()); + assert!( + backend + .set_state_field( + TicketIdOrSlug::from("T-718"), + "approval_state", + legacy_change() + ) + .is_err() + ); + for field in ["from", "to"] { + let mut change = legacy_change(); + if field == "from" { + change.from = "approved".into(); + } else { + change.to = "approved".into(); + } + assert!( + backend + .set_workflow_state(TicketIdOrSlug::from("T-718"), change) + .is_err() + ); + } + let mut change = legacy_change(); + change.reason = " ".into(); + change.body = " \n ".into(); + assert!( + backend + .add_state_changed(TicketIdOrSlug::from("T-718"), change) + .is_err() + ); + assert!( + backend + .close(TicketIdOrSlug::from("T-718"), " ".into()) + .is_err() + ); + assert!(client.requests.lock().unwrap().is_empty()); + } + + #[test] + fn legacy_ticket_close_snapshots_cas_and_generates_distinct_completion_keys() { + let client = decision_client(200, TicketWorkflowState::Ready); + let snapshot: Ticket = serde_json::from_str(&client.response.body).unwrap(); + let backend = WorkspaceHttpTicketBackend::new(client.clone()); + for _ in 0..2 { + backend + .close( + TicketIdOrSlug::from("T-718"), + "Resolved without repository changes".into(), + ) + .unwrap(); + } + let requests = client.requests.lock().unwrap(); + assert_eq!(requests.len(), 4); + let mut keys = BTreeSet::new(); + for pair in requests.chunks_exact(2) { + assert_eq!(pair[0].method, WorkspaceRequestMethod::Get); + assert_eq!(pair[0].path, "/api/w/workspace/tickets/T-718/record"); + assert_eq!(pair[1].method, WorkspaceRequestMethod::Post); + assert_eq!( + pair[1].path, + "/api/w/workspace/tickets/T-718/workflow/close" + ); + let request: ticket::TicketCompletion = + serde_json::from_str(pair[1].body.as_deref().unwrap()).unwrap(); + assert!(uuid::Uuid::parse_str(&request.operation_key).is_ok()); + assert!(keys.insert(request.operation_key)); + assert_eq!( + request.expected_item_revision, + ticket::ticket_item_revision(&snapshot) + ); + assert_eq!(request.expected_state, TicketWorkflowState::Ready); + assert_eq!(request.reason, "Resolved without repository changes"); + assert!(request.references.is_empty()); + assert!(request.author.is_none()); + } + } + + #[test] + fn legacy_ticket_adapters_surface_backend_authority_and_stale_state_rejections() { + for status in [403, 409] { + for close in [false, true] { + let client = decision_client(status, TicketWorkflowState::Ready); + let backend = WorkspaceHttpTicketBackend::new(client.clone()); + let result = if close { + backend.close(TicketIdOrSlug::from("T-718"), "Resolved".into()) + } else { + backend.set_workflow_state(TicketIdOrSlug::from("T-718"), legacy_change()) + }; + assert!( + result + .unwrap_err() + .to_string() + .contains(&format!("HTTP status {status}")) + ); + assert_eq!(client.requests.lock().unwrap().len(), 2); + } + } + } + + #[tokio::test] + async fn native_ticket_close_binds_subject_and_forwards_strong_closed_state_decision() { + let client = decision_client(200, TicketWorkflowState::Closed); + let tools = native_ticket_tools(client.clone(), TicketFeatureAccess::workspace_authoring()) + .unwrap(); + let descriptor = ticket_descriptor(&tools, "Ticket", "bound").unwrap(); + let close = descriptor + .operations + .iter() + .find(|op| op.name == "close") + .unwrap(); + assert!( + !close + .parameters + .iter() + .any(|p| p.name == "ticket" || p.name == "author" || p.name == "role") + ); + for field in [ + "operation_key", + "expected_item_revision", + "expected_state", + "resolution", + ] { + assert!( + close + .parameters + .iter() + .any(|p| p.name == field && p.required) + ); + } + assert!( + close + .parameters + .iter() + .any(|p| p.name == "references" && !p.required) + ); + let revisions = Arc::new(Mutex::new(TicketRevisionState::default())); + let resolver = TicketItemResolver { + tools: operation_map(tools), + permissions: None, + collection_route: "/tickets".into(), + revisions, + }; + let bound = resolver.resolve("T-718").unwrap(); + let before = bound.object.validator; + let other = resolver.resolve("T-719").unwrap().object.validator; + let mut input = close_input(); + input.as_object_mut().unwrap().remove("ticket"); + let arguments = input + .as_object() + .unwrap() + .iter() + .map(|(key, value)| (key.clone(), json_to_wip(value).unwrap())) + .collect(); + bound + .handler + .call( + "close", + &arguments, + WipCallContext { + execution: ToolExecutionContext::direct(), + security_context: "author".into(), + }, + ) + .await + .unwrap_or_else(|_| panic!("bound close should succeed")); + assert_ne!(before, resolver.resolve("T-718").unwrap().object.validator); + assert_eq!(other, resolver.resolve("T-719").unwrap().object.validator); + let requests = client.requests.lock().unwrap(); + assert_eq!(requests.len(), 1); + assert_eq!( + requests[0].path, + "/api/w/workspace/tickets/T-718/state-update" + ); + let body: Value = serde_json::from_str(requests[0].body.as_deref().unwrap()).unwrap(); + assert_eq!( + body, + json!({ + "operation_key": "decision-1", "expected_item_revision": "revision-1", + "expected_state": "planning", "state": "closed", + "reason": "No repository changes are needed", "references": [] + }) + ); + } + + #[tokio::test] + async fn native_ticket_close_preserves_backend_rejections_and_wip_permission_gate() { + for (status, code) in [ + (403, ProtocolErrorCode::PermissionDenied), + (409, ProtocolErrorCode::InvalidArguments), + ] { + let client = decision_client(status, TicketWorkflowState::Planning); + let tools = + native_ticket_tools(client.clone(), TicketFeatureAccess::workflow()).unwrap(); + let tool = tools + .iter() + .find(|tool| tool.name == "TicketClose") + .unwrap(); + let error = execute_native_ticket_tool( + tool, + &None, + close_input(), + WipCallContext { + execution: ToolExecutionContext::direct(), + security_context: "self-reported-author".into(), + }, + ) + .await + .expect_err("Backend rejection must be preserved"); + assert!( + matches!(error, WipOperationError::Protocol(ProtocolError { code: actual, .. }) if actual == code) + ); + assert_eq!(client.requests.lock().unwrap().len(), 1); + } + let client = decision_client(200, TicketWorkflowState::Closed); + let tools = native_ticket_tools(client.clone(), TicketFeatureAccess::workflow()).unwrap(); + let tool = tools + .iter() + .find(|tool| tool.name == "TicketClose") + .unwrap(); + let permissions = Some(ToolPermissionConfig { + default_action: ToolPermissionAction::Deny, + rules: Vec::new(), + }); + let error = execute_native_ticket_tool( + tool, + &permissions, + close_input(), + WipCallContext { + execution: ToolExecutionContext::direct(), + security_context: "author".into(), + }, + ) + .await + .expect_err("WIP permission denial must prevent dispatch"); + assert!(matches!( + error, + WipOperationError::Protocol(ProtocolError { + code: ProtocolErrorCode::PermissionDenied, + .. + }) + )); + assert!(client.requests.lock().unwrap().is_empty()); + } + + #[tokio::test] + async fn completion_accepts_mrless_decisions_and_forwards_cas_without_actor_claims() { + let client = decision_client(200, TicketWorkflowState::Done); + let (_, tool) = workspace_ticket_decision_definition( + client.clone(), + WorkspaceTicketDecisionKind::Complete, + )(); + let output = tool + .execute( + &decision_input().to_string(), + ToolExecutionContext::direct(), + ) + .await + .unwrap(); + let projected: Value = serde_json::from_str(output.content.as_deref().unwrap()).unwrap(); + assert_eq!(projected["meta"]["id"], "T-718"); + assert_eq!(projected["meta"]["workflow_state"], "done"); + let requests = client.requests.lock().unwrap(); + assert_eq!(requests.len(), 1); + assert_eq!(requests[0].path, "/api/w/workspace/tickets/T-718/complete"); + assert_eq!(requests[0].method, WorkspaceRequestMethod::Post); + let body: Value = serde_json::from_str(requests[0].body.as_deref().unwrap()).unwrap(); + assert_eq!( + body, + json!({ + "operation_key": "decision-1", "expected_item_revision": "revision-1", + "expected_state": "planning", "reason": "No repository changes are needed", "references": [] + }) + ); + } + + #[tokio::test] + async fn state_updates_forward_each_display_state_without_other_control_operations() { + for state in [ + "planning", + "ready", + "queued", + "in_progress", + "done", + "closed", + ] { + let client = decision_client(200, TicketWorkflowState::Planning); + let (_, tool) = workspace_ticket_decision_definition( + client.clone(), + WorkspaceTicketDecisionKind::UpdateState, + )(); + let mut input = decision_input(); + input["state"] = json!(state); + tool.execute(&input.to_string(), ToolExecutionContext::direct()) + .await + .unwrap(); + let requests = client.requests.lock().unwrap(); + assert_eq!(requests.len(), 1); + assert_eq!( + requests[0].path, + "/api/w/workspace/tickets/T-718/state-update" + ); + let mut expected = input.as_object().unwrap().clone(); + expected.remove("ticket"); + expected.insert("references".into(), json!([])); + let body: Value = serde_json::from_str(requests[0].body.as_deref().unwrap()).unwrap(); + assert_eq!(body, Value::Object(expected)); + } + } + + #[tokio::test] + async fn decision_inputs_require_reason_cas_and_reject_self_reported_authority_before_dispatch() + { + for kind in [ + WorkspaceTicketDecisionKind::Complete, + WorkspaceTicketDecisionKind::UpdateState, + WorkspaceTicketDecisionKind::Close, + ] { + let client = decision_client(200, TicketWorkflowState::Done); + let (meta, tool) = workspace_ticket_decision_definition(client.clone(), kind)(); + let mut valid = decision_input(); + if matches!(kind, WorkspaceTicketDecisionKind::UpdateState) { + valid["state"] = json!("ready"); + } + let reason_field = if matches!(kind, WorkspaceTicketDecisionKind::Close) { + let reason = valid.as_object_mut().unwrap().remove("reason").unwrap(); + valid["resolution"] = reason; + "resolution" + } else { + "reason" + }; + let validator = jsonschema::validator_for(&meta.input_schema).unwrap(); + assert!(validator.is_valid(&valid)); + for field in [ + "operation_key", + "expected_item_revision", + "expected_state", + reason_field, + ] { + let mut invalid = valid.clone(); + invalid.as_object_mut().unwrap().remove(field); + assert!(!validator.is_valid(&invalid)); + assert!( + tool.execute(&invalid.to_string(), ToolExecutionContext::direct()) + .await + .is_err() + ); + } + for field in [ + reason_field, + "operation_key", + "expected_item_revision", + "ticket", + ] { + let mut invalid = valid.clone(); + invalid[field] = json!(" "); + assert!( + tool.execute(&invalid.to_string(), ToolExecutionContext::direct()) + .await + .is_err() + ); + } + for field in [ + "role", + "author", + "merge_request_ids", + "requirement_approval_event_id", + ] { + let mut invalid = valid.clone(); + invalid[field] = json!("coder"); + assert!(!validator.is_valid(&invalid)); + assert!( + tool.execute(&invalid.to_string(), ToolExecutionContext::direct()) + .await + .is_err() + ); + } + let mut invalid = valid.clone(); + invalid["expected_state"] = json!("approved"); + assert!(!validator.is_valid(&invalid)); + assert!( + tool.execute(&invalid.to_string(), ToolExecutionContext::direct()) + .await + .is_err() + ); + assert!(client.requests.lock().unwrap().is_empty()); + } + } + + #[tokio::test] + async fn decision_tools_preserve_backend_assignment_and_cas_rejections() { + for (status, code) in [ + (403, ProtocolErrorCode::PermissionDenied), + (409, ProtocolErrorCode::InvalidArguments), + ] { + let client = decision_client(status, TicketWorkflowState::Planning); + let tools = + native_ticket_tools(client.clone(), TicketFeatureAccess::work_report()).unwrap(); + let tool = tools + .iter() + .find(|tool| tool.name == "CompleteTicket") + .unwrap(); + let error = execute_native_ticket_tool( + tool, + &None, + decision_input(), + WipCallContext { + execution: ToolExecutionContext::direct(), + security_context: "self-reported-coder".into(), + }, + ) + .await + .expect_err("Backend rejection must be preserved"); + assert!( + matches!(error, WipOperationError::Protocol(ProtocolError { code: actual, .. }) if actual == code) + ); + assert_eq!(client.requests.lock().unwrap().len(), 1); + } + } + + #[tokio::test] + async fn native_ticket_completion_binds_subject_and_stales_only_its_ticket_not_mrs() { + let client = decision_client(200, TicketWorkflowState::Done); + let revisions = Arc::new(Mutex::new(TicketRevisionState::default())); + let tools = + native_ticket_tools(client.clone(), TicketFeatureAccess::work_report()).unwrap(); + let descriptor = ticket_descriptor(&tools, "Ticket", "bound").unwrap(); + let complete = descriptor + .operations + .iter() + .find(|op| op.name == "complete") + .unwrap(); + assert!(!complete.parameters.iter().any(|p| p.name == "ticket")); + for field in [ + "operation_key", + "expected_item_revision", + "expected_state", + "reason", + ] { + assert!( + complete + .parameters + .iter() + .any(|p| p.name == field && p.required) + ); + } + let resolver = TicketItemResolver { + tools: operation_map(tools), + permissions: None, + collection_route: "/tickets".into(), + revisions: revisions.clone(), + }; + let bound = resolver.resolve("T-718").unwrap(); + let before = bound.object.validator; + let other = resolver.resolve("T-719").unwrap().object.validator; + let mut input = decision_input(); + input.as_object_mut().unwrap().remove("ticket"); + let arguments = input + .as_object() + .unwrap() + .iter() + .map(|(key, value)| (key.clone(), json_to_wip(value).unwrap())) + .collect(); + bound + .handler + .call( + "complete", + &arguments, + WipCallContext { + execution: ToolExecutionContext::direct(), + security_context: "coder".into(), + }, + ) + .await + .unwrap_or_else(|_| panic!("bound completion should succeed")); + assert_ne!(before, resolver.resolve("T-718").unwrap().object.validator); + assert_eq!(other, resolver.resolve("T-719").unwrap().object.validator); + assert_eq!( + client.requests.lock().unwrap()[0].path, + "/api/w/workspace/tickets/T-718/complete" + ); + } + #[test] fn native_ticket_projection_matches_role_inventory_and_binds_item_identity() { let client: Arc = Arc::new( @@ -2038,9 +3030,11 @@ mod tests { "comment", "mark_ready", "queue", + "transition", "close", "record_relation", "remove_relation", + "complete", ] ); let item = ticket_descriptor( @@ -2089,6 +3083,7 @@ mod tests { "remove_relation", "record_orchestration_plan", "query_orchestration_plans", + "complete", ] ); assert_eq!( @@ -2362,13 +3357,13 @@ mod tests { assert_eq!(show.name, "ShowTicket"); assert!(show.input_schema["properties"]["event_limit"].is_object()); let tool_names = TicketFeatureAccess::workspace_authoring().tool_names(); - assert_eq!(tool_names.len(), 10); + assert_eq!(tool_names.len(), 12); assert!( tool_names.len() < 13, "authoring catalog must stay below the prior broad catalog" ); let workflow_names = TicketFeatureAccess::workflow().tool_names(); - assert_eq!(workflow_names.len(), 10); + assert_eq!(workflow_names.len(), 11); assert!( workflow_names.len() < 12, "workflow catalog must stay below the prior broad catalog" @@ -2440,7 +3435,7 @@ mod tests { assert!(workspace_tools.contains(&"TicketCreate")); assert!(workspace_tools.contains(&"TicketEditItem")); assert!(workspace_tools.contains(&"TicketQueue")); - assert!(!workspace_tools.contains(&"TicketWorkflowState")); + assert!(workspace_tools.contains(&"TicketWorkflowState")); let orchestration = workspace_feature(TicketFeatureAccess::workflow()); let orchestration_descriptor = orchestration.descriptor(); @@ -2464,7 +3459,8 @@ mod tests { .map(|tool| tool.name.as_str()) .collect::>(); assert!(work_report_tools.contains(&"TicketComment")); - assert!(!work_report_tools.contains(&"TicketWorkflowState")); + assert!(work_report_tools.contains(&"CompleteTicket")); + assert!(work_report_tools.contains(&"TicketWorkflowState")); let review = workspace_feature(TicketFeatureAccess::review()); let review_descriptor = review.descriptor(); @@ -2474,6 +3470,7 @@ mod tests { .map(|tool| tool.name.as_str()) .collect::>(); assert!(!review_tools.contains(&"TicketWorkflowState")); + assert!(!review_tools.contains(&"CompleteTicket")); } #[test] @@ -2522,7 +3519,7 @@ mod tests { assert!(installed.iter().any(|tool| *tool == "TicketQueue")); assert!(installed.iter().any(|tool| *tool == "TicketMarkReady")); assert!(!installed.iter().any(|tool| *tool == "TicketIntakeReady")); - assert!(!installed.iter().any(|tool| *tool == "TicketWorkflowState")); + assert!(installed.iter().any(|tool| *tool == "TicketWorkflowState")); assert!( !installed .iter() diff --git a/crates/worker/src/wip.rs b/crates/worker/src/wip.rs index c2043edca..54e423c7b 100644 --- a/crates/worker/src/wip.rs +++ b/crates/worker/src/wip.rs @@ -6119,17 +6119,6 @@ mod tests { }) .to_string(), }, - crate::worker::WorkspaceResponse { - status: 200, - body: json!({ - "operation_id": "complete-ticket-1", - "ticket_id": "ticket-internal", - "item_revision": "ticket-rev-1", - "merge_request_ids": ["MR-1"], - "requirement_approval_event_id": "review-1" - }) - .to_string(), - }, ])); let orchestrator_config = MergeRequestFeatureConfig { readiness_check: true, @@ -6202,51 +6191,22 @@ mod tests { .inspect("yoi.merge-request/collection/v1".into(), false) .await .unwrap(); - let before_ticket_completion = orchestrator - .host - .projection(&item) - .unwrap() - .object - .validator; - orchestrator + let rejected = orchestrator .call( collection.into(), "yoi.merge-request/collection/v1".into(), "complete_ticket".into(), - json!({ - "ticket": "T-685", - "operation_id": "complete-ticket-1", - "item_revision": "ticket-rev-1", - "merge_request_ids": ["MR-1"], - "requirement_approval_event_id": "review-1" - }), - ToolExecutionContext::direct(), - ) - .await - .unwrap(); - assert_ne!( - before_ticket_completion, - orchestrator - .host - .projection(&item) - .unwrap() - .object - .validator, - "Ticket completion must stale every Merge Request in its exact result set" - ); - let stale = orchestrator - .call( - item, - "yoi.merge-request/item/v1".into(), - "check_readiness".into(), json!({}), ToolExecutionContext::direct(), ) .await - .expect_err("affected Merge Request observation must be stale"); - assert!(stale.to_string().contains("ValidatorMismatch")); + .expect_err("Ticket completion is not a Merge Request operation"); + assert!( + rejected.to_string().contains("operation is not declared"), + "{rejected}" + ); let orchestrator_requests = orchestrator_client.requests(); - assert_eq!(orchestrator_requests.len(), 3); + assert_eq!(orchestrator_requests.len(), 2); assert_eq!( orchestrator_requests[0].path, "/api/w/workspace/merge-requests/MR-1/readiness" @@ -6260,10 +6220,6 @@ mod tests { assert_eq!(completion_body["approval_event_id"], "review-1"); assert_eq!(completion_body["target_ref_before"], "target-1"); assert_eq!(completion_body["target_ref_after"], "result-1"); - assert_eq!( - orchestrator_requests[2].path, - "/api/w/workspace/tickets/T-685/complete" - ); } #[tokio::test] diff --git a/crates/workspace-server/src/authority.rs b/crates/workspace-server/src/authority.rs index 5c875430b..818d71a21 100644 --- a/crates/workspace-server/src/authority.rs +++ b/crates/workspace-server/src/authority.rs @@ -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::>(); 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")); diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index 63f1b7934..94c8b6635 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -6441,6 +6441,9 @@ fn generated_workspace_contract_router(service: ServerApiContractService) -> Rou .merge(server_api::server_api_axum::ticket_workflow_state_set( service.clone(), )) + .merge(server_api::server_api_axum::ticket_state_update( + service.clone(), + )) .merge(server_api::server_api_axum::ticket_mark_ready_record( service.clone(), )) @@ -10229,8 +10232,7 @@ impl server_api::ServerApi for ServerApiContractService { workspace_id: String, id: String, request: server_api::CompleteTicketRequest, - ) -> std::result::Result - { + ) -> std::result::Result { let Json(response) = scoped_complete_ticket( State(self.workspace_api()?.clone()), contract_request_headers(&context)?, @@ -10239,7 +10241,25 @@ impl server_api::ServerApi for ServerApiContractService { ) .await .map_err(ApiError::into_repository_api_error)?; - Ok(response) + Ok(server_api::TicketRecord(response)) + } + + async fn ticket_state_update( + &self, + context: server_api::ServerRequestContext, + workspace_id: String, + id: String, + request: server_api::TicketStateUpdateRequest, + ) -> std::result::Result { + let Json(response) = scoped_ticket_state_update( + State(self.workspace_api()?.clone()), + contract_request_headers(&context)?, + AxumPath((workspace_id, id)), + Json(request.0), + ) + .await + .map_err(ApiError::into_repository_api_error)?; + Ok(server_api::TicketRecord(response)) } async fn ticket_close_record( @@ -10455,12 +10475,14 @@ impl server_api::ServerApi for ServerApiContractService { async fn ticket_state_transition( &self, + context: server_api::ServerRequestContext, workspace_id: String, id: String, request: server_api::BrowserTransitionTicketStateRequest, ) -> std::result::Result { let Json(response) = scoped_transition_ticket_state( State(self.workspace_api()?.clone()), + contract_request_headers(&context)?, AxumPath(ScopedRecordPath { workspace_id, id }), Json(request), ) @@ -10471,6 +10493,7 @@ impl server_api::ServerApi for ServerApiContractService { async fn ticket_ready( &self, + context: server_api::ServerRequestContext, workspace_id: String, id: String, request: server_api::TicketMarkReadyRequest, @@ -10482,6 +10505,7 @@ impl server_api::ServerApi for ServerApiContractService { }; let Json(response) = scoped_mark_ticket_ready_from_browser( State(self.workspace_api()?.clone()), + contract_request_headers(&context)?, AxumPath(ScopedRecordPath { workspace_id, id }), Json(request), ) @@ -10508,12 +10532,14 @@ impl server_api::ServerApi for ServerApiContractService { async fn ticket_queue( &self, + context: server_api::ServerRequestContext, workspace_id: String, id: String, request: server_api::BrowserQueueTicketRequest, ) -> std::result::Result { let Json(response) = scoped_queue_ticket( State(self.workspace_api()?.clone()), + contract_request_headers(&context)?, AxumPath(ScopedRecordPath { workspace_id, id }), Json(request), ) @@ -10524,12 +10550,14 @@ impl server_api::ServerApi for ServerApiContractService { async fn ticket_close( &self, + context: server_api::ServerRequestContext, workspace_id: String, id: String, request: server_api::BrowserCloseTicketRequest, ) -> std::result::Result { let Json(response) = scoped_close_ticket( State(self.workspace_api()?.clone()), + contract_request_headers(&context)?, AxumPath(ScopedRecordPath { workspace_id, id }), Json(request), ) @@ -13707,12 +13735,10 @@ impl ticket::TicketTargetAuthority for WorkspaceTicketTargetAuthority { .map(str::trim) .filter(|value| !value.is_empty()) .or(repository.default_ref.as_deref()) - .ok_or_else(|| { - ticket::TicketError::MissingTargetSelector(repository.repository_key.clone()) - })?; + .map(str::to_owned); Ok(ticket::ResolvedTicketTarget { repository_key: repository.repository_key, - ref_selector: selector.to_owned(), + ref_selector: selector, access: TicketTargetAccess::ReadOnly, }) } @@ -13787,47 +13813,46 @@ async fn scoped_edit_ticket_item( browser_ticket_detail(&api, &path.id) } -async fn scoped_transition_ticket_state( - State(api): State, - AxumPath(path): AxumPath, - Json(request): Json, -) -> ApiResult> { - validate_workspace_scope(&api, &path.workspace_id)?; - let state = match request.state { +fn browser_ticket_workflow_state( + state: server_api::BrowserTicketWorkflowState, +) -> TicketWorkflowState { + match state { server_api::BrowserTicketWorkflowState::Planning => TicketWorkflowState::Planning, server_api::BrowserTicketWorkflowState::Ready => TicketWorkflowState::Ready, server_api::BrowserTicketWorkflowState::Queued => TicketWorkflowState::Queued, server_api::BrowserTicketWorkflowState::Inprogress => TicketWorkflowState::InProgress, server_api::BrowserTicketWorkflowState::Done => TicketWorkflowState::Done, server_api::BrowserTicketWorkflowState::Closed => TicketWorkflowState::Closed, + } +} + +async fn scoped_transition_ticket_state( + State(api): State, + headers: HeaderMap, + AxumPath(path): AxumPath, + Json(request): Json, +) -> ApiResult> { + validate_workspace_scope(&api, &path.workspace_id)?; + let state = browser_ticket_workflow_state(request.state); + let reason = match request.body.filter(|body| !body.is_empty()) { + Some(body) => format!("{}\n\n{body}", request.reason), + None => request.reason, }; - if state == TicketWorkflowState::Done { - return Err(Error::TicketAssignmentConflict( - "done is guarded by CompleteMergeRequest with an approved exact source ref and operation_id".to_string(), - ).into()); - } - let current = api.authority.ticket(&path.id)?; - if state == TicketWorkflowState::InProgress - && current.state != TicketWorkflowState::InProgress.as_str() - { - return Err(Error::TicketAssignmentConflict( - "generic Ticket state mutation cannot enter inprogress; use Queue acceptance or atomic ready-state Coder assignment" - .to_string(), - ) - .into()); - } - let mut change = TicketStateChange::new( - current.state, - state.as_str(), - request - .reason - .unwrap_or_else(|| "state changed from Web Ticket API".to_owned()), - request.body.unwrap_or_default(), - ); - change.author = request.author; - browser_ticket_backend(&api)? - .set_workflow_state(TicketIdOrSlug::Id(path.id.clone()), change) - .map_err(Error::from)?; + let _ = scoped_ticket_state_update( + State(api.clone()), + headers, + AxumPath((path.workspace_id.clone(), path.id.clone())), + Json(ticket::TicketStateUpdate { + state, + operation_key: request.operation_key, + expected_item_revision: request.expected_item_revision, + expected_state: browser_ticket_workflow_state(request.expected_state), + reason, + references: Vec::new(), + author: Some("workspace-user".into()), + }), + ) + .await?; browser_ticket_detail(&api, &path.id) } @@ -13855,26 +13880,24 @@ async fn scoped_append_ticket_event( async fn scoped_mark_ticket_ready_from_browser( State(api): State, + headers: HeaderMap, AxumPath(path): AxumPath, Json(request): Json, ) -> ApiResult> { validate_workspace_scope(&api, &path.workspace_id)?; - browser_ticket_backend(&api)? - .mark_ready( - TicketIdOrSlug::Id(path.id.clone()), - ticket::TicketMarkReady { - operation_key: request.operation_key, - reason: request.reason, - author: Some("web".to_owned()), - intake_summary: None, - }, - ) - .map_err(Error::from)?; + let _ = scoped_mark_ticket_ready( + State(api.clone()), + AxumPath((path.workspace_id.clone(), path.id.clone())), + headers, + Json(request), + ) + .await?; browser_ticket_detail(&api, &path.id) } async fn scoped_queue_ticket( State(api): State, + headers: HeaderMap, AxumPath(path): AxumPath, Json(_request): Json, ) -> ApiResult> { @@ -13882,7 +13905,7 @@ async fn scoped_queue_ticket( let result = execute_ticket_rest_operation( &api, &path.workspace_id, - HeaderMap::new(), + headers, TicketBackendOperation::QueueReady { id: TicketIdOrSlug::Id(path.id.clone()), queued_by: "workspace-web".to_string(), @@ -13897,16 +13920,26 @@ async fn scoped_queue_ticket( async fn scoped_close_ticket( State(api): State, + headers: HeaderMap, AxumPath(path): AxumPath, Json(request): Json, ) -> ApiResult> { validate_workspace_scope(&api, &path.workspace_id)?; - browser_ticket_backend(&api)? - .close( - TicketIdOrSlug::Id(path.id.clone()), - MarkdownText::new(request.resolution), - ) - .map_err(Error::from)?; + let _ = scoped_ticket_state_update( + State(api.clone()), + headers, + AxumPath((path.workspace_id.clone(), path.id.clone())), + Json(ticket::TicketStateUpdate { + state: TicketWorkflowState::Closed, + operation_key: request.operation_key, + expected_item_revision: request.expected_item_revision, + expected_state: browser_ticket_workflow_state(request.expected_state), + reason: request.resolution, + references: Vec::new(), + author: Some("workspace-user".into()), + }), + ) + .await?; browser_ticket_detail(&api, &path.id) } @@ -13919,28 +13952,6 @@ fn generic_ticket_state_change(operation: &TicketBackendOperation) -> Option<&Ti } } -fn reject_unguarded_ticket_start(operation: &TicketBackendOperation) -> Result<()> { - if generic_ticket_state_change(operation) - .is_some_and(|change| change.to == "inprogress" && change.from != "inprogress") - { - return Err(Error::TicketAssignmentConflict( - "inprogress is guarded by Queue acceptance or atomic ready-state Coder assignment" - .to_string(), - )); - } - Ok(()) -} - -fn reject_unguarded_ticket_completion(operation: &TicketBackendOperation) -> Result<()> { - reject_unguarded_ticket_start(operation)?; - if generic_ticket_state_change(operation).is_some_and(|change| change.to == "done") { - return Err(Error::TicketAssignmentConflict( - "done is guarded by CompleteMergeRequest with an approved exact source ref and operation_id".to_string(), - )); - } - Ok(()) -} - fn validate_ticket_target_repository_keys( api: &WorkspaceApi, workspace_id: &str, @@ -14106,9 +14117,26 @@ async fn execute_ticket_rest_operation( // calls carry source headers; when either header is present, the complete pair is required // and authenticated before source attribution is attached. let source = optional_worker_mutation_source(api, workspace_id, &headers)?; - reject_unguarded_ticket_completion(&operation)?; validate_ticket_repository_operation(api, &operation)?; let before = target.as_ref().and_then(|id| backend.show(id.clone()).ok()); + if let Some(source) = source.as_ref() + && (generic_ticket_state_change(&operation).is_some() + || matches!( + operation, + TicketBackendOperation::Close { .. } + | TicketBackendOperation::QueueReady { .. } + | TicketBackendOperation::MarkReady { .. } + )) + { + let context = worker_ticket_source_context(api, workspace_id, source, before.as_ref()); + let is_orchestrator = + find_workspace_orchestrator(api).is_some_and(|worker| worker.worker == *source); + if context.actor_role != "coder" && !is_orchestrator { + return Err(Error::WorkspacePermissionDenied( + "Ticket state decisions require the assigned Worker or registered Workspace Orchestrator".into(), + ).into()); + } + } let mut ticket_item_check = source .as_ref() .cloned() @@ -14125,41 +14153,11 @@ async fn execute_ticket_rest_operation( .to_string(), ) })?; - let dependency_check = backend - .dependency_check(TicketIdOrSlug::Id(ticket.meta.id.clone())) - .map_err(Error::from)?; - if !dependency_check.queue_guard.can_queue_for_orchestrator { - let reason = dependency_check - .queue_guard - .blocked_reason - .or(dependency_check.queue_guard.reason) - .unwrap_or_else(|| "Queue dependency validation failed".to_string()); - return Err(Error::TicketAssignmentConflict(reason).into()); - } - let candidates = dependency_check.queue_tickets; let mut assignment_ids = BTreeMap::new(); - for ticket_id in candidates { - if api - .store - .get_active_ticket_role_assignment( - workspace_id, - &ticket_id, - TicketAssignmentRole::Coder, - )? - .is_some() - { - return Err(Error::TicketAssignmentConflict(format!( - "Queue rejects Ticket {ticket_id} while a Coder assignment is active" - )) - .into()); - } - let assignment = active_orchestrator_assignment(api, workspace_id, &ticket_id)? - .ok_or_else(|| { - Error::TicketAssignmentConflict(format!( - "Queue requires role=orchestrator assignment for Ticket {ticket_id}" - )) - })?; - assignment_ids.insert(ticket_id, assignment.assignment_id); + if let Some(assignment) = + active_orchestrator_assignment(api, workspace_id, &ticket.meta.id)? + { + assignment_ids.insert(ticket.meta.id.clone(), assignment.assignment_id); } let assignment_json = serde_json::to_string(&assignment_ids).map_err(|error| { Error::Config(format!("failed to encode Queue assignment fence: {error}")) @@ -14173,11 +14171,10 @@ async fn execute_ticket_rest_operation( .iter() .map(|byte| format!("{byte:02x}")) .collect::(); + if !assignment_ids.is_empty() { + event_attributes.insert("queue_orchestrator_assignments".into(), assignment_json); + } event_attributes.extend([ - ( - "queue_orchestrator_assignments".to_string(), - assignment_json, - ), ( "routing_principal".to_string(), "workspace-orchestrator".to_string(), @@ -14196,6 +14193,55 @@ async fn execute_ticket_rest_operation( backend = backend.with_event_attributes(event_attributes); } + // Older transport-shaped state/close calls are adapters to the same Ticket + // decision boundary, never alternative completion authorities. + if let Some(before) = before.as_ref() { + let decision = match &operation { + TicketBackendOperation::SetWorkflowState { change, .. } + | TicketBackendOperation::AddStateChanged { change, .. } => Some(( + TicketWorkflowState::parse(&change.to) + .ok_or_else(|| Error::InvalidInput("invalid Ticket state".into()))?, + TicketWorkflowState::parse(&change.from) + .ok_or_else(|| Error::InvalidInput("invalid expected Ticket state".into()))?, + if change.body.as_str().is_empty() { + change.reason.clone() + } else { + format!("{}\n\n{}", change.reason, change.body.as_str()) + }, + change.author.clone(), + change.references.clone(), + )), + TicketBackendOperation::Close { resolution, .. } => Some(( + TicketWorkflowState::Closed, + before.meta.workflow_state, + resolution.as_str().to_string(), + source + .as_ref() + .map(|source| format!("worker:{}/{}", source.runtime_id, source.worker_id)) + .or_else(|| Some("workspace-user".into())), + Vec::new(), + )), + _ => None, + }; + if let Some((state, expected_state, reason, author, references)) = decision { + backend + .update_state( + &before.meta.id, + ticket::TicketStateUpdate { + operation_key: new_id("tdecision"), + expected_item_revision: ticket::ticket_item_revision(before), + expected_state, + state, + reason, + author, + references, + }, + ) + .map_err(Error::from)?; + return Ok(TicketBackendOperationResult::Unit); + } + } + let (result, created_ticket) = match operation { TicketBackendOperation::Create { input } => { let (reference, ticket) = backend.create_with_snapshot(input).map_err(Error::from)?; @@ -14387,15 +14433,6 @@ async fn scoped_create_ticket_record( headers: HeaderMap, Json(input): Json, ) -> ApiResult> { - if input - .workflow_state - .is_some_and(|state| state != TicketWorkflowState::Planning) - { - return Err(settings_bad_request( - "ticket_create_state_bypass", - "Ticket creation must start in planning; use guarded workflow operations for later states", - )); - } let result = execute_ticket_rest_operation( &api, &path.workspace_id, @@ -14474,19 +14511,16 @@ async fn scoped_add_ticket_state_change( State(api): State, AxumPath((workspace_id, id)): AxumPath<(String, String)>, headers: HeaderMap, - Json(change): Json, + Json(request): Json, ) -> ApiResult { - let result = execute_ticket_rest_operation( - &api, - &workspace_id, + let _ = scoped_ticket_state_update( + State(api), headers, - TicketBackendOperation::AddStateChanged { - id: TicketIdOrSlug::Query(id), - change, - }, + AxumPath((workspace_id, id)), + Json(request), ) .await?; - ticket_rest_unit(result) + Ok(StatusCode::NO_CONTENT) } async fn scoped_add_ticket_intake_summary( @@ -14521,39 +14555,35 @@ async fn scoped_set_ticket_state_field( State(api): State, AxumPath((workspace_id, id, field)): AxumPath<(String, String, String)>, headers: HeaderMap, - Json(change): Json, + Json(request): Json, ) -> ApiResult { - let result = execute_ticket_rest_operation( - &api, - &workspace_id, + if field != "state" { + return Err(Error::InvalidInput("unsupported Ticket state field".into()).into()); + } + let _ = scoped_ticket_state_update( + State(api), headers, - TicketBackendOperation::SetStateField { - id: TicketIdOrSlug::Query(id), - field, - change, - }, + AxumPath((workspace_id, id)), + Json(request), ) .await?; - ticket_rest_unit(result) + Ok(StatusCode::NO_CONTENT) } async fn scoped_set_ticket_workflow_state( State(api): State, AxumPath((workspace_id, id)): AxumPath<(String, String)>, headers: HeaderMap, - Json(change): Json, + Json(request): Json, ) -> ApiResult { - let result = execute_ticket_rest_operation( - &api, - &workspace_id, + let _ = scoped_ticket_state_update( + State(api), headers, - TicketBackendOperation::SetWorkflowState { - id: TicketIdOrSlug::Query(id), - change, - }, + AxumPath((workspace_id, id)), + Json(request), ) .await?; - ticket_rest_unit(result) + Ok(StatusCode::NO_CONTENT) } async fn scoped_mark_ticket_ready( @@ -15162,20 +15192,6 @@ fn public_merge_event(event: merge_request::MergeEvent) -> server_api::MergeEven } } -fn public_ticket_completion_event( - event: merge_request::TicketCompletionEvent, -) -> server_api::TicketCompletionEvent { - server_api::TicketCompletionEvent { - operation_id: event.operation_id, - ticket_id: event.ticket_id, - item_revision: event.item_revision, - merge_request_ids: event.merge_request_ids, - requirement_approval_event_id: event.requirement_approval_event_id, - completed_by: public_merge_request_worker_identity(event.completed_by), - created_at: event.created_at.to_rfc3339(), - } -} - fn public_merge_strategy(strategy: merge_request::MergeStrategy) -> server_api::MergeStrategy { match strategy { merge_request::MergeStrategy::FastForward => server_api::MergeStrategy::FastForward, @@ -16154,38 +16170,74 @@ async fn scoped_complete_ticket( headers: HeaderMap, AxumPath((workspace_id, ticket_id)): AxumPath<(String, String)>, Json(input): Json, -) -> ApiResult> { - let workspace_id = parse_workspace_id(&workspace_id)?; +) -> ApiResult> { + scoped_ticket_state_update( + State(api), + headers, + AxumPath((workspace_id, ticket_id)), + Json(ticket::TicketStateUpdate { + operation_key: input.operation_key, + expected_item_revision: input.expected_item_revision, + expected_state: input.expected_state, + state: TicketWorkflowState::Done, + reason: input.reason, + references: input.references, + author: None, + }), + ) + .await +} + +async fn scoped_ticket_state_update( + State(api): State, + headers: HeaderMap, + AxumPath((workspace_id, ticket_id)): AxumPath<(String, String)>, + Json(mut request): Json, +) -> ApiResult> { + validate_workspace_scope(&api, &workspace_id)?; let ticket_id = resolve_workspace_ticket_reference(&api, &workspace_id, &ticket_id)?; - require_workspace_access(&workspace_id, &api)?; - let source = authenticate_worker_mutation_source(&api, &workspace_id, &headers)?; - require_online_workspace_orchestrator_source(&api, &source)?; - // No active assignment is expected after completion. The store checks an - // exact recorded-operation replay before live authority, including its actor - // and evidence fingerprint; an empty identity cannot authorize new completion. - let assignment_id = api - .store - .get_active_ticket_worker_assignment(&workspace_id, &ticket_id)? - .map(|assignment| assignment.assignment_id) - .unwrap_or_default(); - let event = merge_request_store(&api, &workspace_id)?.complete_ticket( - merge_request::CompleteTicket { - ticket_id, - operation_id: input.operation_id, - item_revision: input.item_revision, - merge_request_ids: input.merge_request_ids, - requirement_approval_event_id: input.requirement_approval_event_id, - auth: merge_request::MergeRequestAuth { - workspace_id, - repository_id: String::new(), - runtime_id: source.runtime_id, - worker_id: source.worker_id, - assignment_id, - }, - now: Utc::now(), - }, - )?; - Ok(Json(public_ticket_completion_event(event))) + let mut backend = browser_ticket_backend(&api)?; + let before = backend + .show(TicketIdOrSlug::Id(ticket_id.clone())) + .map_err(Error::from)?; + if let Some(source) = optional_worker_mutation_source(&api, &workspace_id, &headers)? { + let author = format!("worker:{}/{}", source.runtime_id, source.worker_id); + let replay_event = before.events.iter().find(|event| { + event.author.as_deref() == Some(author.as_str()) + && event.attributes.get("operation_key") == Some(&request.operation_key) + && event.attributes.get("source_runtime_id") == Some(&source.runtime_id) + && event.attributes.get("source_worker_id") == Some(&source.worker_id) + }); + let replay = replay_event.is_some(); + let context = worker_ticket_source_context(&api, &workspace_id, &source, Some(&before)); + let is_orchestrator = + find_workspace_orchestrator(&api).is_some_and(|worker| worker.worker == source); + if !replay && context.actor_role != "coder" && !is_orchestrator { + return Err(Error::WorkspacePermissionDenied( + "Ticket state decisions require the assigned Worker or registered Workspace Orchestrator".into(), + ).into()); + } + request.author = Some(author); + let attributes = if let Some(event) = replay_event { + // Replay authenticates the same actor, but retains the receipt's + // original work identity. Completion/reopen never revives that grant. + event + .attributes + .iter() + .filter(|(key, _)| key.starts_with("source_")) + .map(|(key, value)| (key.clone(), value.clone())) + .collect() + } else { + context.attributes("state_update") + }; + backend = backend.with_event_attributes(attributes); + } else { + request.author = Some("workspace-user".into()); + } + let ticket = backend + .update_state(&ticket_id, request) + .map_err(Error::from)?; + Ok(Json(ticket)) } fn reject_non_browser_reopen_auth(headers: &HeaderMap) -> Result<()> { @@ -16199,19 +16251,24 @@ async fn scoped_close_ticket_record( State(api): State, AxumPath((workspace_id, id)): AxumPath<(String, String)>, headers: HeaderMap, - Json(resolution): Json, + Json(request): Json, ) -> ApiResult { - let result = execute_ticket_rest_operation( - &api, - &workspace_id, + let _ = scoped_ticket_state_update( + State(api), headers, - TicketBackendOperation::Close { - id: TicketIdOrSlug::Query(id), - resolution, - }, + AxumPath((workspace_id, id)), + Json(ticket::TicketStateUpdate { + state: TicketWorkflowState::Closed, + operation_key: request.operation_key, + expected_item_revision: request.expected_item_revision, + expected_state: request.expected_state, + reason: request.reason, + references: request.references, + author: request.author, + }), ) .await?; - ticket_rest_unit(result) + Ok(StatusCode::NO_CONTENT) } async fn scoped_record_ticket_relation( @@ -31909,6 +31966,7 @@ impl From for ApiError { ticket::TicketError::Locked { .. } => "ticket_locked", ticket::TicketError::Conflict(_) | ticket::TicketError::StaleWorkflowState { .. } + | ticket::TicketError::StaleItemRevision { .. } | ticket::TicketError::InvalidWorkflowTransition { .. } | ticket::TicketError::BlockingRelations(_) | ticket::TicketError::OperationFingerprintMismatch { .. } => "ticket_conflict", @@ -32025,7 +32083,10 @@ fn api_error_status(error: &Error) -> StatusCode { Error::Ticket( ticket::TicketError::Ambiguous { .. } | ticket::TicketError::Locked { .. } - | ticket::TicketError::Conflict(_), + | ticket::TicketError::Conflict(_) + | ticket::TicketError::StaleItemRevision { .. } + | ticket::TicketError::StaleWorkflowState { .. } + | ticket::TicketError::OperationFingerprintMismatch { .. }, ) => StatusCode::CONFLICT, Error::Ticket( ticket::TicketError::InvalidPathComponent(_) @@ -35664,43 +35725,6 @@ mod tests { assert!(reject_non_browser_reopen_auth(&HeaderMap::new()).is_ok()); } - #[test] - fn generic_worker_state_change_cannot_enter_inprogress() { - for from in ["planning", "ready", "queued"] { - let change = || { - TicketStateChange::new( - from, - "inprogress", - "worker-tool", - "bypass assignment-aware start", - ) - }; - let operations = [ - TicketBackendOperation::SetWorkflowState { - id: TicketIdOrSlug::Query("T1".to_string()), - change: change(), - }, - TicketBackendOperation::SetStateField { - id: TicketIdOrSlug::Query("T1".to_string()), - field: "state".to_string(), - change: change(), - }, - TicketBackendOperation::AddStateChanged { - id: TicketIdOrSlug::Query("T1".to_string()), - change: change(), - }, - ]; - for operation in operations { - let error = reject_unguarded_ticket_completion(&operation).unwrap_err(); - assert!( - error - .to_string() - .contains("atomic ready-state Coder assignment") - ); - } - } - } - #[test] fn feature_completion_api_conversions_preserve_nonce_and_context_for_aba_requests() { for nonce in ["first", "second", "first"] { @@ -35728,36 +35752,6 @@ mod tests { } } - #[test] - fn flow_or_generic_worker_state_change_is_not_ticket_completion_authority() { - let change = || { - TicketStateChange::new( - "inprogress", - "done", - "flow reached terminal state", - "terminal flow state", - ) - }; - for operation in [ - TicketBackendOperation::SetWorkflowState { - id: TicketIdOrSlug::Query("T1".to_string()), - change: change(), - }, - TicketBackendOperation::SetStateField { - id: TicketIdOrSlug::Query("T1".to_string()), - field: "state".to_string(), - change: change(), - }, - TicketBackendOperation::AddStateChanged { - id: TicketIdOrSlug::Query("T1".to_string()), - change: change(), - }, - ] { - let error = reject_unguarded_ticket_completion(&operation).unwrap_err(); - assert!(error.to_string().contains("CompleteMergeRequest")); - } - } - #[test] fn failed_api_log_is_structured_and_omits_query_values() { let uri = "/api/w/workspace/tickets?access_token=secret" @@ -46747,7 +46741,7 @@ mod tests { } #[tokio::test] - async fn mark_ready_accepts_registered_remote_target_and_closes_lifecycle_bypasses() { + async fn progress_and_target_editing_accept_registered_remote_assets_without_git() { let dir = tempfile::tempdir().unwrap(); let api = test_api_with_remote_repository(dir.path()).await; let backend = browser_ticket_backend(&api).unwrap(); @@ -46793,11 +46787,11 @@ mod tests { .count(), 1 ); - assert!(matches!( - backend.edit_item( - TicketIdOrSlug::Id(ticket_ref.id.clone()), - ticket::TicketItemEdit { - targets: Some(ticket::TicketTargetsEdit::Set { + let edited = backend + .edit_item( + ticket_ref.id.clone().into(), + TicketItemEdit { + targets: Some(TicketTargetsEdit::Set { targets: vec![test_ticket_target( "test-repository", "other", @@ -46806,20 +46800,67 @@ mod tests { }), ..Default::default() }, - ), - Err(ticket::TicketError::Conflict(_)) - )); - + ) + .unwrap(); + assert_eq!(edited.meta.workflow_state, TicketWorkflowState::Ready); + assert_eq!( + edited.meta.targets[0].ref_selector.as_deref(), + Some("other") + ); let mut missing = ticket::NewTicket::new("Missing target"); set_test_ticket_target(&mut missing, "unknown", "develop"); assert!(backend.create(missing).is_err()); - assert!(matches!( - backend.set_workflow_state( - TicketIdOrSlug::Id(ticket_ref.id), - TicketStateChange::new("ready", "queued", "bypass", "must use TicketQueue",), - ), - Err(ticket::TicketError::InvalidWorkflowTransition { .. }) - )); + backend + .set_workflow_state( + ticket_ref.id.into(), + TicketStateChange::new( + "ready", + "inprogress", + "Direct progress judgment", + "No Worker start implied", + ), + ) + .unwrap(); + } + + #[tokio::test] + async fn readiness_accepts_read_only_catalog_reference_without_default_selector() { + let dir = tempfile::tempdir().unwrap(); + let api = test_api_with_remote_repository(dir.path()).await; + let mut repository = api + .store + .get_repository_by_key(TEST_WORKSPACE_ID, "test-repository") + .unwrap() + .unwrap(); + repository.repository_id = "reference-without-default".into(); + repository.repository_key = "reference".into(); + repository.default_ref = None; + api.store.upsert_repository(&repository).unwrap(); + let backend = browser_ticket_backend(&api).unwrap(); + let mut input = ticket::NewTicket::new("Reference only; no checkout decision yet"); + input.targets = vec![ticket::TicketTarget { + repository_key: "reference".into(), + ref_selector: None, + access: TicketTargetAccess::ReadOnly, + }]; + let reference = backend.create(input).unwrap(); + let ready = backend + .mark_ready( + reference.id.clone().into(), + ticket::TicketMarkReady { + operation_key: "reference-ready".into(), + reason: Some("Research ready".into()), + author: Some("workspace-user".into()), + intake_summary: None, + }, + ) + .unwrap(); + assert_eq!(ready.meta.workflow_state, TicketWorkflowState::Ready); + assert_eq!(ready.meta.targets[0].ref_selector, None); + assert!( + validated_ticket_implementation_targets(&api, &reference.id).is_err(), + "asset operations still require their own grants and selectors" + ); } #[tokio::test] @@ -46998,7 +47039,7 @@ mod tests { }, ]; for operation in preparatory_operations { - execute_ticket_rest_operation(&api, TEST_WORKSPACE_ID, source_headers(), operation) + execute_ticket_rest_operation(&api, TEST_WORKSPACE_ID, HeaderMap::new(), operation) .await .unwrap(); } @@ -47117,158 +47158,131 @@ mod tests { } #[tokio::test] - async fn complete_ticket_api_replays_done_operation_without_reviving_assignment_authority() { + async fn ticket_decisions_reject_other_workers_and_browser_route_source_bypasses() { let dir = tempfile::tempdir().unwrap(); let api = test_api(dir.path()).await; let backend = browser_ticket_backend(&api).unwrap(); - let mut input = ticket::NewTicket::new("Completed operation replay"); + let mut input = ticket::NewTicket::new("Assigned investigation"); input.workflow_state = Some(TicketWorkflowState::InProgress); - let ticket = backend.create(input).unwrap(); - let coder = seed_live_test_coder_assignment(&api, &ticket.id, "completed-coder").await; - let Json(started) = scoped_start_workspace_orchestrator( + let ticket = backend.create(input.clone()).unwrap(); + let _owner = seed_live_test_coder_assignment(&api, &ticket.id, "owner").await; + let other_ticket = backend.create(input).unwrap(); + let other = seed_live_test_coder_assignment(&api, &other_ticket.id, "other").await; + let before = backend.show(ticket.id.clone().into()).unwrap(); + let request = server_api::CompleteTicketRequest { + operation_key: "forged".into(), + expected_item_revision: ticket::ticket_item_revision(&before), + expected_state: TicketWorkflowState::InProgress, + reason: "Not my task".into(), + references: Vec::new(), + }; + let response = scoped_complete_ticket( State(api.clone()), - AxumPath(ScopedWorkspacePath { - workspace_id: TEST_WORKSPACE_ID.to_string(), + ticket_check_headers(&other), + AxumPath((TEST_WORKSPACE_ID.into(), ticket.id.clone())), + Json(request), + ) + .await + .unwrap_err() + .into_response(); + assert_eq!(response.status(), StatusCode::FORBIDDEN); + let response = scoped_transition_ticket_state( + State(api.clone()), + ticket_check_headers(&other), + AxumPath(ScopedRecordPath { + workspace_id: TEST_WORKSPACE_ID.into(), + id: ticket.id.clone(), + }), + Json(server_api::BrowserTransitionTicketStateRequest { + state: server_api::BrowserTicketWorkflowState::Done, + expected_state: server_api::BrowserTicketWorkflowState::Inprogress, + operation_key: "browser-forged".into(), + expected_item_revision: ticket::ticket_item_revision(&before), + reason: "Not human authority".into(), + body: None, }), ) .await - .unwrap(); - let orchestrator = started.worker.unwrap(); - let orchestrator = RuntimeWorkerRef::new(orchestrator.runtime_id, orchestrator.worker_id); - let recorded = merge_request::TicketCompletionEvent { - operation_id: "recorded-ticket-completion".to_string(), - ticket_id: ticket.id.clone(), - item_revision: api.authority.ticket(&ticket.id).unwrap().item_revision, - merge_request_ids: vec!["merged-result".to_string()], - requirement_approval_event_id: "requirement-approval".to_string(), - completed_by: merge_request::WorkerIdentity { - runtime_id: orchestrator.runtime_id.clone(), - worker_id: orchestrator.worker_id.clone(), - }, - created_at: Utc::now(), + .unwrap_err() + .into_response(); + assert_eq!(response.status(), StatusCode::FORBIDDEN); + assert_eq!(backend.show(ticket.id.into()).unwrap(), before); + } + + #[tokio::test] + async fn assigned_coder_completion_without_mr_replays_and_rejects_changed_decisions() { + let dir = tempfile::tempdir().unwrap(); + let api = test_api(dir.path()).await; + let backend = browser_ticket_backend(&api).unwrap(); + let mut input = ticket::NewTicket::new("Research complete without repository"); + input.workflow_state = Some(TicketWorkflowState::InProgress); + let reference = backend.create(input).unwrap(); + let coder = seed_live_test_coder_assignment(&api, &reference.id, "completed-coder").await; + let before = backend.show(reference.id.clone().into()).unwrap(); + let request = server_api::CompleteTicketRequest { + operation_key: "research-done".into(), + expected_item_revision: ticket::ticket_item_revision(&before), + expected_state: TicketWorkflowState::InProgress, + reason: "Investigation findings recorded in the Ticket thread".into(), + references: Vec::new(), }; - // Seed the persisted completion boundary, not the review/integration flow: - // this test proves API replay authorization against an already-recorded result. - let conn = rusqlite::Connection::open(&api.config.database_path).unwrap(); - crate::store::configure_sqlite(&conn).unwrap(); - conn.execute( - "UPDATE typed_tickets SET workflow_state = 'done' - WHERE workspace_id = ?1 AND ticket_id = ?2", - rusqlite::params![TEST_WORKSPACE_ID, ticket.id], - ) - .unwrap(); - backend - .with_event_attributes(BTreeMap::from([ - ("operation_id".to_string(), recorded.operation_id.clone()), - ( - "ticket_completion".to_string(), - serde_json::to_string(&recorded).unwrap(), - ), - ])) - .add_event( - TicketIdOrSlug::Id(ticket.id.clone()), - NewTicketEvent::new(TicketEventKind::StateChanged, "Recorded completion"), + let call = |request| { + scoped_complete_ticket( + State(api.clone()), + ticket_check_headers(&coder), + AxumPath((TEST_WORKSPACE_ID.into(), reference.id.clone())), + Json(request), ) - .unwrap(); - let mut former_actor_record = recorded.clone(); - former_actor_record.operation_id = "former-actor-completion".to_string(); - former_actor_record.completed_by = merge_request::WorkerIdentity { - runtime_id: coder.runtime_id.clone(), - worker_id: coder.worker_id.clone(), }; - browser_ticket_backend(&api) - .unwrap() - .with_event_attributes(BTreeMap::from([ - ( - "operation_id".to_string(), - former_actor_record.operation_id.clone(), - ), - ( - "ticket_completion".to_string(), - serde_json::to_string(&former_actor_record).unwrap(), - ), - ])) - .add_event( - TicketIdOrSlug::Id(ticket.id.clone()), - NewTicketEvent::new(TicketEventKind::StateChanged, "Former actor completion"), - ) - .unwrap(); - let event_count = browser_ticket_backend(&api) - .unwrap() - .show(TicketIdOrSlug::Id(ticket.id.clone())) - .unwrap() - .events - .len(); - let request = || server_api::CompleteTicketRequest { - operation_id: recorded.operation_id.clone(), - item_revision: recorded.item_revision.clone(), - merge_request_ids: recorded.merge_request_ids.clone(), - requirement_approval_event_id: recorded.requirement_approval_event_id.clone(), - }; - let Json(replayed) = scoped_complete_ticket( - State(api.clone()), - ticket_check_headers(&orchestrator), - AxumPath((TEST_WORKSPACE_ID.to_string(), ticket.id.clone())), - Json(request()), - ) - .await - .unwrap(); - assert_eq!( - serde_json::to_value(replayed).unwrap(), - serde_json::to_value(public_ticket_completion_event(recorded.clone())).unwrap() + let Json(completed) = call(request.clone()).await.unwrap(); + assert_eq!(completed.meta.workflow_state, TicketWorkflowState::Done); + assert!( + merge_request_store(&api, TEST_WORKSPACE_ID) + .unwrap() + .list_for_ticket(TEST_WORKSPACE_ID, &reference.id) + .unwrap() + .is_empty() ); - for field in ["operation", "revision", "mr_set", "approval", "actor"] { - let mut changed = request(); + let Json(replayed) = call(request.clone()).await.unwrap(); + assert_eq!(replayed.events, completed.events); + for field in ["reason", "revision", "state", "references"] { + let mut changed = request.clone(); match field { - "operation" => changed.operation_id = "new-completion-operation".to_string(), - "revision" => changed.item_revision = "different-revision".to_string(), - "mr_set" => changed.merge_request_ids = vec!["different-result".to_string()], - "approval" => { - changed.requirement_approval_event_id = "different-approval".to_string(); - } - "actor" => changed.operation_id = former_actor_record.operation_id.clone(), + "reason" => changed.reason = "different judgment".into(), + "revision" => changed.expected_item_revision = "stale".into(), + "state" => changed.expected_state = TicketWorkflowState::Planning, + "references" => changed.references.push(ticket::TicketReference { + kind: "document".into(), + target: "other".into(), + }), _ => unreachable!(), } - let response = scoped_complete_ticket( - State(api.clone()), - ticket_check_headers(&orchestrator), - AxumPath((TEST_WORKSPACE_ID.to_string(), ticket.id.clone())), - Json(changed), - ) - .await - .unwrap_err() - .into_response(); + let response = call(changed).await.unwrap_err().into_response(); let status = response.status(); let bytes = to_bytes(response.into_body(), usize::MAX).await.unwrap(); - let body = String::from_utf8_lossy(&bytes); - assert_eq!(status, StatusCode::CONFLICT, "field={field}: {body}"); - let reason = if field == "operation" { - "Ticket must be inprogress" - } else { - "Ticket completion operation fingerprint mismatch" - }; - assert!(body.contains(reason), "field={field}: {body}"); + assert_eq!( + status, + StatusCode::CONFLICT, + "{field}: {}", + String::from_utf8_lossy(&bytes) + ); } assert!( api.store - .get_active_ticket_role_assignment_for_worker(TEST_WORKSPACE_ID, &coder) + .get_active_ticket_worker_assignment(TEST_WORKSPACE_ID, &reference.id) .unwrap() .is_none() ); - assert_eq!( + assert!( api.store - .get_current_ticket_coder_assignment(TEST_WORKSPACE_ID, &ticket.id) + .get_current_ticket_coder_assignment(TEST_WORKSPACE_ID, &reference.id) .unwrap() - .unwrap() - .assignment_id, - "completed-coder" + .is_some() ); - let after = browser_ticket_backend(&api) - .unwrap() - .show(TicketIdOrSlug::Id(ticket.id)) - .unwrap(); - assert_eq!(after.meta.workflow_state, TicketWorkflowState::Done); - assert_eq!(after.events.len(), event_count); + let mut fresh = request; + fresh.operation_key = "unassigned-new-operation".into(); + fresh.expected_state = TicketWorkflowState::Done; + assert!(call(fresh).await.is_err()); } #[tokio::test] @@ -47297,7 +47311,14 @@ mod tests { State(api.clone()), AxumPath((TEST_WORKSPACE_ID.to_string(), ticket.id.clone())), HeaderMap::new(), - Json(MarkdownText::new("Normal close ends implementation work")), + Json(ticket::TicketCompletion { + operation_key: "normal-close".into(), + expected_item_revision: api.authority.ticket(&ticket.id).unwrap().item_revision, + expected_state: TicketWorkflowState::InProgress, + reason: "Normal close ends implementation work".into(), + references: Vec::new(), + author: None, + }), ) .await .unwrap(); @@ -47430,22 +47451,6 @@ mod tests { ) .await .unwrap(); - let response = scoped_queue_ticket_record( - State(api.clone()), - AxumPath((TEST_WORKSPACE_ID.to_string(), ticket.id.clone())), - HeaderMap::new(), - ) - .await - .unwrap_err() - .into_response(); - let status = response.status(); - let bytes = to_bytes(response.into_body(), usize::MAX).await.unwrap(); - let body = String::from_utf8_lossy(&bytes); - assert_eq!(status, StatusCode::CONFLICT, "{body}"); - assert!( - body.contains("Queue requires role=orchestrator assignment"), - "{body}" - ); assert!(execution.take_inputs().is_empty()); let Json(assigned) = scoped_set_ticket_assignment( State(api.clone()), @@ -47710,6 +47715,97 @@ mod tests { ); } + #[tokio::test] + async fn active_target_edits_do_not_expand_bound_grants_and_mismatch_fails_closed() { + let fixture = manual_coder_assignment_fixture().await; + let _ = scoped_set_ticket_assignment( + State(fixture.api.clone()), + AxumPath(( + TEST_WORKSPACE_ID.into(), + fixture.ticket_id.clone(), + "coder".into(), + )), + Json(manual_coder_assignment_request( + &fixture, + "bind-original-grants", + )), + ) + .await + .unwrap(); + let backend = browser_ticket_backend(&fixture.api).unwrap(); + let links = fixture + .api + .store + .list_worker_workdir_links(TEST_WORKSPACE_ID, &fixture.worker) + .unwrap(); + let docs_link = links + .iter() + .find(|link| link.workdir_id == fixture.docs_workdir_id) + .unwrap(); + let docs = fixture + .api + .store + .get_workdir_registry(TEST_WORKSPACE_ID, &fixture.docs_workdir_id) + .unwrap() + .unwrap(); + backend + .edit_item( + fixture.ticket_id.clone().into(), + TicketItemEdit { + targets: Some(TicketTargetsEdit::Set { + targets: vec![ + test_ticket_target( + "test-repository", + "develop", + TicketTargetAccess::ReadWrite, + ), + test_ticket_target("docs", "develop", TicketTargetAccess::ReadWrite), + ], + }), + ..Default::default() + }, + ) + .unwrap(); + assert_eq!( + effective_worker_workdir_capabilities( + &fixture.api, + &fixture.worker, + &docs, + docs_link.capabilities + ) + .unwrap(), + workdir::WorkdirSessionCapabilities::READ_ONLY + ); + assert_eq!( + fixture + .runtime + .logical_attachments(&fixture.worker.worker_id) + .iter() + .find(|attachment| attachment.alias.as_str() == "docs") + .unwrap() + .capabilities, + workdir::WorkdirSessionCapabilities::READ_ONLY + ); + backend + .edit_item( + fixture.ticket_id.into(), + TicketItemEdit { + targets: Some(TicketTargetsEdit::Clear), + ..Default::default() + }, + ) + .unwrap(); + assert!( + effective_worker_workdir_capabilities( + &fixture.api, + &fixture.worker, + &docs, + docs_link.capabilities + ) + .is_err() + ); + } + #[tokio::test] async fn manual_coder_assignment_rejects_incomplete_or_mismatched_workdirs_before_state_change() { @@ -48500,131 +48596,132 @@ mod tests { } #[tokio::test] - async fn generic_browser_state_mutation_cannot_bypass_assignment_start() { + async fn browser_progress_decisions_allow_direct_completion_and_reopen_with_cas() { + let dir = tempfile::tempdir().unwrap(); + let api = test_api(dir.path()).await; + let backend = browser_ticket_backend(&api).unwrap(); + let reference = backend + .create(ticket::NewTicket::new("No asset needed")) + .unwrap(); + for (index, state) in [ + server_api::BrowserTicketWorkflowState::Inprogress, + server_api::BrowserTicketWorkflowState::Done, + server_api::BrowserTicketWorkflowState::Closed, + server_api::BrowserTicketWorkflowState::Planning, + ] + .into_iter() + .enumerate() + { + let before = backend.show(reference.id.clone().into()).unwrap(); + let expected_state = + serde_json::from_value(serde_json::json!(before.meta.workflow_state.as_str())) + .unwrap(); + let request = server_api::BrowserTransitionTicketStateRequest { + state: state.clone(), + operation_key: format!("decision-{index}"), + expected_item_revision: ticket::ticket_item_revision(&before), + expected_state, + reason: "Human progress judgment".into(), + body: None, + }; + let Json(after) = scoped_transition_ticket_state( + State(api.clone()), + HeaderMap::new(), + AxumPath(ScopedRecordPath { + workspace_id: TEST_WORKSPACE_ID.into(), + id: reference.id.clone(), + }), + Json(request), + ) + .await + .unwrap(); + assert_eq!(after.state, browser_ticket_workflow_state(state).as_str()); + assert!( + api.store + .get_current_ticket_coder_assignment(TEST_WORKSPACE_ID, &reference.id) + .unwrap() + .is_none() + ); + } + let before = backend.show(reference.id.clone().into()).unwrap(); + let response = scoped_transition_ticket_state( + State(api), + HeaderMap::new(), + AxumPath(ScopedRecordPath { + workspace_id: TEST_WORKSPACE_ID.into(), + id: reference.id, + }), + Json(server_api::BrowserTransitionTicketStateRequest { + state: server_api::BrowserTicketWorkflowState::Done, + operation_key: "stale".into(), + expected_item_revision: "stale-revision".into(), + expected_state: server_api::BrowserTicketWorkflowState::Planning, + reason: "Stale decision".into(), + body: None, + }), + ) + .await + .unwrap_err() + .into_response(); + assert_eq!(response.status(), StatusCode::CONFLICT); + assert_eq!(before.meta.workflow_state, TicketWorkflowState::Planning); + } + + #[tokio::test] + async fn explicit_queue_without_assets_or_assignment_records_only_requested_work() { let dir = tempfile::tempdir().unwrap(); let api = test_api(dir.path()).await; let backend = browser_ticket_backend(&api).unwrap(); let ticket = backend - .create(ticket::NewTicket::new("Generic transition guard")) + .create(ticket::NewTicket::new("Queue research")) .unwrap(); - let path = ScopedRecordPath { - workspace_id: TEST_WORKSPACE_ID.to_string(), - id: ticket.id.clone(), - }; - - let planning = scoped_transition_ticket_state( + let Json(outcome) = scoped_queue_ticket_record( State(api.clone()), - AxumPath(path.clone()), - Json(server_api::BrowserTransitionTicketStateRequest { - state: server_api::BrowserTicketWorkflowState::Inprogress, - reason: None, - body: None, - author: None, - }), - ) - .await - .unwrap_err() - .into_response(); - assert_eq!(planning.status(), StatusCode::CONFLICT); - assert_eq!( - backend - .show(ticket.id.clone().into()) - .unwrap() - .meta - .workflow_state, - TicketWorkflowState::Planning - ); - - let mut ready_input = ticket::NewTicket::new("Ready transition guard"); - ready_input.workflow_state = Some(TicketWorkflowState::Ready); - let ready = backend.create(ready_input).unwrap(); - let ready_result = scoped_transition_ticket_state( - State(api.clone()), - AxumPath(ScopedRecordPath { - workspace_id: TEST_WORKSPACE_ID.to_string(), - id: ready.id.clone(), - }), - Json(server_api::BrowserTransitionTicketStateRequest { - state: server_api::BrowserTicketWorkflowState::Inprogress, - reason: None, - body: None, - author: None, - }), - ) - .await - .unwrap_err() - .into_response(); - assert_eq!(ready_result.status(), StatusCode::CONFLICT); - assert_eq!( - backend.show(ready.id.into()).unwrap().meta.workflow_state, - TicketWorkflowState::Ready - ); - } - - #[tokio::test] - async fn queue_requires_orchestrator_role_and_records_assignment_fence() { - let dir = tempfile::tempdir().unwrap(); - init_clean_git_workspace(dir.path()); - let api = test_api(dir.path()).await; - let backend = browser_ticket_backend(&api).unwrap(); - let mut input = ticket::NewTicket::new("Queue role gate"); - input.workflow_state = Some(TicketWorkflowState::Ready); - set_test_ticket_target(&mut input, "test-repository", "develop"); - let ticket = backend.create(input).unwrap(); - let path = (TEST_WORKSPACE_ID.to_string(), ticket.id.clone()); - - let legacy_missing = scoped_queue_ticket( - State(api.clone()), - AxumPath(ScopedRecordPath { - workspace_id: TEST_WORKSPACE_ID.to_string(), - id: ticket.id.clone(), - }), - Json(server_api::BrowserQueueTicketRequest {}), - ) - .await - .unwrap_err() - .into_response(); - assert_eq!(legacy_missing.status(), StatusCode::CONFLICT); - - let missing = scoped_queue_ticket_record( - State(api.clone()), - AxumPath(path.clone()), + AxumPath((TEST_WORKSPACE_ID.into(), ticket.id.clone())), HeaderMap::new(), ) .await - .unwrap_err() - .into_response(); - assert_eq!(missing.status(), StatusCode::CONFLICT); - assert_eq!( - backend - .show(ticket.id.clone().into()) - .unwrap() - .meta - .workflow_state, - TicketWorkflowState::Ready - ); - - assign_test_orchestrator(&api, &ticket.id); - let _ = scoped_queue_ticket_record(State(api.clone()), AxumPath(path), HeaderMap::new()) - .await - .unwrap(); - let queued = backend.show(ticket.id.into()).unwrap(); + .unwrap(); + assert_eq!(outcome.queued_tickets, vec![ticket.id.clone()]); + let queued = backend.show(ticket.id.clone().into()).unwrap(); assert_eq!(queued.meta.workflow_state, TicketWorkflowState::Queued); - let event = queued.events.last().unwrap(); - let expected_assignment_id = format!("orchestrator-{}", queued.meta.id); - assert_eq!( - event + assert!( + !queued + .events + .last() + .unwrap() .attributes - .get("orchestrator_assignment_id") - .map(String::as_str), - Some(expected_assignment_id.as_str()) + .contains_key("orchestrator_assignment_id") + ); + assert!( + api.store + .get_current_ticket_coder_assignment(TEST_WORKSPACE_ID, &ticket.id) + .unwrap() + .is_none() + ); + // If an explicit routing identity exists, retain its fence for later claim. + let mut input = ticket::NewTicket::new("Fenced request"); + input.workflow_state = Some(TicketWorkflowState::Ready); + let fenced = backend.create(input).unwrap(); + assign_test_orchestrator(&api, &fenced.id); + let _ = scoped_queue_ticket_record( + State(api), + AxumPath((TEST_WORKSPACE_ID.into(), fenced.id.clone())), + HeaderMap::new(), + ) + .await + .unwrap(); + let queued = backend.show(fenced.id.clone().into()).unwrap(); + let event = queued.events.last().unwrap(); + assert_eq!( + event.attributes.get("orchestrator_assignment_id"), + Some(&format!("orchestrator-{}", fenced.id)) ); assert!(event.attributes.contains_key("routing_operation_id")); - assert!(event.attributes.contains_key("routing_request_fingerprint")); } #[tokio::test] - async fn queue_reports_dependency_cycle_before_assignment_validation() { + async fn queue_keeps_cycle_diagnostics_without_submitting_other_tickets() { let dir = tempfile::tempdir().unwrap(); init_clean_git_workspace(dir.path()); let api = test_api(dir.path()).await; @@ -48654,24 +48751,33 @@ mod tests { .unwrap(); } - let error = execute_ticket_rest_operation( + let outcome = execute_ticket_rest_operation( &api, TEST_WORKSPACE_ID, HeaderMap::new(), TicketBackendOperation::QueueReady { - id: TicketIdOrSlug::Id(first.id.clone()), - queued_by: "workspace-web".to_string(), + id: first.id.clone().into(), + queued_by: "workspace-web".into(), }, ) .await - .unwrap_err(); - assert!(error.error.to_string().contains("cycle")); - for ticket_id in [first.id, second.id] { - assert_eq!( - backend.show(ticket_id.into()).unwrap().meta.workflow_state, - TicketWorkflowState::Ready - ); - } + .unwrap(); + let TicketBackendOperationResult::QueueOutcome(outcome) = outcome else { + panic!("queue result") + }; + assert_eq!(outcome.queued_tickets, vec![first.id.clone()]); + assert_eq!( + backend.show(second.id.into()).unwrap().meta.workflow_state, + TicketWorkflowState::Ready + ); + assert!( + backend + .dependency_check(first.id.into()) + .unwrap() + .blockers + .iter() + .any(|blocker| blocker.reason_kind == "dependency_cycle") + ); } #[tokio::test] @@ -48711,96 +48817,19 @@ mod tests { ) .await .unwrap(); + assert_eq!(outcome.queued_tickets, vec![root.id.clone()]); assert_eq!( - outcome.queued_tickets, - vec![dependency.id.clone(), root.id.clone()] + backend.show(root.id.into()).unwrap().meta.workflow_state, + TicketWorkflowState::Queued ); - for ticket_id in [dependency.id, root.id] { - assert_eq!( - backend.show(ticket_id.into()).unwrap().meta.workflow_state, - TicketWorkflowState::Queued - ); - } - } - - #[tokio::test] - async fn queue_requires_assignments_for_every_ready_dependency() { - let dir = tempfile::tempdir().unwrap(); - init_clean_git_workspace(dir.path()); - let api = test_api(dir.path()).await; - let backend = browser_ticket_backend(&api).unwrap(); - let mut dependency_input = ticket::NewTicket::new("Ready dependency"); - dependency_input.workflow_state = Some(TicketWorkflowState::Ready); - set_test_ticket_target(&mut dependency_input, "test-repository", "develop"); - let dependency = backend.create(dependency_input).unwrap(); - let mut root_input = ticket::NewTicket::new("Queue root"); - root_input.workflow_state = Some(TicketWorkflowState::Ready); - set_test_ticket_target(&mut root_input, "test-repository", "develop"); - let root = backend.create(root_input).unwrap(); - backend - .add_ticket_relation( - root.id.clone().into(), - ticket::NewTicketRelation { - kind: ticket::TicketRelationKind::DependsOn, - target: dependency.id.clone(), - note: Some("must queue first".to_string()), - author: Some("test".to_string()), - }, - ) - .unwrap(); - - assign_test_orchestrator(&api, &root.id); - let error = scoped_queue_ticket_record( - State(api.clone()), - AxumPath((TEST_WORKSPACE_ID.to_string(), root.id.clone())), - HeaderMap::new(), - ) - .await - .unwrap_err() - .into_response(); - assert_eq!(error.status(), StatusCode::CONFLICT); assert_eq!( backend - .show(root.id.clone().into()) + .show(dependency.id.into()) .unwrap() .meta .workflow_state, TicketWorkflowState::Ready ); - assert_eq!( - backend - .show(dependency.id.clone().into()) - .unwrap() - .meta - .workflow_state, - TicketWorkflowState::Ready - ); - - assign_test_orchestrator(&api, &dependency.id); - let Json(outcome) = scoped_queue_ticket_record( - State(api.clone()), - AxumPath((TEST_WORKSPACE_ID.to_string(), root.id.clone())), - HeaderMap::new(), - ) - .await - .unwrap(); - assert_eq!(outcome.requested_ticket, root.id); - assert_eq!( - outcome.queued_tickets, - vec![dependency.id.clone(), root.id.clone()] - ); - for ticket_id in [dependency.id, root.id] { - let queued = backend.show(ticket_id.clone().into()).unwrap(); - assert_eq!(queued.meta.workflow_state, TicketWorkflowState::Queued); - assert_eq!( - queued - .events - .last() - .and_then(|event| event.attributes.get("orchestrator_assignment_id")) - .cloned(), - Some(format!("orchestrator-{ticket_id}")) - ); - } } #[tokio::test] @@ -50280,6 +50309,7 @@ mod tests { let Json(ready) = scoped_mark_ticket_ready_from_browser( State(api.clone()), + HeaderMap::new(), AxumPath(path()), Json(InternalTicketMarkReadyRequest { operation_key: "browser-ready".to_owned(), @@ -50306,14 +50336,15 @@ mod tests { let Json(queue_outcome) = scoped_queue_ticket( State(api.clone()), + HeaderMap::new(), AxumPath(path()), Json(server_api::BrowserQueueTicketRequest {}), ) .await .unwrap(); assert_eq!(queue_outcome.requested_ticket, ticket_id); - assert_eq!(queue_outcome.queued_tickets.len(), 2); - assert!(queue_outcome.queued_tickets.contains(&related_ticket_id)); + assert_eq!(queue_outcome.queued_tickets.len(), 1); + assert!(!queue_outcome.queued_tickets.contains(&related_ticket_id)); assert!(queue_outcome.queued_tickets.contains(&ticket_id)); let Json(queued) = scoped_get_ticket(State(api.clone()), AxumPath(path())) .await @@ -50324,8 +50355,12 @@ mod tests { assert_eq!(queued.relations.blockers[0].reason_kind, "depends_on"); let Json(closed) = scoped_close_ticket( State(api), + HeaderMap::new(), AxumPath(path()), Json(server_api::BrowserCloseTicketRequest { + operation_key: "browser-close".into(), + expected_item_revision: queued.item_revision.clone(), + expected_state: server_api::BrowserTicketWorkflowState::Queued, resolution: "Closed through the Browser API.".to_string(), }), ) diff --git a/crates/workspace-server/src/server/tests/ticket_evidence_tests.rs b/crates/workspace-server/src/server/tests/ticket_evidence_tests.rs index f88d38276..d63f64c1a 100644 --- a/crates/workspace-server/src/server/tests/ticket_evidence_tests.rs +++ b/crates/workspace-server/src/server/tests/ticket_evidence_tests.rs @@ -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); +} diff --git a/crates/workspace-server/src/ticket_item_checker.rs b/crates/workspace-server/src/ticket_item_checker.rs index af592f9cc..dd45980d0 100644 --- a/crates/workspace-server/src/ticket_item_checker.rs +++ b/crates/workspace-server/src/ticket_item_checker.rs @@ -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( diff --git a/docs/design/merge-request-wip-projection.md b/docs/design/merge-request-wip-projection.md index 4d314a7a2..cdf991eaf 100644 --- a/docs/design/merge-request-wip-projection.md +++ b/docs/design/merge-request-wip-projection.md @@ -24,7 +24,6 @@ Collection interface `yoi.merge-request/collection/v1`: | Native operation | Existing Tool | Preserved authority inputs | | --- | --- | --- | | `open` | `OpenMergeRequest` | Ticket, repository key, source selector, target selector, summary | -| `complete_ticket` | `CompleteTicket` | Ticket, operation ID, current item revision, exact linked-MR result set, requirement approval event | Item interface `yoi.merge-request/item/v1`: @@ -39,9 +38,9 @@ Typical role surfaces are therefore: - Coder: collection `open`; item `read`. - Reviewer: item `read` and `review`. -- Orchestrator: collection `complete_ticket`; item `read`, `check_readiness`, and `complete`. +- Orchestrator: item `read`, `check_readiness`, and `complete`. -`complete_ticket` is deliberately collection-scoped because its subject is a Ticket and an exact set of linked Merge Request results rather than one route-bound MR. It keeps those identities explicit. The review descriptor never contains the injected capability token, and a review item route must exactly match the Merge Request captured by that capability before the ordinary review Tool can run. +`CompleteTicket` belongs to the Ticket Feature and route-bound Ticket item `complete`, alongside state and close decisions. It takes current item/state CAS, a replay key, reason, and optional references; it requires neither an MR result set nor a Reviewer approval. The former MR collection `complete_ticket` operation is not published. The review descriptor never contains the injected capability token, and a review item route must exactly match the Merge Request captured by that capability before the ordinary review Tool can run. ## Preserved review and integration gates @@ -55,8 +54,8 @@ The native projection delegates to the same ordinary tools and REST endpoints. I - source movement invalidating source approval while target-only movement preserves unchanged-source review; - unresolved request-change and revoked/stale approval rejection; - current target observation, target-before/after proof, strategy, and conflict-resolution checks; -- Orchestrator-only Merge Request and Ticket completion; -- immutable integration/result evidence and exact linked-result-set Ticket completion. +- Orchestrator-only Merge Request integration; +- immutable integration/result evidence and exact linked-result-set requirement attestation. The Worldspace adapter performs no Git or provider integration. A visible `complete` operation remains unusable without the ordinary Backend's authenticated online Orchestrator source and integration evidence. @@ -64,7 +63,7 @@ The Worldspace adapter performs no Git or provider integration. A visible `compl `read` exposes the existing bounded thread page with optional `after` and a `1..=200` limit. Responses keep linked Tickets as references/data, so Ticket or Objective native projections are not prerequisites. Descriptors never expand repository diffs, unbounded thread history, credentials, or capability material. -A successful item read records a response fingerprint. A successful item mutation advances a local mutation generation. Opening records the returned MR and adds its native path to the result. Ticket completion invalidates every MR route named in its exact result set because releasing the assignment can change later ref observations. The common WIP Host rejects calls made with stale object/interface validators; callers must rediscover before deciding whether to call again. +A successful item read records a response fingerprint. A successful item mutation advances a local mutation generation. Opening records the returned MR and adds its native path to the result. Ticket decisions advance the Ticket provider's object generation, not the MR integration state or proofs. Ending Ticket work can affect later live source observation, so clients reread MR authority before MR actions; Backend checks remain authoritative. The common WIP Host rejects calls made with stale object/interface validators; callers must rediscover before deciding whether to call again. Optional fields preserve ordinary JSON-input semantics: omission remains omission and explicit WIP unit/null remains JSON `null` for schema and permission matching. The reconstructed object is validated against the original Tool JSON Schema before the Tool executes. diff --git a/docs/design/ticket-evidence.md b/docs/design/ticket-evidence.md index 638d0a13c..9b9497e5b 100644 --- a/docs/design/ticket-evidence.md +++ b/docs/design/ticket-evidence.md @@ -12,11 +12,13 @@ A merged MR's review status/timestamps describe **integration approval**, not a ## Current requirements -`merge_request::requirement_approval` is shared by CompleteTicket and Ticket reads. It validates an effective Reviewer approval of the current item revision and the **exact complete linked MR/source snapshot**, including the hosting MR's own source and corresponding ReviewRequested event. There is no primary-MR fallback, comment-derived approval, or timestamp-only freshness rule. A post-integration review may attest a new revision/snapshot without replacing MergeResult or its integration approval. +`merge_request::requirement_approval` evaluates Ticket read-side MR evidence; it is not a Ticket completion gate. It validates an effective Reviewer approval of the current item revision and the **exact complete linked MR/source snapshot**, including the hosting MR's own source and corresponding ReviewRequested event. There is no primary-MR fallback, comment-derived approval, or timestamp-only freshness rule. A post-integration review may attest a new revision/snapshot without replacing MergeResult or its integration approval. `approved_current_subject` describes valid source/integration approvals. `review_after_rescope` retains its wire name but now describes exact current requirement attestation, not ordering of wall-clock timestamps. `approved_review` query filtering requires that current attestation. `stale_after_rescope` requires substantive item-edit history and a known source-approved snapshot lacking current attestation; unknown observations are only `missing_evidence`. -`complete_for_integration` means the summary has valid source approvals and current requirement evidence, including for partially or fully integrated sets. It is not a claim that open repositories have been integrated, that target integration readiness has been checked, or that Ticket completion is authorized. CompleteTicket additionally requires all linked results merged and validates explicit approval identity, lifecycle, and assignment transactionally. +`complete_for_integration` means a nonempty linked MR set has valid source approvals and current requirement evidence, including for partially or fully integrated sets. It is not a claim that open repositories have been integrated, that target integration readiness has been checked, or that Ticket completion is authorized. An MRless Ticket has no applicable MR evidence: `missing` is empty, `review_status` is null, and approval/integration flags remain false. It does not match `missing_evidence` or `approved_review` merely because its state is done. + +CompleteTicket belongs to the Ticket domain. It records the authenticated actor's reason/result and optional references using current item revision/state CAS and an operation receipt. It needs no MR or Reviewer approval, and does not create or modify those proofs. State setting, completion, close, and reopen share the same transactional update boundary. Exact replay returns current Ticket authority without reapplying a historical decision or appending an event. Historical MR-based `ticket_completion` records remain readable and distinct from new judgment receipts. Writable targets authorize deliverables, not mandatory MRs for every target. Every linked MR remains part of the exact requirement snapshot; no arbitrary latest-review or single-repository shortcut is used. diff --git a/docs/report/t718-subworker-path-context.md b/docs/report/t718-subworker-path-context.md new file mode 100644 index 000000000..fb70a6329 --- /dev/null +++ b/docs/report/t718-subworker-path-context.md @@ -0,0 +1,15 @@ +# T-718: SubWorker の cwd・参照コンテキストに関する障壁 + +## 観測 + +T-718 の実装を `crates/ticket` / `crates/worker` / `crates/merge-request` に分担した。SubWorkerSpawn の cwd を各 crate に設定すると、子のファイル tool は `src/lib.rs` 等で成功する一方、親で使える repository-root-relative な `crates//src/...` は crate prefix が二重になって失敗した。cwd 未指定の Glob は一部の子で transient bash-output 側のパスを logical path として扱い失敗した。 + +また builtin default の子には ShowTicket が公開されず、別途 read scope に含めたテスト規則の親 root-relative path も子の cwd から参照できなかった。親が authoritative Ticket とテスト規則を先に読み、要件・規則・インターフェースを本文で転送して作業を継続した。 + +## 改善案 + +- 子の startup context に実際の Workdir attachment alias・logical root・tool cwd を明示する。 +- cwd と path root の関係を tool schema / Spawn 説明と一致させ、cwd 外に委譲した read-only scope の参照方法も示す。 +- Ticket implementation の子へ typed Ticket read を委譲できない場合は、親に authoritative item revision と本文を渡す責任があることを明示する。 + +本件では委譲 scope を広げたり Backend storage を直接操作する回避はしていない。製品要件の追加ではなく、分担時の tooling/context で発生した調整コストの記録である。 diff --git a/openapi/server-api.json b/openapi/server-api.json index 2634f31f4..d63e19c92 100644 --- a/openapi/server-api.json +++ b/openapi/server-api.json @@ -301,11 +301,23 @@ "BrowserCloseTicketRequest": { "additionalProperties": false, "properties": { + "expected_item_revision": { + "type": "string" + }, + "expected_state": { + "$ref": "#/components/schemas/BrowserTicketWorkflowState" + }, + "operation_key": { + "type": "string" + }, "resolution": { "type": "string" } }, "required": [ + "operation_key", + "expected_item_revision", + "expected_state", "resolution" ], "type": "object" @@ -459,30 +471,34 @@ "BrowserTransitionTicketStateRequest": { "additionalProperties": false, "properties": { - "author": { - "type": [ - "string", - "null" - ] - }, "body": { "type": [ "string", "null" ] }, + "expected_item_revision": { + "type": "string" + }, + "expected_state": { + "$ref": "#/components/schemas/BrowserTicketWorkflowState" + }, + "operation_key": { + "type": "string" + }, "reason": { - "type": [ - "string", - "null" - ] + "type": "string" }, "state": { "$ref": "#/components/schemas/BrowserTicketWorkflowState" } }, "required": [ - "state" + "state", + "operation_key", + "expected_item_revision", + "expected_state", + "reason" ], "type": "object" }, @@ -1075,6 +1091,37 @@ ], "type": "object" }, + "CompleteTicketRequest": { + "additionalProperties": false, + "properties": { + "expected_item_revision": { + "type": "string" + }, + "expected_state": { + "$ref": "#/components/schemas/TicketWorkflowState" + }, + "operation_key": { + "type": "string" + }, + "reason": { + "type": "string" + }, + "references": { + "default": [], + "items": { + "$ref": "#/components/schemas/TicketReference" + }, + "type": "array" + } + }, + "required": [ + "operation_key", + "expected_item_revision", + "expected_state", + "reason" + ], + "type": "object" + }, "CompletionContext": { "properties": { "argument": { @@ -10199,6 +10246,45 @@ ], "type": "object" }, + "TicketCompletion": { + "additionalProperties": false, + "description": "Audited completion independent of repositories, Git, or Merge Requests.", + "properties": { + "author": { + "default": null, + "type": [ + "string", + "null" + ] + }, + "expected_item_revision": { + "type": "string" + }, + "expected_state": { + "$ref": "#/components/schemas/TicketWorkflowState" + }, + "operation_key": { + "type": "string" + }, + "reason": { + "type": "string" + }, + "references": { + "default": [], + "items": { + "$ref": "#/components/schemas/TicketReference" + }, + "type": "array" + } + }, + "required": [ + "expected_item_revision", + "expected_state", + "operation_key", + "reason" + ], + "type": "object" + }, "TicketDependencyCheck": { "properties": { "blockers": { @@ -12126,39 +12212,46 @@ } ] }, - "TicketStateChange": { + "TicketStateUpdate": { + "additionalProperties": false, + "description": "External state mutation with item/state CAS and an immutable operation receipt.\nExact replay returns the current authoritative Ticket without a new event or\nreapplying the recorded state, even after later edits or reopening.\nSetting `state` to Closed also stores `reason` as the resolution; reopening\nchanges Ticket metadata only and never revives released work identities.", "properties": { "author": { + "default": null, "type": [ "string", "null" ] }, - "body": { - "$ref": "#/components/schemas/MarkdownText" + "expected_item_revision": { + "type": "string" }, - "from": { + "expected_state": { + "$ref": "#/components/schemas/TicketWorkflowState" + }, + "operation_key": { "type": "string" }, "reason": { "type": "string" }, "references": { + "default": [], "items": { "$ref": "#/components/schemas/TicketReference" }, "type": "array" }, - "to": { - "type": "string" + "state": { + "$ref": "#/components/schemas/TicketWorkflowState" } }, "required": [ - "from", - "to", - "reason", - "body", - "references" + "expected_item_revision", + "expected_state", + "state", + "operation_key", + "reason" ], "type": "object" }, @@ -15538,7 +15631,7 @@ "info": { "title": "Yoi Server API", "version": "0.1.0", - "x-yoi-source-digest": "sha256:4f468e00efa86e6e33e8fa4f14deca93ee15b955071920facd56c59c3d2fefb8" + "x-yoi-source-digest": "sha256:ab4b109d99077a8b5af98923b77d8fc1b707f7ee9ad08304e26d497ccb3799f6" }, "openapi": "3.1.0", "paths": { @@ -31998,6 +32091,143 @@ }, "description": "Error response" }, + "422": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RepositoryApiError" + } + } + }, + "description": "Error response" + }, + "500": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RepositoryApiError" + } + } + }, + "description": "Error response" + } + }, + "security": [ + { + "bearerAuth": [] + }, + { + "browserSession": [] + } + ] + } + }, + "/api/w/{workspace_id}/tickets/{id}/complete": { + "post": { + "operationId": "ticket_complete", + "parameters": [ + { + "explode": false, + "in": "path", + "name": "workspace_id", + "required": true, + "schema": { + "$ref": "#/components/schemas/string" + }, + "style": "simple" + }, + { + "explode": false, + "in": "path", + "name": "id", + "required": true, + "schema": { + "$ref": "#/components/schemas/string" + }, + "style": "simple" + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/CompleteTicketRequest" + } + } + }, + "required": true + }, + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Ticket" + } + } + }, + "description": "Successful response" + }, + "400": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RepositoryApiError" + } + } + }, + "description": "Error response" + }, + "401": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RepositoryApiError" + } + } + }, + "description": "Error response" + }, + "403": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RepositoryApiError" + } + } + }, + "description": "Error response" + }, + "404": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RepositoryApiError" + } + } + }, + "description": "Error response" + }, + "409": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RepositoryApiError" + } + } + }, + "description": "Error response" + }, + "422": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RepositoryApiError" + } + } + }, + "description": "Error response" + }, "500": { "content": { "application/json": { @@ -33557,6 +33787,16 @@ }, "description": "Error response" }, + "422": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RepositoryApiError" + } + } + }, + "description": "Error response" + }, "500": { "content": { "application/json": { @@ -33607,7 +33847,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/TicketStateChange" + "$ref": "#/components/schemas/TicketStateUpdate" } } }, @@ -33667,6 +33907,16 @@ }, "description": "Error response" }, + "422": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RepositoryApiError" + } + } + }, + "description": "Error response" + }, "500": { "content": { "application/json": { @@ -33727,7 +33977,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/TicketStateChange" + "$ref": "#/components/schemas/TicketStateUpdate" } } }, @@ -33787,6 +34037,143 @@ }, "description": "Error response" }, + "422": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RepositoryApiError" + } + } + }, + "description": "Error response" + }, + "500": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RepositoryApiError" + } + } + }, + "description": "Error response" + } + }, + "security": [ + { + "bearerAuth": [] + }, + { + "browserSession": [] + } + ] + } + }, + "/api/w/{workspace_id}/tickets/{id}/state-update": { + "post": { + "operationId": "ticket_state_update", + "parameters": [ + { + "explode": false, + "in": "path", + "name": "workspace_id", + "required": true, + "schema": { + "$ref": "#/components/schemas/string" + }, + "style": "simple" + }, + { + "explode": false, + "in": "path", + "name": "id", + "required": true, + "schema": { + "$ref": "#/components/schemas/string" + }, + "style": "simple" + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/TicketStateUpdate" + } + } + }, + "required": true + }, + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Ticket" + } + } + }, + "description": "Successful response" + }, + "400": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RepositoryApiError" + } + } + }, + "description": "Error response" + }, + "401": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RepositoryApiError" + } + } + }, + "description": "Error response" + }, + "403": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RepositoryApiError" + } + } + }, + "description": "Error response" + }, + "404": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RepositoryApiError" + } + } + }, + "description": "Error response" + }, + "409": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RepositoryApiError" + } + } + }, + "description": "Error response" + }, + "422": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RepositoryApiError" + } + } + }, + "description": "Error response" + }, "500": { "content": { "application/json": { @@ -33947,7 +34334,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/TicketStateChange" + "$ref": "#/components/schemas/TicketStateUpdate" } } }, @@ -34007,6 +34394,16 @@ }, "description": "Error response" }, + "422": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RepositoryApiError" + } + } + }, + "description": "Error response" + }, "500": { "content": { "application/json": { @@ -34057,7 +34454,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/MarkdownText" + "$ref": "#/components/schemas/TicketCompletion" } } }, @@ -34117,6 +34514,16 @@ }, "description": "Error response" }, + "422": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RepositoryApiError" + } + } + }, + "description": "Error response" + }, "500": { "content": { "application/json": { diff --git a/web/workspace/src/lib/generated/companion-api.ts b/web/workspace/src/lib/generated/companion-api.ts index 185528645..946e17772 100644 --- a/web/workspace/src/lib/generated/companion-api.ts +++ b/web/workspace/src/lib/generated/companion-api.ts @@ -1,6 +1,6 @@ // @generated by yoi-companion-openapi-typescript@1. Do not edit. // Input: openapi/server-api.json -// Input contract: 0.1.0 (sha256:4f468e00efa86e6e33e8fa4f14deca93ee15b955071920facd56c59c3d2fefb8) +// Input contract: 0.1.0 (sha256:ab4b109d99077a8b5af98923b77d8fc1b707f7ee9ad08304e26d497ccb3799f6) // Options: companion-operation-schema-closure,openapi-3.1,strict-integer-ranges,sorted-output // Output: web/workspace/src/lib/generated/companion-api.ts // Regenerate/check: cargo run -q -p server-api --example generate_companion_api_types -- --check diff --git a/web/workspace/src/lib/generated/repository-api.ts b/web/workspace/src/lib/generated/repository-api.ts index 7f0bbc332..e337d80ba 100644 --- a/web/workspace/src/lib/generated/repository-api.ts +++ b/web/workspace/src/lib/generated/repository-api.ts @@ -1,6 +1,6 @@ // @generated by yoi-repository-openapi-typescript@1. Do not edit. // Input: openapi/server-api.json -// Input contract: 0.1.0 (sha256:4f468e00efa86e6e33e8fa4f14deca93ee15b955071920facd56c59c3d2fefb8) +// Input contract: 0.1.0 (sha256:ab4b109d99077a8b5af98923b77d8fc1b707f7ee9ad08304e26d497ccb3799f6) // Options: repository-operation-schema-closure,openapi-3.1,format:uint64=safe-number,strict-safe-integers,sorted-output // Output: web/workspace/src/lib/generated/repository-api.ts // Regenerate/check: cargo run -q -p server-api --example generate_repository_openapi_types -- --check diff --git a/web/workspace/src/lib/generated/runtime-api.ts b/web/workspace/src/lib/generated/runtime-api.ts index f0474897d..d970eb086 100644 --- a/web/workspace/src/lib/generated/runtime-api.ts +++ b/web/workspace/src/lib/generated/runtime-api.ts @@ -1,6 +1,6 @@ // @generated by yoi-runtime-openapi-typescript@1. Do not edit. // Input: openapi/server-api.json -// Input contract: 0.1.0 (sha256:4f468e00efa86e6e33e8fa4f14deca93ee15b955071920facd56c59c3d2fefb8) +// Input contract: 0.1.0 (sha256:ab4b109d99077a8b5af98923b77d8fc1b707f7ee9ad08304e26d497ccb3799f6) // Options: runtime-operation-schema-closure,openapi-3.1,strict-integer-ranges,sorted-output // Output: web/workspace/src/lib/generated/runtime-api.ts // Regenerate/check: cargo run -q -p server-api --example generate_runtime_api_types -- --check diff --git a/web/workspace/src/lib/generated/ticket-api.ts b/web/workspace/src/lib/generated/ticket-api.ts index c44888c50..b8e36ed49 100644 --- a/web/workspace/src/lib/generated/ticket-api.ts +++ b/web/workspace/src/lib/generated/ticket-api.ts @@ -1,13 +1,13 @@ // @generated by yoi-ticket-browser-openapi-typescript@1. Do not edit. // Input: openapi/server-api.json -// Input contract: 0.1.0 (sha256:4f468e00efa86e6e33e8fa4f14deca93ee15b955071920facd56c59c3d2fefb8) +// Input contract: 0.1.0 (sha256:ab4b109d99077a8b5af98923b77d8fc1b707f7ee9ad08304e26d497ccb3799f6) // Options: ticket-objective-merge-request-browser-operation-schema-closure,openapi-3.1,strict-integer-ranges,sorted-output // Output: web/workspace/src/lib/generated/ticket-api.ts // Regenerate/check: cargo run -q -p server-api --example generate_ticket_api_types -- --check export type BrowserAppendTicketEventRequest = { author?: string | null; body: string; role: BrowserTicketThreadRole; }; -export type BrowserCloseTicketRequest = { resolution: string; }; +export type BrowserCloseTicketRequest = { expected_item_revision: string; expected_state: BrowserTicketWorkflowState; operation_key: string; resolution: string; }; export type BrowserEditTicketRequest = { author?: string | null; body?: string | null; new_string?: string | null; old_string?: string | null; replace_all?: boolean; target?: BrowserTicketTargetEdit | null; title?: string | null; }; @@ -19,7 +19,7 @@ export type BrowserTicketThreadRole = "comment" | "plan" | "decision" | "impleme export type BrowserTicketWorkflowState = "planning" | "ready" | "queued" | "inprogress" | "done" | "closed"; -export type BrowserTransitionTicketStateRequest = { author?: string | null; body?: string | null; reason?: string | null; state: BrowserTicketWorkflowState; }; +export type BrowserTransitionTicketStateRequest = { body?: string | null; expected_item_revision: string; expected_state: BrowserTicketWorkflowState; operation_key: string; reason: string; state: BrowserTicketWorkflowState; }; export type CancelTicketImplementationRequest = { assignment_id: string; operation_id: string; reason: string; }; diff --git a/web/workspace/src/lib/generated/workdir-api.ts b/web/workspace/src/lib/generated/workdir-api.ts index d545d712c..5ad86bc2b 100644 --- a/web/workspace/src/lib/generated/workdir-api.ts +++ b/web/workspace/src/lib/generated/workdir-api.ts @@ -1,6 +1,6 @@ // @generated by yoi-workdir-openapi-typescript@1. Do not edit. // Input: openapi/server-api.json -// Input contract: 0.1.0 (sha256:4f468e00efa86e6e33e8fa4f14deca93ee15b955071920facd56c59c3d2fefb8) +// Input contract: 0.1.0 (sha256:ab4b109d99077a8b5af98923b77d8fc1b707f7ee9ad08304e26d497ccb3799f6) // Options: workdir-operation-schema-closure,openapi-3.1,strict-integer-ranges,sorted-output // Output: web/workspace/src/lib/generated/workdir-api.ts // Regenerate/check: cargo run -q -p server-api --example generate_workdir_api_types -- --check diff --git a/web/workspace/src/lib/generated/worker-launch-api.ts b/web/workspace/src/lib/generated/worker-launch-api.ts index 86e6acc57..33f0a9d0a 100644 --- a/web/workspace/src/lib/generated/worker-launch-api.ts +++ b/web/workspace/src/lib/generated/worker-launch-api.ts @@ -1,6 +1,6 @@ // @generated by yoi-worker-openapi-typescript@1. Do not edit. // Input: openapi/server-api.json -// Input contract: 0.1.0 (sha256:4f468e00efa86e6e33e8fa4f14deca93ee15b955071920facd56c59c3d2fefb8) +// Input contract: 0.1.0 (sha256:ab4b109d99077a8b5af98923b77d8fc1b707f7ee9ad08304e26d497ccb3799f6) // Options: worker-operation-schema-closure,openapi-3.1,strict-integer-ranges,sorted-output // Output: web/workspace/src/lib/generated/worker-launch-api.ts // Regenerate/check: cargo run -q -p server-api --example generate_worker_launch_api_types -- --check diff --git a/web/workspace/src/routes/w/[workspaceId]/tickets/[ticketId]/+page.svelte b/web/workspace/src/routes/w/[workspaceId]/tickets/[ticketId]/+page.svelte index 155a1fe9b..a0f792fbe 100644 --- a/web/workspace/src/routes/w/[workspaceId]/tickets/[ticketId]/+page.svelte +++ b/web/workspace/src/routes/w/[workspaceId]/tickets/[ticketId]/+page.svelte @@ -361,7 +361,10 @@ if ( await mutate("state", "/state", { state: nextState, - reason: transitionReason.trim() || null, + operation_key: crypto.randomUUID(), + expected_item_revision: ticket.item_revision, + expected_state: ticket.state, + reason: transitionReason.trim() || "Progress decision from the Ticket page", }) ) transitionReason = ""; } @@ -381,7 +384,12 @@ event.preventDefault(); if (!resolution.trim()) return; if ( - await mutate("close", "/close", { resolution: resolution.trim() }) + await mutate("close", "/close", { + resolution: resolution.trim(), + operation_key: crypto.randomUUID(), + expected_item_revision: ticket.item_revision, + expected_state: ticket.state, + }) ) resolution = ""; }