fix: project Ticket mutation outputs to human keys
This commit is contained in:
+139
-5
@@ -1223,10 +1223,17 @@ impl Tool for TicketQueueTool {
|
|||||||
) -> Result<ToolOutput, ToolError> {
|
) -> Result<ToolOutput, ToolError> {
|
||||||
let params: TicketQueueParams = parse_input("TicketQueue", input_json)?;
|
let params: TicketQueueParams = parse_input("TicketQueue", input_json)?;
|
||||||
let queued_by = default_author();
|
let queued_by = default_author();
|
||||||
let outcome = self
|
let mut outcome = self
|
||||||
.backend
|
.backend
|
||||||
.queue_ready(TicketIdOrSlug::Query(params.ticket.clone()), &queued_by)
|
.queue_ready(TicketIdOrSlug::Query(params.ticket.clone()), &queued_by)
|
||||||
.map_err(|error| backend_error("TicketQueue", error))?;
|
.map_err(|error| backend_error("TicketQueue", error))?;
|
||||||
|
outcome.requested_ticket =
|
||||||
|
model_ticket_reference(&self.backend, &outcome.requested_ticket, "TicketQueue")?;
|
||||||
|
outcome.queued_tickets = outcome
|
||||||
|
.queued_tickets
|
||||||
|
.into_iter()
|
||||||
|
.map(|ticket| model_ticket_reference(&self.backend, &ticket, "TicketQueue"))
|
||||||
|
.collect::<Result<Vec<_>, _>>()?;
|
||||||
Ok(json_output(
|
Ok(json_output(
|
||||||
format!(
|
format!(
|
||||||
"Queued {} ticket(s) for Orchestrator",
|
"Queued {} ticket(s) for Orchestrator",
|
||||||
@@ -1264,15 +1271,17 @@ impl Tool for TicketWorkflowStateTool {
|
|||||||
self.backend
|
self.backend
|
||||||
.set_workflow_state(TicketIdOrSlug::Query(params.ticket.clone()), change)
|
.set_workflow_state(TicketIdOrSlug::Query(params.ticket.clone()), change)
|
||||||
.map_err(|error| backend_error("TicketWorkflowState", error))?;
|
.map_err(|error| backend_error("TicketWorkflowState", error))?;
|
||||||
|
let ticket_ref =
|
||||||
|
model_ticket_reference(&self.backend, ¶ms.ticket, "TicketWorkflowState")?;
|
||||||
Ok(json_output(
|
Ok(json_output(
|
||||||
format!(
|
format!(
|
||||||
"Transitioned ticket {} state {} -> {}",
|
"Transitioned ticket {} state {} -> {}",
|
||||||
params.ticket,
|
ticket_ref,
|
||||||
from.as_str(),
|
from.as_str(),
|
||||||
to.as_str()
|
to.as_str()
|
||||||
),
|
),
|
||||||
json!({
|
json!({
|
||||||
"ticket": params.ticket,
|
"ticket": ticket_ref,
|
||||||
"from": from.as_str(),
|
"from": from.as_str(),
|
||||||
"to": to.as_str(),
|
"to": to.as_str(),
|
||||||
"state": to.as_str(),
|
"state": to.as_str(),
|
||||||
@@ -1296,9 +1305,10 @@ impl Tool for TicketCloseTool {
|
|||||||
MarkdownText::new(params.resolution),
|
MarkdownText::new(params.resolution),
|
||||||
)
|
)
|
||||||
.map_err(|error| backend_error("TicketClose", error))?;
|
.map_err(|error| backend_error("TicketClose", error))?;
|
||||||
|
let ticket_ref = model_ticket_reference(&self.backend, ¶ms.ticket, "TicketClose")?;
|
||||||
Ok(json_output(
|
Ok(json_output(
|
||||||
format!("Closed ticket {}", params.ticket),
|
format!("Closed ticket {ticket_ref}"),
|
||||||
json!({ "ticket": params.ticket, "state": "closed", "ok": true }),
|
json!({ "ticket": ticket_ref, "state": "closed", "ok": true }),
|
||||||
))
|
))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -1525,6 +1535,29 @@ impl Tool for TicketDependencyCheckTool {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn model_ticket_reference(
|
||||||
|
backend: &TicketToolBackend,
|
||||||
|
reference: &str,
|
||||||
|
tool_name: &str,
|
||||||
|
) -> Result<String, ToolError> {
|
||||||
|
let ticket = backend
|
||||||
|
.show(TicketIdOrSlug::Id(reference.to_string()))
|
||||||
|
.map_err(|error| backend_error(tool_name, error))?;
|
||||||
|
match ticket.meta.resource_key {
|
||||||
|
Some(resource_key) if is_canonical_ticket_resource_key(&resource_key) => Ok(resource_key),
|
||||||
|
Some(_) => Err(ToolError::ExecutionFailed(format!(
|
||||||
|
"{tool_name} failed: required Ticket human key is unavailable"
|
||||||
|
))),
|
||||||
|
None => Ok(ticket.meta.id),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn is_canonical_ticket_resource_key(resource_key: &str) -> bool {
|
||||||
|
resource_key.strip_prefix("T-").is_some_and(|sequence| {
|
||||||
|
!sequence.is_empty() && sequence.bytes().all(|byte| byte.is_ascii_digit())
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
fn parse_input<T: for<'de> Deserialize<'de>>(tool: &str, input_json: &str) -> Result<T, ToolError> {
|
fn parse_input<T: for<'de> Deserialize<'de>>(tool: &str, input_json: &str) -> Result<T, ToolError> {
|
||||||
serde_json::from_str(input_json)
|
serde_json::from_str(input_json)
|
||||||
.map_err(|error| ToolError::InvalidArgument(format!("invalid {tool} input: {error}")))
|
.map_err(|error| ToolError::InvalidArgument(format!("invalid {tool} input: {error}")))
|
||||||
@@ -1922,6 +1955,12 @@ mod tests {
|
|||||||
.with_target_authority(Arc::new(TestTargetAuthority))
|
.with_target_authority(Arc::new(TestTargetAuthority))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn sqlite_backend(temp: &TempDir) -> crate::SqliteTicketBackend {
|
||||||
|
crate::SqliteTicketBackend::open(temp.path().join("tickets.db"), "workspace")
|
||||||
|
.unwrap()
|
||||||
|
.with_target_authority(Arc::new(TestTargetAuthority))
|
||||||
|
}
|
||||||
|
|
||||||
fn tool(definition: ToolDefinition) -> Arc<dyn Tool> {
|
fn tool(definition: ToolDefinition) -> Arc<dyn Tool> {
|
||||||
let (_, tool) = definition();
|
let (_, tool) = definition();
|
||||||
tool
|
tool
|
||||||
@@ -2549,6 +2588,101 @@ mod tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn queue_workflow_and_close_project_internal_inputs_to_ticket_keys() {
|
||||||
|
let temp = TempDir::new().unwrap();
|
||||||
|
let inner = sqlite_backend(&temp);
|
||||||
|
let mut dependency_input = NewTicket::new("Dependency");
|
||||||
|
dependency_input.repository_id = Some("main".to_string());
|
||||||
|
let dependency = inner.create(dependency_input).unwrap();
|
||||||
|
let mut target_input = NewTicket::new("Target");
|
||||||
|
target_input.repository_id = Some("main".to_string());
|
||||||
|
let target = inner.create(target_input).unwrap();
|
||||||
|
inner
|
||||||
|
.add_ticket_relation(
|
||||||
|
TicketIdOrSlug::Id(target.id.clone()),
|
||||||
|
NewTicketRelation {
|
||||||
|
kind: TicketRelationKind::DependsOn,
|
||||||
|
target: dependency.id.clone(),
|
||||||
|
note: None,
|
||||||
|
author: None,
|
||||||
|
},
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
for id in [&dependency.id, &target.id] {
|
||||||
|
inner
|
||||||
|
.mark_ready(
|
||||||
|
TicketIdOrSlug::Id(id.clone()),
|
||||||
|
TicketMarkReady {
|
||||||
|
operation_key: format!("ready-{id}"),
|
||||||
|
reason: None,
|
||||||
|
author: None,
|
||||||
|
intake_summary: None,
|
||||||
|
},
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
}
|
||||||
|
let target_key = target.resource_key.clone().unwrap();
|
||||||
|
let dependency_key = dependency.resource_key.clone().unwrap();
|
||||||
|
let backend = inner;
|
||||||
|
let queue = tool_by_name(TicketToolBackend::new(backend.clone()), "TicketQueue");
|
||||||
|
let workflow = tool_by_name(
|
||||||
|
TicketToolBackend::new(backend.clone()),
|
||||||
|
"TicketWorkflowState",
|
||||||
|
);
|
||||||
|
let close = tool_by_name(TicketToolBackend::new(backend), "TicketClose");
|
||||||
|
|
||||||
|
let queued = queue
|
||||||
|
.execute(
|
||||||
|
&json!({"ticket": target.id.clone()}).to_string(),
|
||||||
|
Default::default(),
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert!(queued.summary.contains("2 ticket(s)"));
|
||||||
|
let queued_content = queued.content.unwrap();
|
||||||
|
assert!(queued_content.contains(&target_key));
|
||||||
|
assert!(queued_content.contains(&dependency_key));
|
||||||
|
assert!(!queued_content.contains(&target.id));
|
||||||
|
assert!(!queued_content.contains(&dependency.id));
|
||||||
|
|
||||||
|
for (from, to) in [("queued", "inprogress"), ("inprogress", "done")] {
|
||||||
|
let transitioned = workflow
|
||||||
|
.execute(
|
||||||
|
&json!({
|
||||||
|
"ticket": target.id.clone(),
|
||||||
|
"from": from,
|
||||||
|
"to": to,
|
||||||
|
"reason": "test_transition",
|
||||||
|
"body": "transitioned",
|
||||||
|
"author": "tester"
|
||||||
|
})
|
||||||
|
.to_string(),
|
||||||
|
Default::default(),
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert!(transitioned.summary.contains(&target_key));
|
||||||
|
assert!(!transitioned.summary.contains(&target.id));
|
||||||
|
let content = transitioned.content.unwrap();
|
||||||
|
assert!(content.contains(&target_key));
|
||||||
|
assert!(!content.contains(&target.id));
|
||||||
|
}
|
||||||
|
|
||||||
|
let closed = close
|
||||||
|
.execute(
|
||||||
|
&json!({"ticket": target.id.clone(), "resolution": "Done"}).to_string(),
|
||||||
|
Default::default(),
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert!(closed.summary.contains(&target_key));
|
||||||
|
assert!(!closed.summary.contains(&target.id));
|
||||||
|
let content = closed.content.unwrap();
|
||||||
|
assert!(content.contains(&target_key));
|
||||||
|
assert!(!content.contains(&target.id));
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn ticket_workflow_tools_mark_ready_and_transition_state() {
|
async fn ticket_workflow_tools_mark_ready_and_transition_state() {
|
||||||
let temp = TempDir::new().unwrap();
|
let temp = TempDir::new().unwrap();
|
||||||
|
|||||||
@@ -62,8 +62,9 @@ impl WorkspaceHttpObjectiveBackend {
|
|||||||
.await
|
.await
|
||||||
.map_err(backend_error)?;
|
.map_err(backend_error)?;
|
||||||
let response = project_objective_detail(response).map_err(ToolError::ExecutionFailed)?;
|
let response = project_objective_detail(response).map_err(ToolError::ExecutionFailed)?;
|
||||||
|
let objective_ref = response.objective_ref().to_string();
|
||||||
Ok(ToolOutput {
|
Ok(ToolOutput {
|
||||||
summary: format!("Read objective {id}"),
|
summary: format!("Read objective {objective_ref}"),
|
||||||
content: Some(serde_json::to_string_pretty(&response).map_err(decode_error)?),
|
content: Some(serde_json::to_string_pretty(&response).map_err(decode_error)?),
|
||||||
attachments: Vec::new(),
|
attachments: Vec::new(),
|
||||||
})
|
})
|
||||||
@@ -710,6 +711,58 @@ mod tests {
|
|||||||
assert_eq!(link["required"], json!(["id", "ticket_id"]));
|
assert_eq!(link["required"], json!(["id", "ticket_id"]));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test(flavor = "multi_thread")]
|
||||||
|
async fn objective_show_summary_uses_projected_human_key() {
|
||||||
|
let listener = TcpListener::bind("127.0.0.1:0").unwrap();
|
||||||
|
let base_url = format!("http://{}", listener.local_addr().unwrap());
|
||||||
|
let server = thread::spawn(move || {
|
||||||
|
let (mut stream, _) = listener.accept().unwrap();
|
||||||
|
let mut buffer = [0_u8; 8192];
|
||||||
|
let len = stream.read(&mut buffer).unwrap();
|
||||||
|
let request = String::from_utf8_lossy(&buffer[..len]);
|
||||||
|
assert!(
|
||||||
|
request.starts_with("POST /api/w/workspace/objectives/00001INTERNAL/show HTTP/1.1")
|
||||||
|
);
|
||||||
|
let body = serde_json::json!({
|
||||||
|
"id": "00001INTERNAL",
|
||||||
|
"resource_key": "O-3",
|
||||||
|
"title": "Objective",
|
||||||
|
"body": "Body",
|
||||||
|
"state": "active",
|
||||||
|
"created_at": null,
|
||||||
|
"updated_at": null,
|
||||||
|
"linked_ticket_summaries": [],
|
||||||
|
"events": [],
|
||||||
|
"event_page": {"next_cursor": null, "has_more": false}
|
||||||
|
})
|
||||||
|
.to_string();
|
||||||
|
write!(
|
||||||
|
stream,
|
||||||
|
"HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\n\r\n{}",
|
||||||
|
body.len(),
|
||||||
|
body
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
});
|
||||||
|
let backend = WorkspaceHttpObjectiveBackend::new(Arc::new(
|
||||||
|
crate::worker::TestWorkspaceHttpClient::new("workspace", base_url),
|
||||||
|
));
|
||||||
|
|
||||||
|
let output = backend
|
||||||
|
.show(ShowObjectiveInput {
|
||||||
|
id: "00001INTERNAL".to_string(),
|
||||||
|
event_limit: None,
|
||||||
|
event_cursor: None,
|
||||||
|
})
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
server.join().unwrap();
|
||||||
|
assert_eq!(output.summary, "Read objective O-3");
|
||||||
|
assert!(!output.summary.contains("00001INTERNAL"));
|
||||||
|
assert!(!output.content.unwrap().contains("00001INTERNAL"));
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test(flavor = "multi_thread")]
|
#[tokio::test(flavor = "multi_thread")]
|
||||||
async fn objective_link_summaries_resolve_internal_ticket_ids_to_human_keys() {
|
async fn objective_link_summaries_resolve_internal_ticket_ids_to_human_keys() {
|
||||||
let listener = TcpListener::bind("127.0.0.1:0").unwrap();
|
let listener = TcpListener::bind("127.0.0.1:0").unwrap();
|
||||||
|
|||||||
@@ -84,6 +84,12 @@ pub(super) struct ModelObjectiveDetail {
|
|||||||
event_page: ModelObjectiveEventPage,
|
event_page: ModelObjectiveEventPage,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
impl ModelObjectiveDetail {
|
||||||
|
pub(super) fn objective_ref(&self) -> &str {
|
||||||
|
&self.objective
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[derive(Debug, Serialize)]
|
#[derive(Debug, Serialize)]
|
||||||
struct ModelWorkerSummary {
|
struct ModelWorkerSummary {
|
||||||
worker: String,
|
worker: String,
|
||||||
|
|||||||
@@ -876,12 +876,22 @@ impl WorkspaceHttpTicketBackend {
|
|||||||
Ok(TicketBackendOperationResult::Tickets(tickets))
|
Ok(TicketBackendOperationResult::Tickets(tickets))
|
||||||
}
|
}
|
||||||
TicketBackendOperation::Show { id } => {
|
TicketBackendOperation::Show { id } => {
|
||||||
let ticket = Self::request(
|
let ticket: Ticket = Self::request(
|
||||||
client,
|
client,
|
||||||
WorkspaceRequestMethod::Get,
|
WorkspaceRequestMethod::Get,
|
||||||
format!("{base}/{}/record", Self::ticket_path(&id)),
|
format!("{base}/{}/record", Self::ticket_path(&id)),
|
||||||
None,
|
None,
|
||||||
)?;
|
)?;
|
||||||
|
if !ticket
|
||||||
|
.meta
|
||||||
|
.resource_key
|
||||||
|
.as_deref()
|
||||||
|
.is_some_and(is_canonical_ticket_resource_key)
|
||||||
|
{
|
||||||
|
return Err(TicketError::Conflict(
|
||||||
|
"required Ticket human key is unavailable".to_string(),
|
||||||
|
));
|
||||||
|
}
|
||||||
Ok(TicketBackendOperationResult::Ticket(ticket))
|
Ok(TicketBackendOperationResult::Ticket(ticket))
|
||||||
}
|
}
|
||||||
TicketBackendOperation::Create { input } => {
|
TicketBackendOperation::Create { input } => {
|
||||||
|
|||||||
Reference in New Issue
Block a user