From 116d610ad051f44bd7704bcc57102286fd47d225 Mon Sep 17 00:00:00 2001 From: Hare Date: Thu, 27 Aug 2026 14:18:48 +0900 Subject: [PATCH] fix: project Ticket mutation outputs to human keys --- crates/ticket/src/tool.rs | 144 +++++++++++++++++- .../worker/src/feature/builtin/objective.rs | 55 ++++++- .../feature/builtin/resource_projection.rs | 6 + crates/worker/src/feature/builtin/ticket.rs | 12 +- 4 files changed, 210 insertions(+), 7 deletions(-) diff --git a/crates/ticket/src/tool.rs b/crates/ticket/src/tool.rs index 14d4c4f8..a38615b2 100644 --- a/crates/ticket/src/tool.rs +++ b/crates/ticket/src/tool.rs @@ -1223,10 +1223,17 @@ impl Tool for TicketQueueTool { ) -> Result { let params: TicketQueueParams = parse_input("TicketQueue", input_json)?; let queued_by = default_author(); - let outcome = self + let mut outcome = self .backend .queue_ready(TicketIdOrSlug::Query(params.ticket.clone()), &queued_by) .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::, _>>()?; Ok(json_output( format!( "Queued {} ticket(s) for Orchestrator", @@ -1264,15 +1271,17 @@ impl Tool for TicketWorkflowStateTool { self.backend .set_workflow_state(TicketIdOrSlug::Query(params.ticket.clone()), change) .map_err(|error| backend_error("TicketWorkflowState", error))?; + let ticket_ref = + model_ticket_reference(&self.backend, ¶ms.ticket, "TicketWorkflowState")?; Ok(json_output( format!( "Transitioned ticket {} state {} -> {}", - params.ticket, + ticket_ref, from.as_str(), to.as_str() ), json!({ - "ticket": params.ticket, + "ticket": ticket_ref, "from": from.as_str(), "to": to.as_str(), "state": to.as_str(), @@ -1296,9 +1305,10 @@ impl Tool for TicketCloseTool { MarkdownText::new(params.resolution), ) .map_err(|error| backend_error("TicketClose", error))?; + let ticket_ref = model_ticket_reference(&self.backend, ¶ms.ticket, "TicketClose")?; Ok(json_output( - format!("Closed ticket {}", params.ticket), - json!({ "ticket": params.ticket, "state": "closed", "ok": true }), + format!("Closed ticket {ticket_ref}"), + 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 { + 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 Deserialize<'de>>(tool: &str, input_json: &str) -> Result { serde_json::from_str(input_json) .map_err(|error| ToolError::InvalidArgument(format!("invalid {tool} input: {error}"))) @@ -1922,6 +1955,12 @@ mod tests { .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 { let (_, tool) = definition(); 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] async fn ticket_workflow_tools_mark_ready_and_transition_state() { let temp = TempDir::new().unwrap(); diff --git a/crates/worker/src/feature/builtin/objective.rs b/crates/worker/src/feature/builtin/objective.rs index a6fc79ea..2c91cefd 100644 --- a/crates/worker/src/feature/builtin/objective.rs +++ b/crates/worker/src/feature/builtin/objective.rs @@ -62,8 +62,9 @@ impl WorkspaceHttpObjectiveBackend { .await .map_err(backend_error)?; let response = project_objective_detail(response).map_err(ToolError::ExecutionFailed)?; + let objective_ref = response.objective_ref().to_string(); 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)?), attachments: Vec::new(), }) @@ -710,6 +711,58 @@ mod tests { 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")] async fn objective_link_summaries_resolve_internal_ticket_ids_to_human_keys() { let listener = TcpListener::bind("127.0.0.1:0").unwrap(); diff --git a/crates/worker/src/feature/builtin/resource_projection.rs b/crates/worker/src/feature/builtin/resource_projection.rs index d766f8e3..0eb32758 100644 --- a/crates/worker/src/feature/builtin/resource_projection.rs +++ b/crates/worker/src/feature/builtin/resource_projection.rs @@ -84,6 +84,12 @@ pub(super) struct ModelObjectiveDetail { event_page: ModelObjectiveEventPage, } +impl ModelObjectiveDetail { + pub(super) fn objective_ref(&self) -> &str { + &self.objective + } +} + #[derive(Debug, Serialize)] struct ModelWorkerSummary { worker: String, diff --git a/crates/worker/src/feature/builtin/ticket.rs b/crates/worker/src/feature/builtin/ticket.rs index feba75c9..0194d9d0 100644 --- a/crates/worker/src/feature/builtin/ticket.rs +++ b/crates/worker/src/feature/builtin/ticket.rs @@ -876,12 +876,22 @@ impl WorkspaceHttpTicketBackend { Ok(TicketBackendOperationResult::Tickets(tickets)) } TicketBackendOperation::Show { id } => { - let ticket = Self::request( + let ticket: Ticket = Self::request( client, WorkspaceRequestMethod::Get, format!("{base}/{}/record", Self::ticket_path(&id)), 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)) } TicketBackendOperation::Create { input } => {