fix: fail closed without queue target authority
This commit is contained in:
+57
-17
@@ -3677,20 +3677,11 @@ impl TicketBackend for SqliteTicketBackend {
|
|||||||
let target_error = queue_tickets.iter().find_map(|candidate| {
|
let target_error = queue_tickets.iter().find_map(|candidate| {
|
||||||
self.load_ticket(conn, candidate)
|
self.load_ticket(conn, candidate)
|
||||||
.and_then(|ticket| {
|
.and_then(|ticket| {
|
||||||
match resolve_ready_target(
|
resolve_ready_target(
|
||||||
self.target_authority.as_ref(),
|
self.target_authority.as_ref(),
|
||||||
&self.workspace_id,
|
&self.workspace_id,
|
||||||
&ticket,
|
&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()
|
.err()
|
||||||
});
|
});
|
||||||
@@ -4487,15 +4478,11 @@ impl TicketBackend for LocalTicketBackend {
|
|||||||
self.find_ticket_dir(&TicketIdOrSlug::Id(candidate.clone()))
|
self.find_ticket_dir(&TicketIdOrSlug::Id(candidate.clone()))
|
||||||
.and_then(|dir| self.ticket_from_dir(&dir))
|
.and_then(|dir| self.ticket_from_dir(&dir))
|
||||||
.and_then(|ticket| {
|
.and_then(|ticket| {
|
||||||
match resolve_ready_target(
|
resolve_ready_target(
|
||||||
self.target_authority.as_ref(),
|
self.target_authority.as_ref(),
|
||||||
"local",
|
"local",
|
||||||
&ticket,
|
&ticket,
|
||||||
) {
|
)
|
||||||
Ok(_) => Ok(()),
|
|
||||||
Err(TicketError::TargetAuthorityUnavailable) => Ok(()),
|
|
||||||
Err(error) => Err(error),
|
|
||||||
}
|
|
||||||
})
|
})
|
||||||
.err()
|
.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]
|
#[test]
|
||||||
fn sqlite_queue_ignores_cycle_behind_done_dependency() {
|
fn sqlite_queue_ignores_cycle_behind_done_dependency() {
|
||||||
let temp = TempDir::new().unwrap();
|
let temp = TempDir::new().unwrap();
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ use std::fmt;
|
|||||||
use std::io;
|
use std::io;
|
||||||
use std::path::{Path, PathBuf};
|
use std::path::{Path, PathBuf};
|
||||||
use std::process::Command;
|
use std::process::Command;
|
||||||
|
use std::sync::Arc;
|
||||||
use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH};
|
use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH};
|
||||||
|
|
||||||
use client::ticket_role::{
|
use client::ticket_role::{
|
||||||
@@ -4125,7 +4126,10 @@ async fn dispatch_ticket_action(
|
|||||||
let config = TicketConfig::load_workspace(&request.workspace_root)
|
let config = TicketConfig::load_workspace(&request.workspace_root)
|
||||||
.map_err(|error| TicketActionError::BackendConfig(error.to_string()))?;
|
.map_err(|error| TicketActionError::BackendConfig(error.to_string()))?;
|
||||||
let backend = LocalTicketBackend::new(config.backend_root())
|
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 {
|
if request.action == NextUserAction::Close {
|
||||||
return dispatch_panel_close(&backend, &request.ticket_id);
|
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<ticket::ResolvedTicketTarget> {
|
||||||
|
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)]
|
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||||
struct PanelQueueHandoffPreflight {
|
struct PanelQueueHandoffPreflight {
|
||||||
ticket_id: String,
|
ticket_id: String,
|
||||||
|
|||||||
Reference in New Issue
Block a user