diff --git a/crates/ticket/src/lib.rs b/crates/ticket/src/lib.rs index d4912cec..0cd51910 100644 --- a/crates/ticket/src/lib.rs +++ b/crates/ticket/src/lib.rs @@ -1147,6 +1147,19 @@ pub fn ticket_queue_guard( 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) @@ -1186,7 +1199,10 @@ fn derive_ticket_workspace_projection( if !relation_blockers.is_empty() { let active_blockers = relation_blockers .iter() - .filter(|blocker| !relation_blocker_allows_ready_queue(blocker)) + .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() { @@ -1225,6 +1241,15 @@ fn derive_ticket_workspace_projection( .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, @@ -1233,7 +1258,8 @@ fn derive_ticket_workspace_projection( visible_overlay: None, disabled_reason: None, key_hint: Some(format!( - "Queue records orchestration demand; dependency relations remain orchestration context ({blockers})." + "Queue targets: {}; active dependencies remain orchestration context ({blockers}).", + queue_targets.join(", ") )), blocked_reason: Some(blockers), queue_guard: TicketQueueGuard { @@ -4441,14 +4467,66 @@ impl TicketBackend for LocalTicketBackend { } fn dependency_check(&self, id: TicketIdOrSlug) -> Result { - let ticket = self.show(id)?; - let summary = ticket_summary_from_meta(ticket.meta.clone()); - let projection = project_ticket_workspace_item(&summary, &ticket.relations.blockers, None); + let requested_dir = self.find_ticket_dir(&id)?; + let requested_ticket = ticket_id_from_dir(&requested_dir)?; + let mut states = HashMap::new(); + for dir in self.iter_ticket_dirs(TicketListQuery::all())? { + let item = dir.join("item.md"); + let meta = ticket_meta_for_dir(&dir, read_item_file(&item)?.frontmatter)?; + states.insert(meta.id, meta.workflow_state); + } + let relations = self.all_ticket_relation_records()?; + let blockers = transitive_dependency_blockers(&requested_ticket, &states, &relations)?; + let ticket = self.ticket_from_dir(&requested_dir)?; + 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(&requested_ticket, &states, &relations) { + Ok(queue_tickets) => { + let target_error = queue_tickets.iter().find_map(|candidate| { + self.find_ticket_dir(&TicketIdOrSlug::Id(candidate.clone())) + .and_then(|dir| self.ticket_from_dir(&dir)) + .and_then(|ticket| { + match resolve_ready_target( + self.target_authority.as_ref(), + "local", + &ticket, + ) { + Ok(_) => Ok(()), + Err(TicketError::TargetAuthorityUnavailable) => Ok(()), + Err(error) => Err(error), + } + }) + .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() + }; Ok(TicketDependencyCheck { ticket: summary, - blockers: ticket.relations.blockers, + blockers, queue_guard: projection.queue_guard, - queue_tickets: Vec::new(), + queue_tickets, recommended_action: projection .next_action .unwrap_or(TicketWorkspaceNextAction::WaitForOrchestrator), @@ -7693,6 +7771,35 @@ state: planning .unwrap(); } + let check = backend + .dependency_check(TicketIdOrSlug::Id(first.id.clone())) + .unwrap(); + assert!(!check.queue_guard.can_queue_for_orchestrator); + assert!( + check + .queue_guard + .blocked_reason + .as_deref() + .unwrap_or_default() + .contains("cycle") + ); + let page = backend + .list_workspace_projection_page(SqliteTicketListPageQuery { + states: vec![TicketWorkflowState::Ready], + limit: 10, + after: None, + }) + .unwrap(); + let item = page + .items + .iter() + .find(|item| item.summary.id == check.ticket.id) + .unwrap(); + assert!( + !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(); diff --git a/crates/tui/src/dashboard/mod.rs b/crates/tui/src/dashboard/mod.rs index d1803727..42780f1a 100644 --- a/crates/tui/src/dashboard/mod.rs +++ b/crates/tui/src/dashboard/mod.rs @@ -4201,17 +4201,32 @@ async fn dispatch_panel_queue( "root-ticket-state-after-orchestration-merge", &preflight.root_top_level, )?; - backend + let queue_outcome = backend .queue_ready(TicketIdOrSlug::Id(ticket_id.to_owned()), "workspace-panel") .map_err(|error| TicketActionError::Ticket(error.to_string()))?; + let expected_queue_tickets = preflight + .queue_tickets + .iter() + .cloned() + .collect::>(); + let actual_queue_tickets = queue_outcome + .queued_tickets + .iter() + .cloned() + .collect::>(); + if actual_queue_tickets != expected_queue_tickets { + return Err(TicketActionError::Stale(format!( + "Queue dependency plan changed after confirmation for Ticket {ticket_id}; reload and retry" + ))); + } let commit = commit_panel_queue_ticket_record(&preflight)?; let sync = sync_panel_queue_to_orchestration(&preflight, &commit)?; verify_panel_queue_synced(&preflight, &commit)?; let notification = notify_workspace_orchestrator(orchestrator, current_ticket).await; Ok(TicketActionOutcome { notice: format!( - "Queued Ticket {}; root Queue commit {}; {}; orchestration sync {}; {}. Orchestrator routing is authorized; implementation side effects still require queued -> inprogress acceptance.", - ticket_id, + "Queued Ticket closure [{}]; root Queue commit {}; {}; orchestration sync {}; {}. Orchestrator routing is authorized; implementation side effects still require queued -> inprogress acceptance.", + queue_outcome.queued_tickets.join(", "), commit.sha, root_merge.sentence(), sync.sentence(), @@ -4225,7 +4240,8 @@ struct PanelQueueHandoffPreflight { ticket_id: String, root_top_level: PathBuf, orchestration: OrchestrationWorktreeLayout, - ticket_record_dir: PathBuf, + queue_tickets: Vec, + ticket_record_dirs: Vec, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -4393,27 +4409,53 @@ fn prepare_panel_queue_handoff( &root_top_level, )?; - let ticket_record_dir = backend.root().join(ticket_id); - if !ticket_record_dir.join("item.md").is_file() { + let dependency_check = backend + .dependency_check(TicketIdOrSlug::Id(ticket_id.to_owned())) + .map_err(|error| TicketActionError::Ticket(error.to_string()))?; + if !dependency_check.queue_guard.can_queue_for_orchestrator { return Err(queue_check_failed( - "target-ticket-record", + "dependency-queue-plan", ticket_id, - &ticket_record_dir, - "target Ticket item.md is missing".to_string(), + &root_top_level, + dependency_check + .queue_guard + .blocked_reason + .or(dependency_check.queue_guard.reason) + .unwrap_or_else(|| "Queue dependency validation failed".to_string()), )); } - ensure_git_path_clean( - "root-ticket-clean", - ticket_id, - &root_top_level, - &ticket_record_dir, - )?; + let queue_tickets = dependency_check.queue_tickets; + let mut ticket_record_dirs = Vec::with_capacity(queue_tickets.len()); + for queue_ticket in &queue_tickets { + let ticket_record_dir = backend.root().join(queue_ticket); + if !ticket_record_dir.join("item.md").is_file() { + return Err(queue_check_failed( + "target-ticket-record", + &queue_ticket, + &ticket_record_dir, + "Queue Ticket item.md is missing".to_string(), + )); + } + let clean_stage = if queue_ticket == ticket_id { + "root-ticket-clean" + } else { + "queue-dependency-clean" + }; + ensure_git_path_clean( + clean_stage, + &queue_ticket, + &root_top_level, + &ticket_record_dir, + )?; + ticket_record_dirs.push(ticket_record_dir); + } Ok(PanelQueueHandoffPreflight { ticket_id: ticket_id.to_string(), root_top_level, orchestration, - ticket_record_dir, + queue_tickets, + ticket_record_dirs, }) } @@ -4504,37 +4546,36 @@ fn sync_orchestration_to_root_before_queue( fn commit_panel_queue_ticket_record( preflight: &PanelQueueHandoffPreflight, ) -> Result { - let ticket_rel = path_relative_to_root( - &preflight.root_top_level, - &preflight.ticket_record_dir, - "target-ticket-record", - &preflight.ticket_id, - )?; + let ticket_rels = preflight + .ticket_record_dirs + .iter() + .map(|ticket_record_dir| { + path_relative_to_root( + &preflight.root_top_level, + ticket_record_dir, + "target-ticket-record", + &preflight.ticket_id, + ) + }) + .collect::, _>>()?; let mut add = Command::new("git"); add.arg("-C") .arg(&preflight.root_top_level) .arg("add") .arg("--") - .arg(&ticket_rel); - run_git_command(add, "stage Queue Ticket record").map_err(|message| { + .args(&ticket_rels); + run_git_command(add, "stage Queue Ticket records").map_err(|message| { queue_check_failed( "queue-commit-stage", &preflight.ticket_id, - &preflight.ticket_record_dir, + &preflight.root_top_level, message, ) })?; - let ticket_rel_string = git_path_string(&ticket_rel); let staged = git_capture( &preflight.root_top_level, - &[ - "diff", - "--cached", - "--name-only", - "--", - ticket_rel_string.as_str(), - ], + &["diff", "--cached", "--name-only"], "list staged Queue Ticket files", ) .map_err(|message| { @@ -4545,19 +4586,31 @@ fn commit_panel_queue_ticket_record( message, ) })?; + let allowed = ticket_rels + .iter() + .map(|path| format!("{}/", git_path_string(path).trim_end_matches('/'))) + .collect::>(); let staged_paths = staged .lines() .filter(|line| !line.trim().is_empty()) .collect::>(); - if staged_paths.is_empty() { + if staged_paths.is_empty() + || staged_paths + .iter() + .any(|path| !allowed.iter().any(|root| path.starts_with(root))) + { return Err(queue_check_failed( "queue-commit-pathscope", &preflight.ticket_id, - &preflight.ticket_record_dir, - "Queue mutation produced no staged Ticket record changes".to_string(), + &preflight.root_top_level, + "Queue mutation staged no Ticket records or included files outside the confirmed dependency closure" + .to_string(), )); } - let message = format!("ticket: queue {}", preflight.ticket_id); + let message = format!( + "chore: queue Ticket dependency closure {}", + preflight.ticket_id + ); let mut commit = Command::new("git"); commit .arg("-C") @@ -4567,8 +4620,8 @@ fn commit_panel_queue_ticket_record( .arg("-m") .arg(message) .arg("--") - .arg(&ticket_rel); - run_git_command(commit, "commit Queue Ticket record").map_err(|message| { + .args(&ticket_rels); + run_git_command(commit, "commit Queue Ticket records").map_err(|message| { queue_check_failed( "queue-commit-create", &preflight.ticket_id, diff --git a/crates/tui/src/workspace_panel.rs b/crates/tui/src/workspace_panel.rs index bcbfea10..fa2dcc40 100644 --- a/crates/tui/src/workspace_panel.rs +++ b/crates/tui/src/workspace_panel.rs @@ -2290,12 +2290,7 @@ mod tests { .unwrap_or_default() .contains(&dependency.id) ); - assert!( - row.key_hint - .as_deref() - .unwrap() - .contains("dependency relations remain orchestration context") - ); + assert!(row.key_hint.as_deref().unwrap().contains("Queue targets:")); assert!(row.key_hint.as_deref().unwrap().contains(&dependency.id)); } diff --git a/crates/workspace-server/src/authority.rs b/crates/workspace-server/src/authority.rs index 506b3022..16f9e0f8 100644 --- a/crates/workspace-server/src/authority.rs +++ b/crates/workspace-server/src/authority.rs @@ -892,6 +892,7 @@ impl SqliteWorkspaceAuthority { && !has_coder && has_target && !has_blockers, + queue_tickets: dependency_check.queue_tickets.clone(), blockers: action_blockers, }; let merge_request = match self.merge_request_store.get(&self.workspace_id, id) { diff --git a/crates/workspace-server/src/records.rs b/crates/workspace-server/src/records.rs index e78022d2..91b85100 100644 --- a/crates/workspace-server/src/records.rs +++ b/crates/workspace-server/src/records.rs @@ -296,6 +296,7 @@ pub struct TicketActionEligibility { pub can_unassign_orchestrator: bool, pub can_queue: bool, pub can_start_manual_coder: bool, + pub queue_tickets: Vec, pub blockers: Vec, } diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index c2ebf05c..3ab662e9 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -1,4 +1,4 @@ -use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; +use std::collections::{BTreeMap, HashMap, HashSet}; use std::path::{Component, Path, PathBuf}; use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::{Arc, Mutex, Weak}; @@ -3951,39 +3951,6 @@ fn reject_unguarded_ticket_completion(operation: &TicketBackendOperation) -> Res Ok(()) } -fn queue_assignment_candidates( - backend: &dyn TicketBackend, - requested_ticket_id: &str, -) -> ticket::Result> { - fn visit( - backend: &dyn TicketBackend, - ticket_id: &str, - visited: &mut BTreeSet, - ready: &mut BTreeSet, - ) -> ticket::Result<()> { - if !visited.insert(ticket_id.to_owned()) { - return Ok(()); - } - let ticket = backend.show(TicketIdOrSlug::Id(ticket_id.to_owned()))?; - if ticket.meta.workflow_state == TicketWorkflowState::Ready { - ready.insert(ticket.meta.id.clone()); - } - for blocker in ticket.relations.blockers { - visit(backend, &blocker.blocking_ticket, visited, ready)?; - } - Ok(()) - } - - let mut ready = BTreeSet::new(); - visit( - backend, - requested_ticket_id, - &mut BTreeSet::new(), - &mut ready, - )?; - Ok(ready.into_iter().collect()) -} - async fn execute_ticket_rest_operation( api: &WorkspaceApi, workspace_id: &str, @@ -4021,8 +3988,18 @@ async fn execute_ticket_rest_operation( .to_string(), ) })?; - let candidates = - queue_assignment_candidates(&backend, &ticket.meta.id).map_err(Error::from)?; + 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 @@ -16491,6 +16468,59 @@ mod tests { assert!(event.attributes.contains_key("routing_request_fingerprint")); } + #[tokio::test] + async fn queue_reports_dependency_cycle_before_assignment_validation() { + 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 first_input = ticket::NewTicket::new("First cycle Ticket"); + first_input.workflow_state = Some(TicketWorkflowState::Ready); + first_input.repository_id = Some(TEST_REPOSITORY_ID.to_string()); + first_input.ref_selector = Some("develop".to_string()); + let first = backend.create(first_input).unwrap(); + let mut second_input = ticket::NewTicket::new("Second cycle Ticket"); + second_input.workflow_state = Some(TicketWorkflowState::Ready); + second_input.repository_id = Some(TEST_REPOSITORY_ID.to_string()); + second_input.ref_selector = Some("develop".to_string()); + let second = backend.create(second_input).unwrap(); + for (ticket_id, target) in [ + (first.id.clone(), second.id.clone()), + (second.id.clone(), first.id.clone()), + ] { + backend + .add_ticket_relation( + ticket_id.into(), + ticket::NewTicketRelation { + kind: ticket::TicketRelationKind::DependsOn, + target, + note: None, + author: Some("test".to_string()), + }, + ) + .unwrap(); + } + + let error = execute_ticket_rest_operation( + &api, + TEST_WORKSPACE_ID, + HeaderMap::new(), + TicketBackendOperation::QueueReady { + id: TicketIdOrSlug::Id(first.id.clone()), + queued_by: "workspace-web".to_string(), + }, + ) + .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 + ); + } + } + #[tokio::test] async fn queue_requires_assignments_for_every_ready_dependency() { let dir = tempfile::tempdir().unwrap(); diff --git a/web/workspace/src/lib/generated/ticket-api.ts b/web/workspace/src/lib/generated/ticket-api.ts index ee66b0d6..f9dae989 100644 --- a/web/workspace/src/lib/generated/ticket-api.ts +++ b/web/workspace/src/lib/generated/ticket-api.ts @@ -21,7 +21,7 @@ export type TicketRoleAssignmentSummary = { assignment_id: string, role: string, export type TicketAssignmentPrincipalSummary = { "kind": "user", account_id: string, } | { "kind": "worker", runtime_id: string, worker_id: string, } | { "kind": "workspace_agent", agent_key: string, }; -export type TicketActionEligibility = { can_assign_orchestrator: boolean, can_unassign_orchestrator: boolean, can_queue: boolean, can_start_manual_coder: boolean, blockers: Array, }; +export type TicketActionEligibility = { can_assign_orchestrator: boolean, can_unassign_orchestrator: boolean, can_queue: boolean, can_start_manual_coder: boolean, queue_tickets: Array, blockers: Array, }; export type TicketMergeRequestSummary = { merge_request_id: string, repository_id: string, state: string, review_status: string, selector_from: string | null, selector_to: string, updated_at: string, current_subject_ref: string | null, review_subject_ref: string | null, review_requested_at: string | null, review_submitted_at: string | null, review_excerpt: string | null, }; diff --git a/web/workspace/src/lib/workspace/tickets/ticket-panel.test.ts b/web/workspace/src/lib/workspace/tickets/ticket-panel.test.ts index a9259428..4a5ee65f 100644 --- a/web/workspace/src/lib/workspace/tickets/ticket-panel.test.ts +++ b/web/workspace/src/lib/workspace/tickets/ticket-panel.test.ts @@ -120,10 +120,9 @@ Deno.test("ticket detail uses server-derived role assignment actions", async () assertEquals(source.includes("ticket.action_eligibility.can_queue"), true); assertEquals(source.includes("ticket.relations.blockers.length > 0"), true); - assertEquals( - source.includes("Queue records orchestration demand. Dependency relations remain visible"), - true, - ); + assertEquals(source.includes("ticket.action_eligibility.queue_tickets"), true); + assertEquals(source.includes("This operation queues:"), true); + assertEquals(source.includes("outcome.queued_tickets.join"), true); assertEquals( source.includes("resolve the listed blockers before Queue"), false, 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 63095682..653931a6 100644 --- a/web/workspace/src/routes/w/[workspaceId]/tickets/[ticketId]/+page.svelte +++ b/web/workspace/src/routes/w/[workspaceId]/tickets/[ticketId]/+page.svelte @@ -38,6 +38,11 @@ if (!loadedTicket) throw new Error(initialData.ticket.error ?? "ticket load failed"); const loadedRepositories = initialData.repositories.data; + type QueueOutcome = { + requested_ticket: string; + queued_tickets: string[]; + }; + let ticket = $state(loadedTicket); const mergeRequest = $derived(ticket.merge_request); let editing = $state(false); @@ -52,6 +57,7 @@ let resolution = $state(""); let busy = $state(null); let errorMessage = $state(null); + let queueMessage = $state(null); let readyOperationKey = $state(null); let manualRuntimeId = $state(""); let manualWorkerId = $state(""); @@ -117,6 +123,25 @@ } } + async function queueTicket(): Promise { + if (busy) return; + busy = "queue"; + errorMessage = null; + queueMessage = null; + try { + const outcome = await workspaceApiJsonWithBody( + `${ticketPath}/queue`, + { method: "POST", body: JSON.stringify({}) }, + ); + queueMessage = `Queued ${outcome.queued_tickets.length} Ticket(s): ${outcome.queued_tickets.join(", ")}`; + applyTicket(await workspaceApiJson(ticketPath)); + } catch (error) { + errorMessage = error instanceof Error ? error.message : String(error); + } finally { + busy = null; + } + } + async function mutateAssignment( action: string, role: "orchestrator" | "coder", @@ -272,6 +297,10 @@ {/if} + {#if queueMessage} +
{queueMessage}
+ {/if} + {#if editing}
@@ -462,13 +491,16 @@

Choose a healthy repository and an effective ref selector before marking ready.

{/if} {:else if ticket.state === "ready"} - {#if !ticket.action_eligibility.can_queue}

Queue requires a valid target, an active Orchestrator assignment, no active Coder assignment, and no dependency still in planning.

- {:else if ticket.relations.blockers.length > 0} -

Ready dependencies are queued atomically. Queued or in-progress dependencies remain unchanged for the Orchestrator to schedule.

+ {:else if ticket.action_eligibility.queue_tickets.length > 0} +

This operation queues: {ticket.action_eligibility.queue_tickets.join(", ")}.

+ {#if ticket.relations.blockers.length > 0} +

Ready dependencies are queued atomically. Queued or in-progress dependencies remain unchanged for the Orchestrator to schedule.

+ {/if} {/if} {/if}