From 379ae214fc128d7ef80f3ad7e6d2b74037cc91d5 Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 24 Aug 2026 13:40:33 +0900 Subject: [PATCH 1/9] feat: queue tickets with dependency context --- crates/ticket/src/lib.rs | 139 ++++++------------ crates/ticket/src/tool.rs | 2 +- crates/tui/src/dashboard/render.rs | 2 +- crates/tui/src/dashboard/tests.rs | 15 +- crates/tui/src/workspace_panel.rs | 27 ++-- crates/workspace-server/src/authority.rs | 14 +- crates/workspace-server/src/server.rs | 13 +- .../workspace/tickets/ticket-panel.test.ts | 9 ++ .../tickets/[ticketId]/+page.svelte | 4 +- 9 files changed, 101 insertions(+), 124 deletions(-) diff --git a/crates/ticket/src/lib.rs b/crates/ticket/src/lib.rs index 452ab0ae..5e5a6a2f 100644 --- a/crates/ticket/src/lib.rs +++ b/crates/ticket/src/lib.rs @@ -1117,7 +1117,7 @@ pub fn project_ticket_workspace_item( pub fn ticket_queue_guard( summary: &TicketSummary, - relation_blockers: &[TicketRelationBlocker], + _relation_blockers: &[TicketRelationBlocker], orchestration_overlay: Option<&TicketWorkspaceStateOverlay>, ) -> TicketQueueGuard { if orchestration_overlay.is_some() { @@ -1140,18 +1140,6 @@ pub fn ticket_queue_guard( blocked_reason: None, }; } - let active_blockers = relation_blockers - .iter() - .filter(|blocker| !relation_blocker_allows_ready_queue(blocker)) - .collect::>(); - if !active_blockers.is_empty() { - let blockers = format_workspace_relation_blockers(&active_blockers); - return TicketQueueGuard { - can_queue_for_orchestrator: false, - reason: Some(format!("waiting for {blockers}")), - blocked_reason: Some(blockers), - }; - } TicketQueueGuard { can_queue_for_orchestrator: true, reason: None, @@ -1168,7 +1156,7 @@ fn derive_ticket_workspace_projection( .iter() .filter(|blocker| !relation_blocker_allows_ready_queue(blocker)) .collect::>(); - if !active_blockers.is_empty() || summary.workflow_state != TicketWorkflowState::Ready { + if summary.workflow_state != TicketWorkflowState::Ready { let blockers_to_report = if active_blockers.is_empty() { relation_blockers.iter().collect::>() } else { @@ -1188,9 +1176,9 @@ fn derive_ticket_workspace_projection( visible_state: summary.workflow_state.as_str().to_string(), visible_overlay: None, disabled_reason: Some(format!( - "Queue disabled: {waiting_reason}. Resolve dependency/blocker before ready -> queued." + "Dependency context: {waiting_reason}. The Orchestrator decides whether work waits or starts in parallel." )), - key_hint: Some(format!("Gate: {waiting_reason}")), + key_hint: Some(format!("Dependencies: {waiting_reason}")), blocked_reason: Some(blockers), queue_guard: TicketQueueGuard { can_queue_for_orchestrator: false, @@ -1213,9 +1201,9 @@ fn derive_ticket_workspace_projection( visible_overlay: None, disabled_reason: None, key_hint: Some(format!( - "Queue allowed: prerequisites are already queued/in progress; Orchestrator will preserve order ({blockers})." + "Queue records orchestration demand; dependency relations remain scheduling context ({blockers})." )), - blocked_reason: None, + blocked_reason: Some(blockers), queue_guard: TicketQueueGuard { can_queue_for_orchestrator: true, reason: None, @@ -3921,16 +3909,6 @@ impl TicketBackend for SqliteTicketBackend { &self.workspace_id, &ticket, )?; - let blockers = ticket - .relations - .blockers - .iter() - .filter(|blocker| !relation_blocker_allows_queue(blocker)) - .cloned() - .collect::>(); - if !blockers.is_empty() { - return Err(TicketError::BlockingRelations(format_relation_blockers(&blockers))); - } let at = now_utc(); conn.execute("UPDATE typed_tickets SET workflow_state = 'queued', workflow_state_explicit = 1, queued_by = ?3, queued_at = ?4, repository_id = ?5, ref_selector = ?6, updated_at = ?4 WHERE workspace_id = ?1 AND ticket_id = ?2 AND workflow_state = 'ready'", params![self.workspace_id, ticket_id, queued_by, at, target.repository_id, target.ref_selector]).map_err(sqlite_err)?; 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: BTreeMap::from([("queued_by".to_owned(), queued_by.to_owned()), ("queued_at".to_owned(), at), ("repository_id".to_owned(), target.repository_id), ("ref_selector".to_owned(), target.ref_selector)]) }) @@ -4566,18 +4544,6 @@ impl TicketBackend for LocalTicketBackend { } let ticket = self.ticket_from_dir(&dir)?; let target = resolve_ready_target(self.target_authority.as_ref(), "local", &ticket)?; - let blockers = self.relation_blockers_for_meta(&meta)?; - let active_blockers = blockers - .into_iter() - .filter(|blocker| !relation_blocker_allows_queue(blocker)) - .collect::>(); - if !active_blockers.is_empty() { - return Err(TicketError::BlockingRelations(format!( - "{}: {}", - meta.id, - format_relation_blockers(&active_blockers) - ))); - } let at = now_utc(); let mut change = TicketStateChange::new( TicketWorkflowState::Ready.as_str(), @@ -6945,7 +6911,7 @@ mod tests { } #[test] - fn workspace_projection_blocks_ready_queue_on_unstarted_dependency() { + fn workspace_projection_queues_ready_ticket_with_unstarted_dependency_context() { let summary = summary_with_state(TicketWorkflowState::Ready); let blockers = [blocker_with_state(TicketWorkflowState::Planning)]; let projection = project_ticket_workspace_item(&summary, &blockers, None); @@ -6953,15 +6919,22 @@ 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()); + assert!( + projection + .key_hint + .as_deref() + .unwrap_or_default() + .contains("scheduling context") + ); } #[test] - fn workspace_projection_allows_ready_queue_when_dependency_is_already_queued() { + fn workspace_projection_queues_ready_ticket_with_queued_dependency_context() { let summary = summary_with_state(TicketWorkflowState::Ready); let blockers = [blocker_with_state(TicketWorkflowState::Queued)]; let projection = project_ticket_workspace_item(&summary, &blockers, None); @@ -6971,13 +6944,13 @@ mod tests { Some(TicketWorkspaceNextAction::QueueForOrchestrator) ); assert!(projection.queue_guard.can_queue_for_orchestrator); - assert!(projection.blocked_reason.is_none()); + assert!(projection.blocked_reason.is_some()); assert!( projection .key_hint .as_deref() .unwrap_or_default() - .contains("Orchestrator will preserve order") + .contains("scheduling context") ); } @@ -7238,7 +7211,7 @@ state: planning } #[test] - fn sqlite_mark_ready_and_queue_enforce_target_and_blockers_atomically() { + fn sqlite_mark_ready_and_queue_preserve_dependency_context_atomically() { let tmp = TempDir::new().unwrap(); let backend = SqliteTicketBackend::open(tmp.path().join("workspace.db"), "workspace-test") .unwrap() @@ -7284,43 +7257,17 @@ state: planning .count(), 1 ); - assert!(matches!( - backend.queue_ready( - TicketIdOrSlug::Id(implementation.id.clone()), - "orchestrator", - ), - Err(TicketError::BlockingRelations(_)) - )); - let after_rejection = backend - .show(TicketIdOrSlug::Id(implementation.id.clone())) - .unwrap(); - assert_eq!( - after_rejection.meta.workflow_state, - TicketWorkflowState::Ready - ); - assert!(!after_rejection.events.iter().any(|event| { - event.from.as_deref() == Some("ready") && event.to.as_deref() == Some("queued") - })); - backend - .close( - TicketIdOrSlug::Id(dependency.id), - MarkdownText::new("resolved"), - ) - .unwrap(); backend .queue_ready( TicketIdOrSlug::Id(implementation.id.clone()), "orchestrator", ) .unwrap(); - assert_eq!( - backend - .show(TicketIdOrSlug::Id(implementation.id)) - .unwrap() - .meta - .workflow_state, - TicketWorkflowState::Queued - ); + let queued = backend.show(TicketIdOrSlug::Id(implementation.id)).unwrap(); + assert_eq!(queued.meta.workflow_state, TicketWorkflowState::Queued); + assert_eq!(queued.meta.queued_by.as_deref(), Some("orchestrator")); + assert_eq!(queued.relations.blockers.len(), 1); + assert_eq!(queued.relations.blockers[0].blocking_ticket, dependency.id); } #[test] @@ -8374,7 +8321,7 @@ state: planning } #[test] - fn queue_gate_rejects_unresolved_dependency_and_incoming_blocker() { + fn queue_accepts_unresolved_dependency_and_incoming_blocker_as_context() { let tmp = TempDir::new().unwrap(); let backend = backend(&tmp); let mut blocked_input = NewTicket::new("Blocked Ready"); @@ -8392,12 +8339,15 @@ state: planning }, ) .unwrap(); - let err = backend + backend .queue_ready(TicketIdOrSlug::Id(blocked.id.clone()), "test") - .unwrap_err() - .to_string(); - assert!(err.contains("unresolved blocking relation"), "{err}"); - assert!(err.contains(&dependency.id), "{err}"); + .unwrap(); + let queued = backend + .show(TicketIdOrSlug::Id(blocked.id.clone())) + .unwrap(); + assert_eq!(queued.meta.workflow_state, TicketWorkflowState::Queued); + assert_eq!(queued.relations.blockers.len(), 1); + assert_eq!(queued.relations.blockers[0].blocking_ticket, dependency.id); let mut incoming_input = NewTicket::new("Incoming Blocked Ready"); incoming_input.workflow_state = Some(TicketWorkflowState::Ready); @@ -8414,12 +8364,21 @@ state: planning }, ) .unwrap(); - let err = backend + backend .queue_ready(TicketIdOrSlug::Id(incoming.id.clone()), "test") - .unwrap_err() - .to_string(); - assert!(err.contains("unresolved blocking relation"), "{err}"); - assert!(err.contains(&blocker.id), "{err}"); + .unwrap(); + let queued_incoming = backend + .show(TicketIdOrSlug::Id(incoming.id.clone())) + .unwrap(); + assert_eq!( + queued_incoming.meta.workflow_state, + TicketWorkflowState::Queued + ); + assert_eq!(queued_incoming.relations.blockers.len(), 1); + assert_eq!( + queued_incoming.relations.blockers[0].blocking_ticket, + blocker.id + ); } #[test] diff --git a/crates/ticket/src/tool.rs b/crates/ticket/src/tool.rs index 4781a0bf..d8763657 100644 --- a/crates/ticket/src/tool.rs +++ b/crates/ticket/src/tool.rs @@ -143,7 +143,7 @@ The backend applies the same target validation and lock as TicketMarkReady and c state_changed event, effective target, and planning -> ready transition atomically."; const QUEUE_DESCRIPTION: &str = "Queue a ready Ticket for Orchestrator routing through the typed \ Ticket backend. The backend performs the gated ready -> queued transition, records queued_by/queued_at, \ -and rejects unresolved blocking relations."; +and preserves unresolved blocking relations as Orchestrator scheduling context rather than Queue admission gates."; 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 \ diff --git a/crates/tui/src/dashboard/render.rs b/crates/tui/src/dashboard/render.rs index f2a5f78e..417620cd 100644 --- a/crates/tui/src/dashboard/render.rs +++ b/crates/tui/src/dashboard/render.rs @@ -462,7 +462,7 @@ pub(super) fn panel_ticket_detail(row: &PanelRow) -> String { .as_ref() .and_then(|ticket| ticket.blocked_reason.as_deref()) { - parts.push(format!("Gate: waiting for {blocked_reason}")); + parts.push(format!("Dependencies: {blocked_reason}")); } else { parts.push("Gate: clear".to_string()); } diff --git a/crates/tui/src/dashboard/tests.rs b/crates/tui/src/dashboard/tests.rs index d6381100..24f32f3e 100644 --- a/crates/tui/src/dashboard/tests.rs +++ b/crates/tui/src/dashboard/tests.rs @@ -1846,24 +1846,23 @@ fn panel_orchestration_overlay_uses_compact_status_column_and_detail_line() { } #[test] -fn ready_ticket_with_waiting_gate_shows_queue_disabled_reason() { +fn ready_ticket_with_dependency_context_keeps_queue_action_available() { let mut row = panel_test_ticket_row( "00001WAITING", - "Ready but gated", - ActionPriority::Background, - NextUserAction::Wait, + "Ready with dependency context", + ActionPriority::ReadyForQueue, + NextUserAction::Queue, "ready", ); - row.disabled_reason = Some("Queue disabled: waiting for BLOCKER-1".to_string()); row.ticket.as_mut().unwrap().blocked_reason = Some("BLOCKER-1 via depends_on".to_string()); let lines = panel_row_lines(&row, true, 160); let detail = &lines[1]; let detail_line = plain_line(&detail); - assert!(detail_line.contains("Gate: waiting for BLOCKER-1 via depends_on")); - assert!(detail_line.contains("Action: queue disabled")); - assert!(detail_line.contains("Reason: Queue disabled: waiting for BLOCKER-1")); + assert!(detail_line.contains("Dependencies: BLOCKER-1 via depends_on")); + assert!(detail_line.contains("Action: Queue")); + assert!(!detail_line.contains("Queue disabled")); } #[test] diff --git a/crates/tui/src/workspace_panel.rs b/crates/tui/src/workspace_panel.rs index 92a84f6f..1400fff4 100644 --- a/crates/tui/src/workspace_panel.rs +++ b/crates/tui/src/workspace_panel.rs @@ -2203,7 +2203,7 @@ mod tests { } #[test] - fn workspace_panel_marks_ready_ticket_with_unresolved_relation_waiting_gate() { + fn workspace_panel_queues_ready_ticket_with_unresolved_relation_context() { let temp = TempDir::new().unwrap(); write_ticket_config(temp.path()); let backend = LocalTicketBackend::new(temp.path().join(".yoi/tickets")); @@ -2233,14 +2233,9 @@ mod tests { .unwrap(); assert_eq!(row.kind, PanelRowKind::Ticket); - assert_eq!(row.next_action, Some(NextUserAction::Wait)); - assert_eq!(row.priority, ActionPriority::Background); - assert!( - row.disabled_reason - .as_deref() - .unwrap() - .contains("Queue disabled: waiting for") - ); + assert_eq!(row.next_action, Some(NextUserAction::Queue)); + assert_eq!(row.priority, ActionPriority::ReadyForQueue); + assert!(row.disabled_reason.is_none()); assert!( row.ticket .as_ref() @@ -2253,7 +2248,7 @@ mod tests { } #[test] - fn workspace_panel_allows_ready_ticket_when_relation_prerequisite_is_queued() { + fn workspace_panel_queues_ready_ticket_when_relation_prerequisite_is_queued() { let temp = TempDir::new().unwrap(); write_ticket_config(temp.path()); let backend = LocalTicketBackend::new(temp.path().join(".yoi/tickets")); @@ -2286,12 +2281,20 @@ mod tests { assert_eq!(row.next_action, Some(NextUserAction::Queue)); assert_eq!(row.priority, ActionPriority::ReadyForQueue); assert!(row.disabled_reason.is_none()); - assert!(row.ticket.as_ref().unwrap().blocked_reason.is_none()); + assert!( + row.ticket + .as_ref() + .unwrap() + .blocked_reason + .as_deref() + .unwrap_or_default() + .contains(&dependency.id) + ); assert!( row.key_hint .as_deref() .unwrap() - .contains("Queue allowed: prerequisites are already queued/in progress") + .contains("dependency relations remain scheduling context") ); 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 f15dd440..66b91ddb 100644 --- a/crates/workspace-server/src/authority.rs +++ b/crates/workspace-server/src/authority.rs @@ -847,20 +847,16 @@ impl SqliteWorkspaceAuthority { can_queue: ticket.meta.workflow_state == TicketWorkflowState::Ready && has_orchestrator && !has_coder - && has_target - && !has_blockers, + && has_target, can_start_manual_coder: ticket.meta.workflow_state == TicketWorkflowState::Ready && !has_orchestrator && !has_coder && has_target && !has_blockers, - blockers: [ - (!has_target).then_some("Ticket target is required".to_string()), - has_blockers.then_some("unresolved blocking relations remain".to_string()), - ] - .into_iter() - .flatten() - .collect(), + blockers: [(!has_target).then_some("Ticket target is required".to_string())] + .into_iter() + .flatten() + .collect(), }; let merge_request = match self.merge_request_store.get(&self.workspace_id, id) { Ok(request) => { diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index dc8d2b58..1dfca8a4 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -17210,7 +17210,7 @@ mod tests { .add_ticket_relation( ticket_id.clone().into(), ticket::NewTicketRelation { - kind: ticket::TicketRelationKind::Related, + kind: ticket::TicketRelationKind::DependsOn, target: related_ticket_id.clone(), note: Some("Browser relation".to_string()), author: Some("browser-user".to_string()), @@ -17244,7 +17244,7 @@ mod tests { assert!(edited.assignment_diagnostics.is_empty()); assert_eq!(edited.relations.outgoing.len(), 1); assert_eq!(edited.relations.outgoing[0].target, related_ticket_id); - assert_eq!(edited.relations.outgoing[0].kind, "related"); + assert_eq!(edited.relations.outgoing[0].kind, "depends_on"); let Json(commented) = scoped_append_ticket_event( State(api.clone()), @@ -17274,6 +17274,13 @@ mod tests { .unwrap(); assert_eq!(ready.state, "ready"); assign_test_orchestrator(&api, &ticket_id); + let Json(ready_detail) = scoped_get_ticket(State(api.clone()), AxumPath(path())) + .await + .unwrap(); + assert!(ready_detail.action_eligibility.can_queue); + assert!(ready_detail.action_eligibility.blockers.is_empty()); + assert_eq!(ready_detail.relations.blockers.len(), 1); + assert_eq!(ready_detail.relations.blockers[0].reason_kind, "depends_on"); let Json(queued) = scoped_queue_ticket( State(api.clone()), @@ -17284,6 +17291,8 @@ mod tests { .unwrap(); assert_eq!(queued.state, "queued"); assert_eq!(queued.queued_by.as_deref(), Some("workspace-web")); + assert_eq!(queued.relations.blockers.len(), 1); + assert_eq!(queued.relations.blockers[0].reason_kind, "depends_on"); let Json(closed) = scoped_close_ticket( State(api), AxumPath(path()), 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 a58f507d..a9259428 100644 --- a/web/workspace/src/lib/workspace/tickets/ticket-panel.test.ts +++ b/web/workspace/src/lib/workspace/tickets/ticket-panel.test.ts @@ -119,6 +119,15 @@ 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("resolve the listed blockers before Queue"), + false, + ); assertEquals( source.includes("ticket.action_eligibility.can_assign_orchestrator"), true, 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 a0afba90..97a3e263 100644 --- a/web/workspace/src/routes/w/[workspaceId]/tickets/[ticketId]/+page.svelte +++ b/web/workspace/src/routes/w/[workspaceId]/tickets/[ticketId]/+page.svelte @@ -466,7 +466,9 @@ {busy === "queue" ? "Queueing…" : "Queue ticket"} {#if !ticket.action_eligibility.can_queue} -

Assign the Orchestrator role and resolve the listed blockers before Queue.

+

Queue requires a valid target, an active Orchestrator assignment, and no active Coder assignment.

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

Queue records orchestration demand. Dependency relations remain visible so the Orchestrator can decide whether to wait or start work in parallel.

{/if} {/if} From d57b4d1d5ef1642e8bb5f22eb65e053e8739c4a3 Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 24 Aug 2026 13:42:39 +0900 Subject: [PATCH 2/9] fix: align ticket queue projection test --- crates/workspace-server/src/authority.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/workspace-server/src/authority.rs b/crates/workspace-server/src/authority.rs index 66b91ddb..3ea7decc 100644 --- a/crates/workspace-server/src/authority.rs +++ b/crates/workspace-server/src/authority.rs @@ -3047,7 +3047,10 @@ 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); From f079479160b88255ab10191a6d8fb2238e9e0097 Mon Sep 17 00:00:00 2001 From: Hare Date: Tue, 25 Aug 2026 10:53:31 +0900 Subject: [PATCH 3/9] feat: queue ready dependency closures atomically --- crates/client/src/workspace_product.rs | 8 +- crates/ticket/src/lib.rs | 597 +++++++++++++++--- crates/ticket/src/tool.rs | 26 +- crates/tui/src/workspace_panel.rs | 10 +- crates/worker/src/feature/builtin/ticket.rs | 27 +- crates/workspace-server/src/authority.rs | 5 +- crates/workspace-server/src/server.rs | 198 +++++- .../tickets/[ticketId]/+page.svelte | 4 +- 8 files changed, 733 insertions(+), 142 deletions(-) diff --git a/crates/client/src/workspace_product.rs b/crates/client/src/workspace_product.rs index b892c210..46207451 100644 --- a/crates/client/src/workspace_product.rs +++ b/crates/client/src/workspace_product.rs @@ -473,8 +473,12 @@ impl TicketBackend for BackendWorkspaceProductClient { .map_err(ticket_client_error) } - fn queue_ready(&self, id: TicketIdOrSlug, _queued_by: &str) -> ticket::Result<()> { - self.send_unit::<()>( + fn queue_ready( + &self, + id: TicketIdOrSlug, + _queued_by: &str, + ) -> ticket::Result { + self.send_json::<(), _>( Method::POST, &format!( "/tickets/{}/workflow/queue", diff --git a/crates/ticket/src/lib.rs b/crates/ticket/src/lib.rs index 5e5a6a2f..21cf9b5a 100644 --- a/crates/ticket/src/lib.rs +++ b/crates/ticket/src/lib.rs @@ -807,6 +807,12 @@ pub struct TicketDependencyCheck { pub recommended_action: TicketWorkspaceNextAction, } +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct TicketQueueOutcome { + pub requested_ticket: String, + pub queued_tickets: Vec, +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] #[serde(rename_all = "lowercase")] pub enum TicketListState { @@ -1117,7 +1123,7 @@ pub fn project_ticket_workspace_item( pub fn ticket_queue_guard( summary: &TicketSummary, - _relation_blockers: &[TicketRelationBlocker], + relation_blockers: &[TicketRelationBlocker], orchestration_overlay: Option<&TicketWorkspaceStateOverlay>, ) -> TicketQueueGuard { if orchestration_overlay.is_some() { @@ -1140,9 +1146,34 @@ pub fn ticket_queue_guard( blocked_reason: None, }; } + 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: None, + reason: (!relation_blockers.is_empty()).then(|| { + "Ready dependencies will be queued atomically; active dependencies remain unchanged" + .to_string() + }), blocked_reason: None, } } @@ -1156,7 +1187,7 @@ fn derive_ticket_workspace_projection( .iter() .filter(|blocker| !relation_blocker_allows_ready_queue(blocker)) .collect::>(); - if summary.workflow_state != TicketWorkflowState::Ready { + if summary.workflow_state != TicketWorkflowState::Ready || !active_blockers.is_empty() { let blockers_to_report = if active_blockers.is_empty() { relation_blockers.iter().collect::>() } else { @@ -1201,7 +1232,7 @@ fn derive_ticket_workspace_projection( visible_overlay: None, disabled_reason: None, key_hint: Some(format!( - "Queue records orchestration demand; dependency relations remain scheduling context ({blockers})." + "Queue records orchestration demand; dependency relations remain orchestration context ({blockers})." )), blocked_reason: Some(blockers), queue_guard: TicketQueueGuard { @@ -1409,7 +1440,7 @@ fn compact_ticket_state_label(state: TicketWorkflowState) -> &'static str { fn relation_blocker_allows_ready_queue(blocker: &TicketRelationBlocker) -> bool { matches!( blocker.blocking_state, - TicketWorkflowState::Queued | TicketWorkflowState::InProgress + TicketWorkflowState::Ready | TicketWorkflowState::Queued | TicketWorkflowState::InProgress ) } @@ -1733,7 +1764,7 @@ pub trait TicketBackend { ) -> Result<()>; fn set_workflow_state(&self, id: TicketIdOrSlug, change: TicketStateChange) -> Result<()>; fn mark_ready(&self, id: TicketIdOrSlug, request: TicketMarkReady) -> Result; - fn queue_ready(&self, id: TicketIdOrSlug, queued_by: &str) -> Result<()>; + fn queue_ready(&self, id: TicketIdOrSlug, queued_by: &str) -> Result; fn close(&self, id: TicketIdOrSlug, resolution: MarkdownText) -> Result<()>; fn add_ticket_relation( &self, @@ -1856,6 +1887,7 @@ pub enum TicketBackendOperationResult { Ticket(Ticket), TicketRef(TicketRef), DependencyCheck(TicketDependencyCheck), + QueueOutcome(TicketQueueOutcome), Relation(TicketRelation), Relations(Vec), RelationView(TicketRelationView), @@ -1916,8 +1948,7 @@ where TicketBackendOperationResult::Ticket(backend.mark_ready(id, request)?) } TicketBackendOperation::QueueReady { id, queued_by } => { - backend.queue_ready(id, &queued_by)?; - TicketBackendOperationResult::Unit + TicketBackendOperationResult::QueueOutcome(backend.queue_ready(id, &queued_by)?) } TicketBackendOperation::Close { id, resolution } => { backend.close(id, resolution)?; @@ -3893,25 +3924,90 @@ impl TicketBackend for SqliteTicketBackend { }) } - fn queue_ready(&self, id: TicketIdOrSlug, queued_by: &str) -> Result<()> { + fn queue_ready(&self, id: TicketIdOrSlug, queued_by: &str) -> Result { validate_required_event_value("queued_by", queued_by)?; self.with_write(|conn| { - let ticket_id = self.resolve_ticket_id(conn, id)?; - let ticket = self.load_ticket(conn, &ticket_id)?; - if ticket.meta.workflow_state != TicketWorkflowState::Ready { - return Err(TicketError::StaleWorkflowState { - expected: TicketWorkflowState::Ready.as_str().to_owned(), - actual: ticket.meta.workflow_state.as_str().to_owned(), - }); + 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(), + )); + } } - let target = resolve_ready_target( - self.target_authority.as_ref(), - &self.workspace_id, - &ticket, - )?; + + let mut targets = Vec::with_capacity(queued_tickets.len()); + for ticket_id in &queued_tickets { + let ticket = self.load_ticket(conn, ticket_id)?; + let target = resolve_ready_target( + self.target_authority.as_ref(), + &self.workspace_id, + &ticket, + )?; + targets.push((ticket_id.clone(), target)); + } + let at = now_utc(); - conn.execute("UPDATE typed_tickets SET workflow_state = 'queued', workflow_state_explicit = 1, queued_by = ?3, queued_at = ?4, repository_id = ?5, ref_selector = ?6, updated_at = ?4 WHERE workspace_id = ?1 AND ticket_id = ?2 AND workflow_state = 'ready'", params![self.workspace_id, ticket_id, queued_by, at, target.repository_id, target.ref_selector]).map_err(sqlite_err)?; - 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: BTreeMap::from([("queued_by".to_owned(), queued_by.to_owned()), ("queued_at".to_owned(), at), ("repository_id".to_owned(), target.repository_id), ("ref_selector".to_owned(), target.ref_selector)]) }) + for (ticket_id, target) in targets { + let updated = conn.execute( + "UPDATE typed_tickets SET workflow_state = 'queued', workflow_state_explicit = 1, queued_by = ?3, queued_at = ?4, repository_id = ?5, ref_selector = ?6, updated_at = ?4 WHERE workspace_id = ?1 AND ticket_id = ?2 AND workflow_state = 'ready'", + params![self.workspace_id, ticket_id, queued_by, at, target.repository_id, target.ref_selector], + ).map_err(sqlite_err)?; + if updated != 1 { + return Err(TicketError::Conflict(format!( + "Ticket {ticket_id} changed while the dependency queue plan was being applied" + ))); + } + let mut attributes = BTreeMap::from([ + ("queued_by".to_owned(), queued_by.to_owned()), + ("queued_at".to_owned(), at.clone()), + ("repository_id".to_owned(), target.repository_id), + ("ref_selector".to_owned(), target.ref_selector), + ("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, + })?; + } + + Ok(TicketQueueOutcome { + requested_ticket, + queued_tickets, + }) }) } @@ -4530,40 +4626,56 @@ impl TicketBackend for LocalTicketBackend { self.ticket_from_dir(&dir) } - fn queue_ready(&self, id: TicketIdOrSlug, queued_by: &str) -> Result<()> { + fn queue_ready(&self, id: TicketIdOrSlug, queued_by: &str) -> Result { validate_required_event_value("queued_by", queued_by)?; let _lock = self.acquire_lock()?; - let dir = self.find_ticket_dir(&id)?; - let item = dir.join("item.md"); - let meta = ticket_meta_for_dir(&dir, read_item_file(&item)?.frontmatter)?; - if meta.workflow_state != TicketWorkflowState::Ready { - return Err(TicketError::StaleWorkflowState { - expected: TicketWorkflowState::Ready.as_str().to_owned(), - actual: meta.workflow_state.as_str().to_owned(), - }); + 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 ticket = self.ticket_from_dir(&dir)?; - let target = resolve_ready_target(self.target_authority.as_ref(), "local", &ticket)?; - let at = now_utc(); - let mut change = TicketStateChange::new( - TicketWorkflowState::Ready.as_str(), - TicketWorkflowState::Queued.as_str(), - "queued", - self.queued_ready_body(queued_by), - ); - change.author = Some(queued_by.to_string()); - self.apply_workflow_state_change( - &dir, - TicketWorkflowState::Ready, - TicketWorkflowState::Queued, - change, - &[ - ("queued_by", queued_by), - ("queued_at", at.as_str()), - ("repository_id", target.repository_id.as_str()), - ("ref_selector", target.ref_selector.as_str()), - ], - ) + let relations = self.all_ticket_relation_records()?; + let queued_tickets = dependency_queue_plan(&requested_ticket, &states, &relations)?; + + let mut planned = Vec::with_capacity(queued_tickets.len()); + for ticket_id in &queued_tickets { + let dir = self.find_ticket_dir(&TicketIdOrSlug::Id(ticket_id.clone()))?; + let ticket = self.ticket_from_dir(&dir)?; + let target = resolve_ready_target(self.target_authority.as_ref(), "local", &ticket)?; + planned.push((dir, target)); + } + + for (dir, target) in planned { + let at = now_utc(); + let mut change = TicketStateChange::new( + TicketWorkflowState::Ready.as_str(), + TicketWorkflowState::Queued.as_str(), + "queued", + self.queued_ready_body(queued_by), + ); + change.author = Some(queued_by.to_string()); + self.apply_workflow_state_change( + &dir, + TicketWorkflowState::Ready, + TicketWorkflowState::Queued, + change, + &[ + ("queued_by", queued_by), + ("queued_at", at.as_str()), + ("repository_id", target.repository_id.as_str()), + ("ref_selector", target.ref_selector.as_str()), + ("queue_root_ticket", requested_ticket.as_str()), + ], + )?; + } + + Ok(TicketQueueOutcome { + requested_ticket, + queued_tickets, + }) } fn close(&self, id: TicketIdOrSlug, resolution: MarkdownText) -> Result<()> { @@ -5395,6 +5507,142 @@ fn ticket_state_resolved(state: TicketWorkflowState) -> bool { ) } +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, + prerequisites: &BTreeMap>, + marks: &mut BTreeMap, + stack: &mut Vec, + ordered: &mut Vec, + ) -> Result<()> { + 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, 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, + &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], @@ -6892,6 +7140,86 @@ mod tests { } } + fn dependency_relation(ticket_id: &str, target: &str) -> TicketRelation { + TicketRelation { + ticket_id: ticket_id.to_owned(), + kind: TicketRelationKind::DependsOn, + target: target.to_owned(), + note: None, + author: "test".to_owned(), + at: String::new(), + } + } + + #[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 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"), + ]; + + assert_eq!( + dependency_queue_plan("root", &states, &relations).unwrap(), + vec!["root"] + ); + } + + #[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); @@ -6911,7 +7239,7 @@ mod tests { } #[test] - fn workspace_projection_queues_ready_ticket_with_unstarted_dependency_context() { + fn workspace_projection_blocks_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); @@ -6919,18 +7247,11 @@ mod tests { assert_eq!(projection.kind, TicketWorkspaceRowKind::Ticket); assert_eq!( projection.next_action, - Some(TicketWorkspaceNextAction::QueueForOrchestrator) + Some(TicketWorkspaceNextAction::WaitForOrchestrator) ); - 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_none()); - assert!( - projection - .key_hint - .as_deref() - .unwrap_or_default() - .contains("scheduling context") - ); + assert!(projection.disabled_reason.is_some()); } #[test] @@ -6950,7 +7271,7 @@ mod tests { .key_hint .as_deref() .unwrap_or_default() - .contains("scheduling context") + .contains("orchestration context") ); } @@ -7211,13 +7532,104 @@ state: planning } #[test] - fn sqlite_mark_ready_and_queue_preserve_dependency_context_atomically() { + fn sqlite_queue_cycle_diagnostic_leaves_all_tickets_ready() { + let temp = TempDir::new().unwrap(); + let backend = SqliteTicketBackend::open(temp.path().join("tickets.db"), "workspace-test") + .unwrap() + .with_target_authority(Arc::new(TestTargetAuthority)); + let mut first_input = NewTicket::new("First ready Ticket"); + first_input.workflow_state = Some(TicketWorkflowState::Ready); + first_input.repository_id = Some("main".to_string()); + let first = backend.create(first_input).unwrap(); + let mut second_input = NewTicket::new("Second ready Ticket"); + second_input.workflow_state = Some(TicketWorkflowState::Ready); + second_input.repository_id = Some("main".to_string()); + let second = backend.create(second_input).unwrap(); + for (ticket, target) in [ + (first.id.clone(), second.id.clone()), + (second.id.clone(), first.id.clone()), + ] { + backend + .add_ticket_relation( + TicketIdOrSlug::Id(ticket), + NewTicketRelation { + kind: TicketRelationKind::DependsOn, + target, + note: None, + author: None, + }, + ) + .unwrap(); + } + + 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] { + assert_eq!( + backend + .show(TicketIdOrSlug::Id(ticket_id)) + .unwrap() + .meta + .workflow_state, + TicketWorkflowState::Ready + ); + } + } + + #[test] + fn sqlite_queue_target_failure_rolls_back_entire_ready_closure() { + let temp = TempDir::new().unwrap(); + let backend = SqliteTicketBackend::open(temp.path().join("tickets.db"), "workspace-test") + .unwrap() + .with_target_authority(Arc::new(TestTargetAuthority)); + let mut dependency_input = NewTicket::new("Invalid ready dependency"); + dependency_input.workflow_state = Some(TicketWorkflowState::Ready); + dependency_input.repository_id = Some("unknown".to_string()); + let dependency = backend.create(dependency_input).unwrap(); + let mut root_input = NewTicket::new("Queue root"); + root_input.workflow_state = Some(TicketWorkflowState::Ready); + root_input.repository_id = Some("main".to_string()); + let root = backend.create(root_input).unwrap(); + backend + .add_ticket_relation( + TicketIdOrSlug::Id(root.id.clone()), + NewTicketRelation { + kind: TicketRelationKind::DependsOn, + target: dependency.id.clone(), + note: None, + author: None, + }, + ) + .unwrap(); + + 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] { + assert_eq!( + backend + .show(TicketIdOrSlug::Id(ticket_id)) + .unwrap() + .meta + .workflow_state, + TicketWorkflowState::Ready + ); + } + } + + #[test] + fn sqlite_queue_atomically_queues_ready_dependency_closure() { let tmp = TempDir::new().unwrap(); let backend = SqliteTicketBackend::open(tmp.path().join("workspace.db"), "workspace-test") .unwrap() .with_target_authority(Arc::new(TestTargetAuthority)); let mut dependency = NewTicket::new("Dependency"); dependency.repository_id = Some("main".to_owned()); + dependency.workflow_state = Some(TicketWorkflowState::Ready); let dependency = backend.create(dependency).unwrap(); let mut implementation = NewTicket::new("Implementation"); implementation.repository_id = Some("main".to_owned()); @@ -7257,14 +7669,28 @@ state: planning .count(), 1 ); - backend + let outcome = backend .queue_ready( TicketIdOrSlug::Id(implementation.id.clone()), "orchestrator", ) .unwrap(); - let queued = backend.show(TicketIdOrSlug::Id(implementation.id)).unwrap(); + assert_eq!(outcome.requested_ticket, implementation.id); + assert_eq!( + outcome.queued_tickets, + vec![dependency.id.clone(), implementation.id.clone()] + ); + let queued = backend + .show(TicketIdOrSlug::Id(implementation.id.clone())) + .unwrap(); + let queued_dependency = backend + .show(TicketIdOrSlug::Id(dependency.id.clone())) + .unwrap(); assert_eq!(queued.meta.workflow_state, TicketWorkflowState::Queued); + assert_eq!( + queued_dependency.meta.workflow_state, + TicketWorkflowState::Queued + ); assert_eq!(queued.meta.queued_by.as_deref(), Some("orchestrator")); assert_eq!(queued.relations.blockers.len(), 1); assert_eq!(queued.relations.blockers[0].blocking_ticket, dependency.id); @@ -8321,7 +8747,7 @@ state: planning } #[test] - fn queue_accepts_unresolved_dependency_and_incoming_blocker_as_context() { + fn queue_rejects_planning_dependency_and_incoming_blocker_without_mutation() { let tmp = TempDir::new().unwrap(); let backend = backend(&tmp); let mut blocked_input = NewTicket::new("Blocked Ready"); @@ -8339,15 +8765,19 @@ state: planning }, ) .unwrap(); - backend + let error = backend .queue_ready(TicketIdOrSlug::Id(blocked.id.clone()), "test") - .unwrap(); - let queued = backend + .unwrap_err(); + assert!(matches!(error, TicketError::BlockingRelations(_))); + let unchanged = backend .show(TicketIdOrSlug::Id(blocked.id.clone())) .unwrap(); - assert_eq!(queued.meta.workflow_state, TicketWorkflowState::Queued); - assert_eq!(queued.relations.blockers.len(), 1); - assert_eq!(queued.relations.blockers[0].blocking_ticket, dependency.id); + assert_eq!(unchanged.meta.workflow_state, TicketWorkflowState::Ready); + assert_eq!(unchanged.relations.blockers.len(), 1); + assert_eq!( + unchanged.relations.blockers[0].blocking_ticket, + dependency.id + ); let mut incoming_input = NewTicket::new("Incoming Blocked Ready"); incoming_input.workflow_state = Some(TicketWorkflowState::Ready); @@ -8364,19 +8794,20 @@ state: planning }, ) .unwrap(); - backend + let error = backend .queue_ready(TicketIdOrSlug::Id(incoming.id.clone()), "test") - .unwrap(); - let queued_incoming = backend + .unwrap_err(); + assert!(matches!(error, TicketError::BlockingRelations(_))); + let unchanged_incoming = backend .show(TicketIdOrSlug::Id(incoming.id.clone())) .unwrap(); assert_eq!( - queued_incoming.meta.workflow_state, - TicketWorkflowState::Queued + unchanged_incoming.meta.workflow_state, + TicketWorkflowState::Ready ); - assert_eq!(queued_incoming.relations.blockers.len(), 1); + assert_eq!(unchanged_incoming.relations.blockers.len(), 1); assert_eq!( - queued_incoming.relations.blockers[0].blocking_ticket, + unchanged_incoming.relations.blockers[0].blocking_ticket, blocker.id ); } diff --git a/crates/ticket/src/tool.rs b/crates/ticket/src/tool.rs index d8763657..f6fb2842 100644 --- a/crates/ticket/src/tool.rs +++ b/crates/ticket/src/tool.rs @@ -142,8 +142,8 @@ const INTAKE_READY_DESCRIPTION: &str = "Record a bounded intake summary and mark The backend applies the same target validation and lock as TicketMarkReady and commits the summary, \ state_changed event, effective target, and planning -> ready transition atomically."; const QUEUE_DESCRIPTION: &str = "Queue a ready Ticket for Orchestrator routing through the typed \ -Ticket backend. The backend performs the gated ready -> queued transition, records queued_by/queued_at, \ -and preserves unresolved blocking relations as Orchestrator scheduling context rather than Queue admission gates."; +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 \ @@ -316,7 +316,11 @@ impl TicketBackend for TicketToolBackend { self.backend.mark_ready(id, request) } - fn queue_ready(&self, id: TicketIdOrSlug, queued_by: &str) -> TicketResult<()> { + fn queue_ready( + &self, + id: TicketIdOrSlug, + queued_by: &str, + ) -> TicketResult { self.backend.queue_ready(id, queued_by) } @@ -1219,12 +1223,22 @@ impl Tool for TicketQueueTool { ) -> Result { let params: TicketQueueParams = parse_input("TicketQueue", input_json)?; let queued_by = default_author(); - self.backend + let outcome = self + .backend .queue_ready(TicketIdOrSlug::Query(params.ticket.clone()), &queued_by) .map_err(|error| backend_error("TicketQueue", error))?; Ok(json_output( - format!("Queued ticket {} for Orchestrator", params.ticket), - json!({ "ticket": params.ticket, "state": "queued", "queued_by": queued_by, "ok": true }), + format!( + "Queued {} ticket(s) for Orchestrator", + outcome.queued_tickets.len() + ), + json!({ + "ticket": outcome.requested_ticket, + "queued_tickets": outcome.queued_tickets, + "state": "queued", + "queued_by": queued_by, + "ok": true + }), )) } } diff --git a/crates/tui/src/workspace_panel.rs b/crates/tui/src/workspace_panel.rs index 1400fff4..bcbfea10 100644 --- a/crates/tui/src/workspace_panel.rs +++ b/crates/tui/src/workspace_panel.rs @@ -2203,7 +2203,7 @@ mod tests { } #[test] - fn workspace_panel_queues_ready_ticket_with_unresolved_relation_context() { + fn workspace_panel_blocks_ready_ticket_with_planning_relation() { let temp = TempDir::new().unwrap(); write_ticket_config(temp.path()); let backend = LocalTicketBackend::new(temp.path().join(".yoi/tickets")); @@ -2233,9 +2233,9 @@ mod tests { .unwrap(); assert_eq!(row.kind, PanelRowKind::Ticket); - assert_eq!(row.next_action, Some(NextUserAction::Queue)); - assert_eq!(row.priority, ActionPriority::ReadyForQueue); - assert!(row.disabled_reason.is_none()); + assert_eq!(row.next_action, Some(NextUserAction::Wait)); + assert_eq!(row.priority, ActionPriority::Background); + assert!(row.disabled_reason.is_some()); assert!( row.ticket .as_ref() @@ -2294,7 +2294,7 @@ mod tests { row.key_hint .as_deref() .unwrap() - .contains("dependency relations remain scheduling context") + .contains("dependency relations remain orchestration context") ); assert!(row.key_hint.as_deref().unwrap().contains(&dependency.id)); } diff --git a/crates/worker/src/feature/builtin/ticket.rs b/crates/worker/src/feature/builtin/ticket.rs index 6a70263b..9001d6bb 100644 --- a/crates/worker/src/feature/builtin/ticket.rs +++ b/crates/worker/src/feature/builtin/ticket.rs @@ -873,12 +873,13 @@ impl WorkspaceHttpTicketBackend { })?), ) .map(TicketBackendOperationResult::Ticket), - TicketBackendOperation::QueueReady { id, .. } => Self::request_unit( + TicketBackendOperation::QueueReady { id, .. } => Self::request( client, WorkspaceRequestMethod::Post, format!("{base}/{}/workflow/queue", Self::ticket_path(&id)), None, - ), + ) + .map(TicketBackendOperationResult::QueueOutcome), TicketBackendOperation::Close { id, resolution } => Self::request_unit( client, WorkspaceRequestMethod::Post, @@ -1099,16 +1100,18 @@ impl TicketBackend for WorkspaceHttpTicketBackend { ) } - fn queue_ready(&self, id: TicketIdOrSlug, queued_by: &str) -> TicketResult<()> { - match self.invoke(TicketBackendOperation::QueueReady { - id, - queued_by: queued_by.to_string(), - })? { - TicketBackendOperationResult::Unit => Ok(()), - other => Err(TicketError::Conflict(format!( - "unexpected ticket backend response: {other:?}" - ))), - } + fn queue_ready( + &self, + id: TicketIdOrSlug, + queued_by: &str, + ) -> TicketResult { + expect_ticket_result!( + self.invoke(TicketBackendOperation::QueueReady { + id, + queued_by: queued_by.to_string(), + }), + TicketBackendOperationResult::QueueOutcome + ) } fn close(&self, id: TicketIdOrSlug, resolution: MarkdownText) -> TicketResult<()> { diff --git a/crates/workspace-server/src/authority.rs b/crates/workspace-server/src/authority.rs index 3ea7decc..66b91ddb 100644 --- a/crates/workspace-server/src/authority.rs +++ b/crates/workspace-server/src/authority.rs @@ -3047,10 +3047,7 @@ 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, - "ready_for_queue" - ); + assert_eq!(tickets.items[0].workspace_action_priority, "background"); let ticket_by_key = authority.ticket(&tickets.items[0].resource_key).unwrap(); assert_eq!(ticket_by_key.id, tickets.items[0].id); diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index 1dfca8a4..5fdbb4d9 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -1,4 +1,4 @@ -use std::collections::{BTreeMap, HashMap, HashSet}; +use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; use std::path::{Component, Path, PathBuf}; use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::{Arc, Mutex, Weak}; @@ -3951,6 +3951,39 @@ 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, @@ -3988,27 +4021,34 @@ async fn execute_ticket_rest_operation( .to_string(), ) })?; - let assignment = active_orchestrator_assignment(api, workspace_id, &ticket.meta.id)? - .ok_or_else(|| { - Error::TicketAssignmentConflict( - "Queue requires role=orchestrator assignment to workspace-orchestrator" - .to_string(), - ) - })?; + let candidates = + queue_assignment_candidates(&backend, &ticket.meta.id).map_err(Error::from)?; + let mut assignment_ids = BTreeMap::new(); + for ticket_id in candidates { + 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); + } + let assignment_json = serde_json::to_string(&assignment_ids).map_err(|error| { + Error::Config(format!("failed to encode Queue assignment fence: {error}")) + })?; let operation_id = new_id("tqueue"); let fingerprint = Sha256::digest(format!( - "ticket-queue:v1\0{workspace_id}\0{}\0{}\0{}", + "ticket-queue:v2\0{workspace_id}\0{}\0{}\0{assignment_json}", ticket.meta.id, ticket.meta.workflow_state.as_str(), - assignment.assignment_id )) .iter() .map(|byte| format!("{byte:02x}")) .collect::(); event_attributes.extend([ ( - "orchestrator_assignment_id".to_string(), - assignment.assignment_id, + "queue_orchestrator_assignments".to_string(), + assignment_json, ), ( "routing_principal".to_string(), @@ -4029,18 +4069,30 @@ async fn execute_ticket_rest_operation( } let result = execute_ticket_backend_operation(&backend, operation).map_err(Error::from)?; - if is_mutation - && let Some(target) = target - && let Ok(ticket) = backend.show(target) - { - notify_ticket_recipients( - api, - workspace_id, - &ticket.meta.id, - &previous_state, - ticket.meta.workflow_state.as_str(), - source, - ); + if is_mutation { + if let TicketBackendOperationResult::QueueOutcome(outcome) = &result { + for ticket_id in &outcome.queued_tickets { + notify_ticket_recipients( + api, + workspace_id, + ticket_id, + TicketWorkflowState::Ready.as_str(), + TicketWorkflowState::Queued.as_str(), + source.clone(), + ); + } + } else if let Some(target) = target + && let Ok(ticket) = backend.show(target) + { + notify_ticket_recipients( + api, + workspace_id, + &ticket.meta.id, + &previous_state, + ticket.meta.workflow_state.as_str(), + source, + ); + } } Ok(result) } @@ -4364,7 +4416,7 @@ async fn scoped_queue_ticket_record( State(api): State, AxumPath((workspace_id, id)): AxumPath<(String, String)>, headers: HeaderMap, -) -> ApiResult { +) -> ApiResult> { let result = execute_ticket_rest_operation( &api, &workspace_id, @@ -4375,7 +4427,10 @@ async fn scoped_queue_ticket_record( }, ) .await?; - ticket_rest_unit(result) + ticket_rest_result(result, |result| match result { + TicketBackendOperationResult::QueueOutcome(outcome) => Some(outcome), + _ => None, + }) } #[derive(Debug, serde::Deserialize)] @@ -16404,7 +16459,7 @@ mod tests { ); assign_test_orchestrator(&api, &ticket.id); - scoped_queue_ticket_record(State(api.clone()), AxumPath(path), HeaderMap::new()) + let _ = scoped_queue_ticket_record(State(api.clone()), AxumPath(path), HeaderMap::new()) .await .unwrap(); let queued = backend.show(ticket.id.into()).unwrap(); @@ -16422,6 +16477,88 @@ mod tests { assert!(event.attributes.contains_key("routing_request_fingerprint")); } + #[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); + dependency_input.repository_id = Some(TEST_REPOSITORY_ID.to_string()); + dependency_input.ref_selector = Some("develop".to_string()); + let dependency = backend.create(dependency_input).unwrap(); + let mut root_input = ticket::NewTicket::new("Queue root"); + root_input.workflow_state = Some(TicketWorkflowState::Ready); + root_input.repository_id = Some(TEST_REPOSITORY_ID.to_string()); + root_input.ref_selector = Some("develop".to_string()); + 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()) + .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] async fn queued_ticket_mutation_succeeds_without_orchestrator() { let dir = tempfile::tempdir().unwrap(); @@ -17200,9 +17337,13 @@ mod tests { workspace_id: TEST_WORKSPACE_ID.to_string(), id: ticket_id.clone(), }; + let mut related_input = ticket::NewTicket::new("Related Browser Ticket"); + related_input.workflow_state = Some(TicketWorkflowState::Ready); + related_input.repository_id = Some(TEST_REPOSITORY_ID.to_string()); + related_input.ref_selector = Some("develop".to_string()); let related_ticket_id = browser_ticket_backend(&api) .unwrap() - .create(ticket::NewTicket::new("Related Browser Ticket")) + .create(related_input) .unwrap() .id; browser_ticket_backend(&api) @@ -17274,6 +17415,7 @@ mod tests { .unwrap(); assert_eq!(ready.state, "ready"); assign_test_orchestrator(&api, &ticket_id); + assign_test_orchestrator(&api, &related_ticket_id); let Json(ready_detail) = scoped_get_ticket(State(api.clone()), AxumPath(path())) .await .unwrap(); 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 97a3e263..63095682 100644 --- a/web/workspace/src/routes/w/[workspaceId]/tickets/[ticketId]/+page.svelte +++ b/web/workspace/src/routes/w/[workspaceId]/tickets/[ticketId]/+page.svelte @@ -466,9 +466,9 @@ {busy === "queue" ? "Queueing…" : "Queue ticket"} {#if !ticket.action_eligibility.can_queue} -

Queue requires a valid target, an active Orchestrator assignment, and no active Coder assignment.

+

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} -

Queue records orchestration demand. Dependency relations remain visible so the Orchestrator can decide whether to wait or start work in parallel.

+

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

{/if} {/if} From cabe38db1d38cdce6c55bac42ae996d498d367c9 Mon Sep 17 00:00:00 2001 From: Hare Date: Tue, 25 Aug 2026 11:19:27 +0900 Subject: [PATCH 4/9] fix: align queue projections with dependency closure --- crates/ticket/src/lib.rs | 341 ++++++++++++++++------- crates/workspace-server/src/authority.rs | 74 ++++- crates/workspace-server/src/server.rs | 20 +- 3 files changed, 326 insertions(+), 109 deletions(-) diff --git a/crates/ticket/src/lib.rs b/crates/ticket/src/lib.rs index 21cf9b5a..d4912cec 100644 --- a/crates/ticket/src/lib.rs +++ b/crates/ticket/src/lib.rs @@ -804,6 +804,7 @@ pub struct TicketDependencyCheck { pub ticket: TicketSummary, pub blockers: Vec, pub queue_guard: TicketQueueGuard, + pub queue_tickets: Vec, pub recommended_action: TicketWorkspaceNextAction, } @@ -2810,97 +2811,17 @@ impl SqliteTicketBackend { conn: &Connection, summaries: &[TicketSummary], ) -> Result>> { - let listed_ids = summaries + let states = self.state_index(conn)?; + let relations = self.all_relations(conn)?; + summaries .iter() - .map(|summary| summary.id.as_str()) - .collect::>(); - let mut statement = conn - .prepare( - "SELECT relation.ticket_id, relation.kind, relation.target, relation.note, - source.workflow_state, target.workflow_state - FROM typed_ticket_relations AS relation - LEFT JOIN typed_tickets AS source - ON source.workspace_id = relation.workspace_id - AND source.ticket_id = relation.ticket_id - LEFT JOIN typed_tickets AS target - ON target.workspace_id = relation.workspace_id - AND target.ticket_id = relation.target - WHERE relation.workspace_id = ?1 - AND relation.kind IN ('depends_on', 'blocks') - AND EXISTS ( - SELECT 1 FROM json_each(?2) AS listed - WHERE listed.value = relation.ticket_id - OR listed.value = relation.target - )", - ) - .map_err(sqlite_err)?; - let listed_ids_json = serde_json::to_string( - &summaries - .iter() - .map(|summary| summary.id.as_str()) - .collect::>(), - ) - .map_err(|error| TicketError::Sqlite(error.to_string()))?; - let rows = statement - .query_map(params![self.workspace_id, listed_ids_json], |row| { + .map(|summary| { Ok(( - row.get::<_, String>(0)?, - row.get::<_, String>(1)?, - row.get::<_, String>(2)?, - row.get::<_, Option>(3)?, - row.get::<_, Option>(4)?, - row.get::<_, Option>(5)?, + summary.id.clone(), + transitive_dependency_blockers(&summary.id, &states, &relations)?, )) }) - .map_err(sqlite_err)?; - let mut blockers = HashMap::>::new(); - for row in rows { - let (source, kind, target, note, source_state, target_state) = - row.map_err(sqlite_err)?; - let (listed_ticket, blocking_ticket, reason_kind, relation_kind, blocking_state) = - match kind.as_str() { - "depends_on" if listed_ids.contains(source.as_str()) => ( - source, - target, - "depends_on", - TicketRelationKind::DependsOn, - target_state, - ), - "blocks" if listed_ids.contains(target.as_str()) => ( - target, - source, - "blocked_by", - TicketRelationKind::Blocks, - source_state, - ), - _ => continue, - }; - let blocking_state = blocking_state - .as_deref() - .and_then(TicketWorkflowState::parse) - .unwrap_or(TicketWorkflowState::Planning); - if ticket_state_resolved(blocking_state) { - continue; - } - blockers - .entry(listed_ticket) - .or_default() - .push(TicketRelationBlocker { - blocking_ticket, - reason_kind: reason_kind.to_string(), - relation_kind, - note, - blocking_state, - }); - } - for ticket_blockers in blockers.values_mut() { - ticket_blockers.sort_by(|a, b| { - a.reason_kind - .cmp(&b.reason_kind) - .then_with(|| a.blocking_ticket.cmp(&b.blocking_ticket)) - }); - } - Ok(blockers) + .collect() } pub fn import_from_local_backend(&self, local: &LocalTicketBackend) -> Result<()> { @@ -3716,17 +3637,71 @@ impl TicketBackend for SqliteTicketBackend { } fn dependency_check(&self, id: TicketIdOrSlug) -> Result { - let ticket = self.show(id)?; - let blockers = ticket.relations.blockers.clone(); - let summary = ticket_summary_from_meta(ticket.meta.clone()); - let projection = project_ticket_workspace_item(&summary, &blockers, None); - Ok(TicketDependencyCheck { - ticket: summary, - blockers, - queue_guard: projection.queue_guard, - recommended_action: projection - .next_action - .unwrap_or(TicketWorkspaceNextAction::WaitForOrchestrator), + self.with_read(|conn| { + let ticket_id = self.resolve_ticket_id(conn, id)?; + let ticket = self.load_ticket(conn, &ticket_id)?; + let states = self.state_index(conn)?; + let relations = self.all_relations(conn)?; + 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| { + match resolve_ready_target( + self.target_authority.as_ref(), + &self.workspace_id, + &ticket, + ) { + Ok(_) => Ok(()), + Err(TicketError::TargetAuthorityUnavailable) + if ticket.meta.repository_id.is_some() + && ticket.meta.ref_selector.is_some() => + { + 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, + queue_guard: projection.queue_guard, + queue_tickets, + recommended_action: projection + .next_action + .unwrap_or(TicketWorkspaceNextAction::WaitForOrchestrator), + }) }) } @@ -4473,6 +4448,7 @@ impl TicketBackend for LocalTicketBackend { ticket: summary, blockers: ticket.relations.blockers, queue_guard: projection.queue_guard, + queue_tickets: Vec::new(), recommended_action: projection .next_action .unwrap_or(TicketWorkspaceNextAction::WaitForOrchestrator), @@ -5507,6 +5483,96 @@ fn ticket_state_resolved(state: TicketWorkflowState) -> bool { ) } +fn transitive_dependency_blockers( + requested_ticket: &str, + states: &HashMap, + relations: &[TicketRelation], +) -> Result> { + type DependencyEdge = (String, String, TicketRelationKind, Option); + let mut prerequisites = BTreeMap::>::new(); + for relation in relations { + let edge = match relation.kind { + TicketRelationKind::DependsOn => Some(( + relation.ticket_id.clone(), + ( + relation.target.clone(), + "depends_on".to_string(), + relation.kind, + relation.note.clone(), + ), + )), + TicketRelationKind::Blocks => Some(( + relation.target.clone(), + ( + relation.ticket_id.clone(), + "blocked_by".to_string(), + relation.kind, + relation.note.clone(), + ), + )), + TicketRelationKind::Related + | TicketRelationKind::Supersedes + | TicketRelationKind::DuplicateOf => None, + }; + if let Some((ticket, edge)) = edge { + prerequisites.entry(ticket).or_default().push(edge); + } + } + for dependencies in prerequisites.values_mut() { + dependencies.sort_by(|left, right| left.0.cmp(&right.0)); + } + + fn collect( + ticket: &str, + states: &HashMap, + prerequisites: &BTreeMap>, + visited: &mut BTreeSet, + blockers: &mut BTreeMap, + ) -> Result<()> { + if !visited.insert(ticket.to_owned()) { + return Ok(()); + } + let state = states + .get(ticket) + .copied() + .ok_or_else(|| TicketError::NotFound(ticket.to_owned()))?; + if ticket_state_resolved(state) { + return Ok(()); + } + if let Some(dependencies) = prerequisites.get(ticket) { + for (dependency, reason_kind, relation_kind, note) in dependencies { + let dependency_state = states + .get(dependency) + .copied() + .ok_or_else(|| TicketError::NotFound(dependency.clone()))?; + if !ticket_state_resolved(dependency_state) { + blockers + .entry(dependency.clone()) + .or_insert_with(|| TicketRelationBlocker { + blocking_ticket: dependency.clone(), + reason_kind: reason_kind.clone(), + relation_kind: *relation_kind, + note: note.clone(), + blocking_state: dependency_state, + }); + collect(dependency, states, prerequisites, visited, blockers)?; + } + } + } + Ok(()) + } + + let mut blockers = BTreeMap::new(); + collect( + requested_ticket, + states, + &prerequisites, + &mut BTreeSet::new(), + &mut blockers, + )?; + Ok(blockers.into_values().collect()) +} + fn dependency_queue_plan( requested_ticket: &str, states: &HashMap, @@ -7531,6 +7597,71 @@ state: planning assert_ticket_target_edit_semantics(&backend); } + #[test] + fn sqlite_dependency_check_blocks_transitive_planning_dependency() { + let temp = TempDir::new().unwrap(); + let backend = SqliteTicketBackend::open(temp.path().join("tickets.db"), "workspace-test") + .unwrap() + .with_target_authority(Arc::new(TestTargetAuthority)); + let mut root_input = NewTicket::new("Ready root"); + root_input.workflow_state = Some(TicketWorkflowState::Ready); + root_input.repository_id = Some("main".to_string()); + let root = backend.create(root_input).unwrap(); + let mut middle_input = NewTicket::new("Queued middle"); + middle_input.workflow_state = Some(TicketWorkflowState::Queued); + middle_input.repository_id = Some("main".to_string()); + let middle = backend.create(middle_input).unwrap(); + let leaf = backend.create(NewTicket::new("Planning leaf")).unwrap(); + for (ticket, target) in [ + (root.id.clone(), middle.id.clone()), + (middle.id.clone(), leaf.id.clone()), + ] { + backend + .add_ticket_relation( + TicketIdOrSlug::Id(ticket), + NewTicketRelation { + kind: TicketRelationKind::DependsOn, + target, + note: None, + author: None, + }, + ) + .unwrap(); + } + + 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.blockers.iter().any(|blocker| { + blocker.blocking_ticket == leaf.id + && blocker.blocking_state == TicketWorkflowState::Planning + })); + + 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!(item.relation_blockers.iter().any(|blocker| { + blocker.blocking_ticket == leaf.id + && blocker.blocking_state == TicketWorkflowState::Planning + })); + assert!( + !project_ticket_workspace_item(&item.summary, &item.relation_blockers, None) + .queue_guard + .can_queue_for_orchestrator + ); + } + #[test] fn sqlite_queue_cycle_diagnostic_leaves_all_tickets_ready() { let temp = TempDir::new().unwrap(); @@ -7605,6 +7736,18 @@ state: planning ) .unwrap(); + 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") + ); let error = backend .queue_ready(TicketIdOrSlug::Id(root.id.clone()), "orchestrator") .unwrap_err(); @@ -8015,7 +8158,7 @@ state: planning .expect("following method"); let projection_source = &source[start..end]; assert_eq!(projection_source.matches("self.with_read(").count(), 2); - assert_eq!(projection_source.matches(".prepare(").count(), 3); + assert_eq!(projection_source.matches(".prepare(").count(), 2); let item_loop = projection_source .split("items: summaries") .nth(1) diff --git a/crates/workspace-server/src/authority.rs b/crates/workspace-server/src/authority.rs index 66b91ddb..506b3022 100644 --- a/crates/workspace-server/src/authority.rs +++ b/crates/workspace-server/src/authority.rs @@ -723,6 +723,9 @@ impl SqliteWorkspaceAuthority { request: TicketShowRequest, ) -> Result { let id = ticket.meta.id.as_str(); + let dependency_check = self + .ticket_backend + .dependency_check(TicketIdOrSlug::Id(id.to_string()))?; let (body, body_truncated) = truncate_body(ticket.document.body.as_str(), DETAIL_BODY_LIMIT); let event_limit = request @@ -822,6 +825,27 @@ impl SqliteWorkspaceAuthority { .any(|assignment| assignment.role == TicketAssignmentRole::Coder); let has_target = ticket.meta.repository_id.is_some() && ticket.meta.ref_selector.is_some(); 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_current_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,6 +857,19 @@ 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_eligibility = TicketActionEligibility { can_assign_orchestrator: matches!( ticket.meta.workflow_state, @@ -847,16 +884,15 @@ impl SqliteWorkspaceAuthority { can_queue: ticket.meta.workflow_state == TicketWorkflowState::Ready && has_orchestrator && !has_coder - && has_target, + && has_target + && dependency_check.queue_guard.can_queue_for_orchestrator + && queue_assignments_valid, can_start_manual_coder: ticket.meta.workflow_state == TicketWorkflowState::Ready && !has_orchestrator && !has_coder && has_target && !has_blockers, - blockers: [(!has_target).then_some("Ticket target is required".to_string())] - .into_iter() - .flatten() - .collect(), + blockers: action_blockers, }; let merge_request = match self.merge_request_store.get(&self.workspace_id, id) { Ok(request) => { @@ -2923,7 +2959,7 @@ mod tests { async fn sqlite_workspace_authority_reads_sqlite_records_without_filesystem_authority() { let dir = tempfile::tempdir().unwrap(); write_ticket(dir.path(), "00000000001J2", "Read bridge", "ready"); - write_ticket(dir.path(), "00000000001J5", "Second ticket", "planning"); + write_ticket(dir.path(), "00000000001J5", "Second ticket", "queued"); write_ticket(dir.path(), "00000000001J6", "Third ticket", "planning"); let db_path = dir.path().join("workspace.db"); let store = SqliteWorkspaceStore::open(&db_path).unwrap(); @@ -3034,10 +3070,22 @@ VALUES ('workspace-test', 'ticket', 4); .ticket_backend .add_ticket_relation( TicketIdOrSlug::Id("00000000001J2".to_string()), + ticket::NewTicketRelation { + kind: ticket::TicketRelationKind::DependsOn, + target: "00000000001J5".to_string(), + note: Some("queued dependency with a transitive blocker".to_string()), + author: Some("tester".to_string()), + }, + ) + .unwrap(); + authority + .ticket_backend + .add_ticket_relation( + TicketIdOrSlug::Id("00000000001J5".to_string()), ticket::NewTicketRelation { kind: ticket::TicketRelationKind::DependsOn, target: "00000000001J6".to_string(), - note: Some("separate dependency relation".to_string()), + note: Some("transitive planning dependency".to_string()), author: Some("tester".to_string()), }, ) @@ -3052,6 +3100,14 @@ VALUES ('workspace-test', 'ticket', 4); 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.body.contains("Ticket body")); assert!(ticket.body_truncated); assert!(!ticket.body.contains("Deep Ticket marker")); @@ -3135,8 +3191,8 @@ VALUES ('workspace-test', 'ticket', 4); assert!(note_only_kind.items.is_empty()); let crossed_relation_filters = authority .query_tickets(TicketQueryRequest { - related_ticket_id: Some("00000000001J5".to_string()), - relation_kind: Some("depends_on".to_string()), + related_ticket_id: Some("00000000001J6".to_string()), + relation_kind: Some("related".to_string()), ..TicketQueryRequest::default() }) .unwrap(); diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index 5fdbb4d9..c2ebf05c 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -4025,6 +4025,20 @@ async fn execute_ticket_rest_operation( queue_assignment_candidates(&backend, &ticket.meta.id).map_err(Error::from)?; let mut assignment_ids = BTreeMap::new(); for ticket_id in candidates { + if api + .store + .get_current_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!( @@ -17419,7 +17433,11 @@ mod tests { let Json(ready_detail) = scoped_get_ticket(State(api.clone()), AxumPath(path())) .await .unwrap(); - assert!(ready_detail.action_eligibility.can_queue); + assert!( + ready_detail.action_eligibility.can_queue, + "Queue blockers: {:?}", + ready_detail.action_eligibility.blockers + ); assert!(ready_detail.action_eligibility.blockers.is_empty()); assert_eq!(ready_detail.relations.blockers.len(), 1); assert_eq!(ready_detail.relations.blockers[0].reason_kind, "depends_on"); From a41147916befbaba17355617b332f22a371eca29 Mon Sep 17 00:00:00 2001 From: Hare Date: Tue, 25 Aug 2026 11:51:40 +0900 Subject: [PATCH 5/9] fix: confirm queue closures across clients --- crates/ticket/src/lib.rs | 121 +++++++++++++++- crates/tui/src/dashboard/mod.rs | 131 ++++++++++++------ crates/tui/src/workspace_panel.rs | 7 +- crates/workspace-server/src/authority.rs | 1 + crates/workspace-server/src/records.rs | 1 + crates/workspace-server/src/server.rs | 102 +++++++++----- web/workspace/src/lib/generated/ticket-api.ts | 2 +- .../workspace/tickets/ticket-panel.test.ts | 7 +- .../tickets/[ticketId]/+page.svelte | 40 +++++- 9 files changed, 315 insertions(+), 97 deletions(-) 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} From 5857e6121c0e1f84184e7ce63de861b1372a8426 Mon Sep 17 00:00:00 2001 From: Hare Date: Tue, 25 Aug 2026 12:05:43 +0900 Subject: [PATCH 6/9] fix: preserve dependency queue atomicity --- crates/ticket/src/lib.rs | 164 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 161 insertions(+), 3 deletions(-) diff --git a/crates/ticket/src/lib.rs b/crates/ticket/src/lib.rs index 0cd51910..a347e0f3 100644 --- a/crates/ticket/src/lib.rs +++ b/crates/ticket/src/lib.rs @@ -4702,6 +4702,24 @@ impl TicketBackend for LocalTicketBackend { planned.push((dir, target)); } + let snapshots = planned + .iter() + .map(|(dir, _)| { + let item_path = dir.join("item.md"); + let thread_path = dir.join("thread.md"); + Ok(( + item_path.clone(), + fs::read(&item_path).map_err(|error| io_err(&item_path, error))?, + thread_path.clone(), + if thread_path.exists() { + Some(fs::read(&thread_path).map_err(|error| io_err(&thread_path, error))?) + } else { + None + }, + )) + }) + .collect::>>()?; + for (dir, target) in planned { let at = now_utc(); let mut change = TicketStateChange::new( @@ -4711,7 +4729,7 @@ impl TicketBackend for LocalTicketBackend { self.queued_ready_body(queued_by), ); change.author = Some(queued_by.to_string()); - self.apply_workflow_state_change( + if let Err(error) = self.apply_workflow_state_change( &dir, TicketWorkflowState::Ready, TicketWorkflowState::Queued, @@ -4723,7 +4741,29 @@ impl TicketBackend for LocalTicketBackend { ("ref_selector", target.ref_selector.as_str()), ("queue_root_ticket", requested_ticket.as_str()), ], - )?; + ) { + for (item_path, item_before, thread_path, thread_before) in &snapshots { + if fs::read(item_path).ok().as_deref() != Some(item_before.as_slice()) { + fs::write(item_path, item_before) + .map_err(|rollback| io_err(item_path, rollback))?; + } + match thread_before { + Some(thread_before) + if fs::read(thread_path).ok().as_deref() + != Some(thread_before.as_slice()) => + { + fs::write(thread_path, thread_before) + .map_err(|rollback| io_err(thread_path, rollback))?; + } + None if thread_path.exists() => { + fs::remove_file(thread_path) + .map_err(|rollback| io_err(thread_path, rollback))?; + } + _ => {} + } + } + return Err(error); + } } Ok(TicketQueueOutcome { @@ -5690,11 +5730,19 @@ fn dependency_queue_plan( 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) => { @@ -5713,7 +5761,7 @@ fn dependency_queue_plan( stack.push(ticket.to_owned()); if let Some(dependencies) = prerequisites.get(ticket) { for dependency in dependencies { - visit(dependency, prerequisites, marks, stack, ordered)?; + visit(dependency, states, prerequisites, marks, stack, ordered)?; } } stack.pop(); @@ -5725,6 +5773,7 @@ fn dependency_queue_plan( let mut ordered = Vec::new(); visit( requested_ticket, + states, &prerequisites, &mut BTreeMap::new(), &mut Vec::new(), @@ -7323,6 +7372,7 @@ mod tests { let relations = [ dependency_relation("root", "done"), dependency_relation("done", "planning"), + dependency_relation("done", "root"), ]; assert_eq!( @@ -7740,6 +7790,59 @@ state: planning ); } + #[test] + fn sqlite_queue_ignores_cycle_behind_done_dependency() { + let temp = TempDir::new().unwrap(); + let backend = SqliteTicketBackend::open(temp.path().join("tickets.db"), "workspace-test") + .unwrap() + .with_target_authority(Arc::new(TestTargetAuthority)); + let mut root_input = NewTicket::new("Ready root"); + root_input.workflow_state = Some(TicketWorkflowState::Ready); + root_input.repository_id = Some("main".to_string()); + let root = backend.create(root_input).unwrap(); + let mut done_input = NewTicket::new("Done dependency"); + done_input.workflow_state = Some(TicketWorkflowState::Done); + done_input.repository_id = Some("main".to_string()); + let done = backend.create(done_input).unwrap(); + for (ticket_id, target) in [ + (root.id.clone(), done.id.clone()), + (done.id.clone(), root.id.clone()), + ] { + backend + .add_ticket_relation( + TicketIdOrSlug::Id(ticket_id), + NewTicketRelation { + kind: TicketRelationKind::DependsOn, + target, + note: None, + author: None, + }, + ) + .unwrap(); + } + + let outcome = backend + .queue_ready(TicketIdOrSlug::Id(root.id.clone()), "orchestrator") + .unwrap(); + assert_eq!(outcome.queued_tickets, vec![root.id.clone()]); + assert_eq!( + backend + .show(TicketIdOrSlug::Id(root.id)) + .unwrap() + .meta + .workflow_state, + TicketWorkflowState::Queued + ); + assert_eq!( + backend + .show(TicketIdOrSlug::Id(done.id)) + .unwrap() + .meta + .workflow_state, + TicketWorkflowState::Done + ); + } + #[test] fn sqlite_queue_cycle_diagnostic_leaves_all_tickets_ready() { let temp = TempDir::new().unwrap(); @@ -8680,6 +8783,61 @@ state: planning ); } + #[cfg(unix)] + #[test] + fn local_queue_rolls_back_ready_dependency_when_later_write_fails() { + use std::os::unix::fs::PermissionsExt; + + let tmp = TempDir::new().unwrap(); + let backend = backend(&tmp); + let mut dependency_input = NewTicket::new("Ready dependency"); + dependency_input.workflow_state = Some(TicketWorkflowState::Ready); + dependency_input.repository_id = Some("main".to_string()); + dependency_input.ref_selector = Some("develop".to_string()); + let dependency = backend.create(dependency_input).unwrap(); + let mut root_input = NewTicket::new("Ready root"); + root_input.workflow_state = Some(TicketWorkflowState::Ready); + root_input.repository_id = Some("main".to_string()); + root_input.ref_selector = Some("develop".to_string()); + let root = backend.create(root_input).unwrap(); + backend + .add_ticket_relation( + TicketIdOrSlug::Id(root.id.clone()), + NewTicketRelation { + kind: TicketRelationKind::DependsOn, + target: dependency.id.clone(), + note: None, + author: None, + }, + ) + .unwrap(); + + let root_dir = backend + .find_ticket_dir(&TicketIdOrSlug::Id(root.id.clone())) + .unwrap(); + let root_item = root_dir.join("item.md"); + fs::set_permissions(&root_dir, fs::Permissions::from_mode(0o555)).unwrap(); + fs::set_permissions(&root_item, fs::Permissions::from_mode(0o444)).unwrap(); + let result = backend.queue_ready(TicketIdOrSlug::Id(root.id.clone()), "test"); + fs::set_permissions(&root_dir, fs::Permissions::from_mode(0o755)).unwrap(); + fs::set_permissions(&root_item, fs::Permissions::from_mode(0o644)).unwrap(); + + assert!(result.is_err()); + for ticket_id in [dependency.id, root.id] { + let ticket = backend.show(TicketIdOrSlug::Id(ticket_id)).unwrap(); + assert_eq!(ticket.meta.workflow_state, TicketWorkflowState::Ready); + assert!( + !ticket + .events + .iter() + .any(|event| event.to.as_deref() == Some("queued")), + "Ticket {} retained queued events: {:?}", + ticket.meta.id, + ticket.events + ); + } + } + #[test] fn state_cannot_be_changed_through_generic_field_api() { let tmp = TempDir::new().unwrap(); From 17497570361557a5c14b924beaf020f59c4a4f43 Mon Sep 17 00:00:00 2001 From: Hare Date: Tue, 25 Aug 2026 12:24:31 +0900 Subject: [PATCH 7/9] fix: align queue eligibility with target authority --- crates/workspace-server/src/authority.rs | 20 +++++-- crates/workspace-server/src/server.rs | 68 ++++++++++++++++++++++-- 2 files changed, 80 insertions(+), 8 deletions(-) diff --git a/crates/workspace-server/src/authority.rs b/crates/workspace-server/src/authority.rs index 16f9e0f8..1d01aad7 100644 --- a/crates/workspace-server/src/authority.rs +++ b/crates/workspace-server/src/authority.rs @@ -704,6 +704,15 @@ impl SqliteWorkspaceAuthority { &self, reference: &str, request: TicketShowRequest, + ) -> Result { + self.read_ticket_detail_with_backend(reference, request, &self.ticket_backend) + } + + pub(crate) fn read_ticket_detail_with_backend( + &self, + reference: &str, + request: TicketShowRequest, + backend: &SqliteTicketBackend, ) -> Result { let id = self .store @@ -713,19 +722,19 @@ impl SqliteWorkspaceAuthority { reference, )? .ok_or_else(|| Error::Ticket(ticket::TicketError::NotFound(reference.to_string())))?; - let ticket = self.ticket_backend.show(TicketIdOrSlug::Id(id))?; - self.ticket_detail_from_ticket(ticket, request) + let ticket = backend.show(TicketIdOrSlug::Id(id))?; + self.ticket_detail_from_ticket(ticket, request, backend) } fn ticket_detail_from_ticket( &self, ticket: ticket::Ticket, request: TicketShowRequest, + dependency_backend: &SqliteTicketBackend, ) -> Result { let id = ticket.meta.id.as_str(); - let dependency_check = self - .ticket_backend - .dependency_check(TicketIdOrSlug::Id(id.to_string()))?; + let dependency_check = + dependency_backend.dependency_check(TicketIdOrSlug::Id(id.to_string()))?; let (body, body_truncated) = truncate_body(ticket.document.body.as_str(), DETAIL_BODY_LIMIT); let event_limit = request @@ -1117,6 +1126,7 @@ impl TicketAuthority for SqliteWorkspaceAuthority { event_limit: Some(TICKET_EVENT_LIMIT), event_cursor: None, }, + &self.ticket_backend, )?; if ticket_matches_query( &summary, diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index 3ab662e9..367e3055 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -3133,7 +3133,7 @@ async fn scoped_get_ticket( AxumPath(path): AxumPath, ) -> ApiResult> { validate_workspace_scope(&api, &path.workspace_id)?; - get_ticket(State(api), AxumPath(path.id)).await + browser_ticket_detail(&api, &path.id) } async fn scoped_query_tickets( @@ -3151,7 +3151,10 @@ async fn scoped_show_ticket( Json(query): Json, ) -> ApiResult> { validate_workspace_scope(&api, &path.workspace_id)?; - Ok(Json(api.authority.show_ticket(&path.id, query)?)) + let backend = browser_ticket_backend(&api)?; + Ok(Json(api.authority.read_ticket_detail_with_backend( + &path.id, query, &backend, + )?)) } #[derive(Debug, Serialize, Deserialize, PartialEq, Eq)] @@ -3766,7 +3769,12 @@ fn browser_ticket_backend(api: &WorkspaceApi) -> Result { } fn browser_ticket_detail(api: &WorkspaceApi, ticket_id: &str) -> ApiResult> { - Ok(Json(api.authority.ticket(ticket_id)?)) + let backend = browser_ticket_backend(api)?; + Ok(Json(api.authority.read_ticket_detail_with_backend( + ticket_id, + TicketShowRequest::default(), + &backend, + )?)) } async fn scoped_edit_ticket_item( @@ -16521,6 +16529,60 @@ mod tests { } } + #[tokio::test] + async fn browser_queue_eligibility_uses_authoritative_dependency_targets() { + 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("Invalid target dependency"); + dependency_input.workflow_state = Some(TicketWorkflowState::Ready); + dependency_input.repository_id = Some(TEST_REPOSITORY_ID.to_string()); + dependency_input.ref_selector = Some("missing-ref".to_string()); + let dependency = backend.create(dependency_input).unwrap(); + let mut root_input = ticket::NewTicket::new("Queue root"); + root_input.workflow_state = Some(TicketWorkflowState::Ready); + root_input.repository_id = Some(TEST_REPOSITORY_ID.to_string()); + root_input.ref_selector = Some("develop".to_string()); + 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: None, + author: Some("test".to_string()), + }, + ) + .unwrap(); + assign_test_orchestrator(&api, &root.id); + assign_test_orchestrator(&api, &dependency.id); + + let Json(detail) = browser_ticket_detail(&api, &root.id).unwrap(); + assert!(!detail.action_eligibility.can_queue); + assert!( + detail + .action_eligibility + .blockers + .iter() + .any(|blocker| blocker.contains("missing-ref")) + ); + let result = scoped_queue_ticket_record( + State(api.clone()), + AxumPath((TEST_WORKSPACE_ID.to_string(), root.id.clone())), + HeaderMap::new(), + ) + .await; + assert!(result.is_err()); + for ticket_id in [dependency.id, root.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(); From 7dd8809e385c50dc9dea04ad18b5ea9017e3b4b5 Mon Sep 17 00:00:00 2001 From: Hare Date: Tue, 25 Aug 2026 12:44:03 +0900 Subject: [PATCH 8/9] fix: fail closed without queue target authority --- crates/ticket/src/lib.rs | 74 +++++++++++++++++++++++++-------- crates/tui/src/dashboard/mod.rs | 45 +++++++++++++++++++- 2 files changed, 101 insertions(+), 18 deletions(-) diff --git a/crates/ticket/src/lib.rs b/crates/ticket/src/lib.rs index a347e0f3..65113312 100644 --- a/crates/ticket/src/lib.rs +++ b/crates/ticket/src/lib.rs @@ -3677,20 +3677,11 @@ impl TicketBackend for SqliteTicketBackend { let target_error = queue_tickets.iter().find_map(|candidate| { self.load_ticket(conn, candidate) .and_then(|ticket| { - match resolve_ready_target( + resolve_ready_target( self.target_authority.as_ref(), &self.workspace_id, &ticket, - ) { - Ok(_) => Ok(()), - Err(TicketError::TargetAuthorityUnavailable) - if ticket.meta.repository_id.is_some() - && ticket.meta.ref_selector.is_some() => - { - Ok(()) - } - Err(error) => Err(error), - } + ) }) .err() }); @@ -4487,15 +4478,11 @@ impl TicketBackend for LocalTicketBackend { self.find_ticket_dir(&TicketIdOrSlug::Id(candidate.clone())) .and_then(|dir| self.ticket_from_dir(&dir)) .and_then(|ticket| { - match resolve_ready_target( + resolve_ready_target( self.target_authority.as_ref(), "local", &ticket, - ) { - Ok(_) => Ok(()), - Err(TicketError::TargetAuthorityUnavailable) => Ok(()), - Err(error) => Err(error), - } + ) }) .err() }); @@ -7790,6 +7777,59 @@ state: planning ); } + #[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.repository_id = Some("main".to_string()); + sqlite_input.ref_selector = Some("develop".to_string()); + 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") + ); + assert!(matches!( + sqlite.queue_ready(TicketIdOrSlug::Id(sqlite_ticket.id), "test"), + Err(TicketError::TargetAuthorityUnavailable) + )); + + let local_temp = TempDir::new().unwrap(); + let local = LocalTicketBackend::new(local_temp.path()); + let mut local_input = NewTicket::new("Local ready Ticket"); + local_input.workflow_state = Some(TicketWorkflowState::Ready); + local_input.repository_id = Some("main".to_string()); + local_input.ref_selector = Some("develop".to_string()); + let local_ticket = local.create(local_input).unwrap(); + let local_check = local + .dependency_check(TicketIdOrSlug::Id(local_ticket.id.clone())) + .unwrap(); + assert!(!local_check.queue_guard.can_queue_for_orchestrator); + assert!( + local_check + .queue_guard + .blocked_reason + .as_deref() + .unwrap_or_default() + .contains("target authority is unavailable") + ); + assert!(matches!( + local.queue_ready(TicketIdOrSlug::Id(local_ticket.id), "test"), + Err(TicketError::TargetAuthorityUnavailable) + )); + } + #[test] fn sqlite_queue_ignores_cycle_behind_done_dependency() { let temp = TempDir::new().unwrap(); diff --git a/crates/tui/src/dashboard/mod.rs b/crates/tui/src/dashboard/mod.rs index 42780f1a..3ef57175 100644 --- a/crates/tui/src/dashboard/mod.rs +++ b/crates/tui/src/dashboard/mod.rs @@ -4,6 +4,7 @@ use std::fmt; use std::io; use std::path::{Path, PathBuf}; use std::process::Command; +use std::sync::Arc; use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; use client::ticket_role::{ @@ -4125,7 +4126,10 @@ async fn dispatch_ticket_action( let config = TicketConfig::load_workspace(&request.workspace_root) .map_err(|error| TicketActionError::BackendConfig(error.to_string()))?; let backend = LocalTicketBackend::new(config.backend_root()) - .with_record_language(config.ticket_record_language()); + .with_record_language(config.ticket_record_language()) + .with_target_authority(Arc::new(DashboardTicketTargetAuthority { + workspace_root: request.workspace_root.clone(), + })); if request.action == NextUserAction::Close { return dispatch_panel_close(&backend, &request.ticket_id); } @@ -4235,6 +4239,45 @@ async fn dispatch_panel_queue( }) } +struct DashboardTicketTargetAuthority { + workspace_root: PathBuf, +} + +impl ticket::TicketTargetAuthority for DashboardTicketTargetAuthority { + fn resolve_target( + &self, + _workspace_id: &str, + repository_id: Option<&str>, + ref_selector: Option<&str>, + ) -> ticket::Result { + let repository_id = repository_id.unwrap_or("main"); + if repository_id != "main" { + return Err(ticket::TicketError::UnknownTargetRepository( + repository_id.to_string(), + )); + } + let ref_selector = ref_selector.unwrap_or("HEAD"); + git_capture( + &self.workspace_root, + &[ + "rev-parse", + "--verify", + &format!("{ref_selector}^{{commit}}"), + ], + "resolve Queue Ticket target", + ) + .map_err(|reason| ticket::TicketError::InvalidTargetSelector { + repository_id: repository_id.to_string(), + selector: ref_selector.to_string(), + reason, + })?; + Ok(ticket::ResolvedTicketTarget { + repository_id: repository_id.to_string(), + ref_selector: ref_selector.to_string(), + }) + } +} + #[derive(Debug, Clone, PartialEq, Eq)] struct PanelQueueHandoffPreflight { ticket_id: String, From 097c363fbc2b0592b7ef426238d9092cd190ac62 Mon Sep 17 00:00:00 2001 From: Hare Date: Tue, 25 Aug 2026 12:54:47 +0900 Subject: [PATCH 9/9] fix: block internal dependency cycles in projections --- crates/ticket/src/lib.rs | 83 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 75 insertions(+), 8 deletions(-) diff --git a/crates/ticket/src/lib.rs b/crates/ticket/src/lib.rs index 65113312..1140f5ea 100644 --- a/crates/ticket/src/lib.rs +++ b/crates/ticket/src/lib.rs @@ -5629,14 +5629,12 @@ fn transitive_dependency_blockers( fn collect( ticket: &str, + requested_ticket: &str, states: &HashMap, prerequisites: &BTreeMap>, - visited: &mut BTreeSet, + marks: &mut BTreeMap, blockers: &mut BTreeMap, ) -> Result<()> { - if !visited.insert(ticket.to_owned()) { - return Ok(()); - } let state = states .get(ticket) .copied() @@ -5644,6 +5642,30 @@ fn transitive_dependency_blockers( if ticket_state_resolved(state) { return Ok(()); } + match marks.get(ticket).copied() { + Some(2) => return Ok(()), + Some(1) => { + let requested_state = states + .get(requested_ticket) + .copied() + .ok_or_else(|| TicketError::NotFound(requested_ticket.to_owned()))?; + blockers.insert( + requested_ticket.to_owned(), + TicketRelationBlocker { + blocking_ticket: requested_ticket.to_owned(), + reason_kind: "dependency_cycle".to_string(), + relation_kind: TicketRelationKind::DependsOn, + note: Some(format!( + "dependency cycle reachable through Ticket {ticket}" + )), + blocking_state: requested_state, + }, + ); + return Ok(()); + } + _ => {} + } + marks.insert(ticket.to_owned(), 1); if let Some(dependencies) = prerequisites.get(ticket) { for (dependency, reason_kind, relation_kind, note) in dependencies { let dependency_state = states @@ -5660,19 +5682,28 @@ fn transitive_dependency_blockers( note: note.clone(), blocking_state: dependency_state, }); - collect(dependency, states, prerequisites, visited, blockers)?; + collect( + dependency, + requested_ticket, + states, + prerequisites, + marks, + blockers, + )?; } } } + marks.insert(ticket.to_owned(), 2); Ok(()) } let mut blockers = BTreeMap::new(); collect( + requested_ticket, requested_ticket, states, &prerequisites, - &mut BTreeSet::new(), + &mut BTreeMap::new(), &mut blockers, )?; Ok(blockers.into_values().collect()) @@ -7349,6 +7380,37 @@ mod tests { ); } + #[test] + fn transitive_blockers_project_internal_dependency_cycle_as_blocking() { + let states = HashMap::from([ + ("root".to_owned(), TicketWorkflowState::Ready), + ("branch".to_owned(), TicketWorkflowState::Ready), + ("cycle".to_owned(), TicketWorkflowState::Ready), + ]); + let relations = [ + dependency_relation("root", "branch"), + dependency_relation("branch", "cycle"), + dependency_relation("cycle", "branch"), + ]; + + let blockers = transitive_dependency_blockers("root", &states, &relations).unwrap(); + assert!(blockers.iter().any(|blocker| { + blocker.blocking_ticket == "root" && blocker.reason_kind == "dependency_cycle" + })); + let summary = summary_with_state(TicketWorkflowState::Ready); + let mut summary = summary; + summary.id = "root".to_string(); + assert!( + !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([ @@ -7897,9 +7959,14 @@ state: planning second_input.workflow_state = Some(TicketWorkflowState::Ready); second_input.repository_id = Some("main".to_string()); let second = backend.create(second_input).unwrap(); + let mut third_input = NewTicket::new("Third ready Ticket"); + third_input.workflow_state = Some(TicketWorkflowState::Ready); + third_input.repository_id = Some("main".to_string()); + let third = backend.create(third_input).unwrap(); for (ticket, target) in [ (first.id.clone(), second.id.clone()), - (second.id.clone(), first.id.clone()), + (second.id.clone(), third.id.clone()), + (third.id.clone(), second.id.clone()), ] { backend .add_ticket_relation( @@ -7948,7 +8015,7 @@ state: planning .unwrap_err(); assert!(matches!(error, TicketError::Conflict(_))); assert!(error.to_string().contains(" -> ")); - for ticket_id in [first.id, second.id] { + for ticket_id in [first.id, second.id, third.id] { assert_eq!( backend .show(TicketIdOrSlug::Id(ticket_id))