diff --git a/crates/ticket/src/tool.rs b/crates/ticket/src/tool.rs index 7e383ba7a..08e5bef15 100644 --- a/crates/ticket/src/tool.rs +++ b/crates/ticket/src/tool.rs @@ -925,6 +925,7 @@ impl Tool for TicketCreateTool { ) -> Result { let params: TicketCreateParams = parse_input("TicketCreate", input_json)?; execute_ticket_create(&self.backend, params, _ctx) + .map(TicketCommandResult::into_tool_output) } } @@ -932,7 +933,21 @@ fn execute_ticket_create( backend: &TicketToolBackend, params: TicketCreateParams, _context: agen::tool::ToolExecutionContext, -) -> Result { +) -> Result { + let created = create_ticket(backend, params)?; + Ok(command_result( + format!("Created ticket {}", created.id), + json!(TicketRefOutput { + id: created.id, + state: "open".to_string(), + }), + )) +} + +fn create_ticket( + backend: &TicketToolBackend, + params: TicketCreateParams, +) -> Result { if params .state .is_some_and(|state| !matches!(state.into_state(), TicketState::Open)) @@ -963,16 +978,9 @@ fn execute_ticket_create( input.queued_at = params.queued_at; input.targets = params.targets; - let created = backend + backend .create(input) - .map_err(|error| backend_error("TicketCreate", error))?; - Ok(json_output( - format!("Created ticket {}", created.id), - json!(TicketRefOutput { - id: created.id, - state: "open".to_string(), - }), - )) + .map_err(|error| backend_error("TicketCreate", error)) } #[async_trait] @@ -984,6 +992,7 @@ impl Tool for TicketEditItemTool { ) -> Result { let params: TicketEditItemParams = parse_input("TicketEditItem", input_json)?; execute_ticket_edit_item(&self.backend, params, _ctx) + .map(TicketCommandResult::into_tool_output) } } @@ -991,7 +1000,7 @@ fn execute_ticket_edit_item( backend: &TicketToolBackend, params: TicketEditItemParams, _context: agen::tool::ToolExecutionContext, -) -> Result { +) -> Result { let body_replacement = match (params.old_string, params.new_string) { (Some(old_string), Some(new_string)) => Some(TicketBodyReplacement { old_string, @@ -1018,7 +1027,7 @@ fn execute_ticket_edit_item( let ticket = backend .edit_item(TicketIdOrSlug::from(params.ticket), edit) .map_err(|error| backend_error("TicketEditItem", error))?; - Ok(json_output( + Ok(command_result( format!("Edited ticket {}", ticket.meta.id), ticket_json( &ticket, @@ -1038,6 +1047,7 @@ impl Tool for TicketEditPostTool { ) -> Result { let params: TicketEditPostParams = parse_input("TicketEditPost", input_json)?; execute_ticket_edit_post(&self.backend, params, _ctx) + .map(TicketCommandResult::into_tool_output) } } @@ -1045,7 +1055,7 @@ fn execute_ticket_edit_post( backend: &TicketToolBackend, params: TicketEditPostParams, _context: agen::tool::ToolExecutionContext, -) -> Result { +) -> Result { let ticket = backend .edit_post( TicketIdOrSlug::from(params.ticket), @@ -1057,7 +1067,7 @@ fn execute_ticket_edit_post( }, ) .map_err(|error| backend_error("TicketEditPost", error))?; - Ok(json_output( + Ok(command_result( format!("Edited Ticket post in {}", ticket.meta.id), ticket_json( &ticket, @@ -1076,7 +1086,7 @@ impl Tool for QueryTicketTool { _ctx: agen::tool::ToolExecutionContext, ) -> Result { let params: QueryTicketParams = parse_input("QueryTicket", input_json)?; - execute_query_ticket(&self.backend, params, _ctx) + execute_query_ticket(&self.backend, params, _ctx).map(TicketCommandResult::into_tool_output) } } @@ -1084,7 +1094,7 @@ fn execute_query_ticket( backend: &TicketToolBackend, params: QueryTicketParams, _context: agen::tool::ToolExecutionContext, -) -> Result { +) -> Result { let (filter, state_filter, params_limit) = params .into_query() .map_err(|error| backend_error("QueryTicket", error))?; @@ -1106,7 +1116,7 @@ fn execute_query_ticket( limit, tickets: returned_tickets, }; - Ok(json_output( + Ok(command_result( format!( "Listed {} ticket(s) for state {state_filter}{}", output.returned, @@ -1124,7 +1134,7 @@ impl Tool for ShowTicketTool { _ctx: agen::tool::ToolExecutionContext, ) -> Result { let params: ShowTicketParams = parse_input("ShowTicket", input_json)?; - execute_show_ticket(&self.backend, params, _ctx) + execute_show_ticket(&self.backend, params, _ctx).map(TicketCommandResult::into_tool_output) } } @@ -1132,7 +1142,7 @@ fn execute_show_ticket( backend: &TicketToolBackend, params: ShowTicketParams, _context: agen::tool::ToolExecutionContext, -) -> Result { +) -> Result { let query = id_or_query(params.id, params.query)?; let event_limit = bounded(params.event_limit, DEFAULT_EVENT_LIMIT, MAX_EVENT_LIMIT); let artifact_limit = bounded( @@ -1153,7 +1163,7 @@ fn execute_show_ticket( ticket.meta.id, ticket.meta.workflow_state.as_str() ); - Ok(json_output( + Ok(command_result( summary, ticket_json(&ticket, event_limit, artifact_limit, body_max_bytes), )) @@ -1164,7 +1174,7 @@ fn execute_ticket_thread_event( tool_name: &str, kind: TicketEventKind, params: TicketThreadEventParams, -) -> Result { +) -> Result { let role = kind.as_str().to_string(); backend .add_event( @@ -1172,7 +1182,7 @@ fn execute_ticket_thread_event( NewTicketEvent::new(kind, params.body), ) .map_err(|error| backend_error(tool_name, error))?; - Ok(json_output( + Ok(command_result( format!("Appended {role} event to ticket {}", params.ticket), json!({ "ticket": params.ticket, "event": role, "ok": true }), )) @@ -1191,6 +1201,7 @@ impl Tool for TicketCommentTool { TicketEventKind::Comment, parse_input("TicketComment", input_json)?, ) + .map(TicketCommandResult::into_tool_output) } } @@ -1209,6 +1220,7 @@ macro_rules! impl_ticket_thread_event_tool { $kind, parse_input($name, input_json)?, ) + .map(TicketCommandResult::into_tool_output) } } }; @@ -1235,6 +1247,7 @@ impl Tool for TicketMarkReadyTool { ) -> Result { let params: TicketMarkReadyParams = parse_input("TicketMarkReady", input_json)?; execute_ticket_mark_ready(&self.backend, params, ctx) + .map(TicketCommandResult::into_tool_output) } } @@ -1242,7 +1255,7 @@ fn execute_ticket_mark_ready( backend: &TicketToolBackend, params: TicketMarkReadyParams, ctx: agen::tool::ToolExecutionContext, -) -> Result { +) -> Result { let ticket = backend .mark_ready( TicketIdOrSlug::Query(params.ticket.clone()), @@ -1254,7 +1267,7 @@ fn execute_ticket_mark_ready( }, ) .map_err(|error| backend_error("TicketMarkReady", error))?; - Ok(json_output( + Ok(command_result( format!("Marked ticket {} routing readiness ready", params.ticket), json!({ "ticket": ticket.meta.id, @@ -1274,6 +1287,7 @@ impl Tool for TicketIntakeReadyTool { ) -> Result { let params: TicketIntakeReadyParams = parse_input("TicketIntakeReady", input_json)?; execute_ticket_intake_ready(&self.backend, params, ctx) + .map(TicketCommandResult::into_tool_output) } } @@ -1281,7 +1295,7 @@ fn execute_ticket_intake_ready( backend: &TicketToolBackend, params: TicketIntakeReadyParams, ctx: agen::tool::ToolExecutionContext, -) -> Result { +) -> Result { let ticket = backend .mark_ready( TicketIdOrSlug::Query(params.ticket.clone()), @@ -1293,7 +1307,7 @@ fn execute_ticket_intake_ready( }, ) .map_err(|error| backend_error("TicketIntakeReady", error))?; - Ok(json_output( + Ok(command_result( format!( "Marked ticket {} routing readiness ready after intake", params.ticket @@ -1315,7 +1329,7 @@ impl Tool for TicketQueueTool { _ctx: agen::tool::ToolExecutionContext, ) -> Result { let params: TicketQueueParams = parse_input("TicketQueue", input_json)?; - execute_ticket_queue(&self.backend, params, _ctx) + execute_ticket_queue(&self.backend, params, _ctx).map(TicketCommandResult::into_tool_output) } } @@ -1323,7 +1337,7 @@ fn execute_ticket_queue( backend: &TicketToolBackend, params: TicketQueueParams, _context: agen::tool::ToolExecutionContext, -) -> Result { +) -> Result { let queued_by = default_author(); let mut outcome = backend .queue_ready(TicketIdOrSlug::Query(params.ticket.clone()), &queued_by) @@ -1335,7 +1349,7 @@ fn execute_ticket_queue( .into_iter() .map(|ticket| model_ticket_reference(backend, &ticket, "TicketQueue")) .collect::, _>>()?; - Ok(json_output( + Ok(command_result( format!( "Queued {} ticket(s) for Orchestrator", outcome.queued_tickets.len() @@ -1359,6 +1373,7 @@ impl Tool for TicketCaseUpdateTool { ) -> Result { let params: TicketCaseUpdateParams = parse_input("TicketCaseUpdate", input_json)?; execute_ticket_case_update(&self.backend, params, _ctx) + .map(TicketCommandResult::into_tool_output) } } @@ -1366,7 +1381,7 @@ fn execute_ticket_case_update( backend: &TicketToolBackend, params: TicketCaseUpdateParams, _context: agen::tool::ToolExecutionContext, -) -> Result { +) -> Result { let record = backend .update_case( ¶ms.ticket, @@ -1383,7 +1398,7 @@ fn execute_ticket_case_update( ) .map_err(|error| backend_error("TicketCaseUpdate", error))?; let reference = model_ticket_reference(backend, ¶ms.ticket, "TicketCaseUpdate")?; - Ok(json_output( + Ok(command_result( format!( "Updated Ticket {} case to {}", reference, record.meta.workflow_state @@ -1403,6 +1418,7 @@ impl Tool for TicketRelationRecordTool { ) -> Result { let params: TicketRelationRecordParams = parse_input("TicketRelationRecord", input_json)?; execute_ticket_relation_record(&self.backend, params, _ctx) + .map(TicketCommandResult::into_tool_output) } } @@ -1410,7 +1426,7 @@ fn execute_ticket_relation_record( backend: &TicketToolBackend, params: TicketRelationRecordParams, _context: agen::tool::ToolExecutionContext, -) -> Result { +) -> Result { let relation = NewTicketRelation { kind: params.kind.into_kind(), target: params.target.clone(), @@ -1420,7 +1436,7 @@ fn execute_ticket_relation_record( let output = backend .add_ticket_relation(TicketIdOrSlug::Id(params.ticket.clone()), relation) .map_err(|error| backend_error("TicketRelationRecord", error))?; - Ok(json_output( + Ok(command_result( format!( "Recorded ticket relation {} {} {}", output.ticket_id, output.kind, output.target @@ -1438,6 +1454,7 @@ impl Tool for TicketRelationRemoveTool { ) -> Result { let params: TicketRelationRemoveParams = parse_input("TicketRelationRemove", input_json)?; execute_ticket_relation_remove(&self.backend, params, _ctx) + .map(TicketCommandResult::into_tool_output) } } @@ -1445,7 +1462,7 @@ fn execute_ticket_relation_remove( backend: &TicketToolBackend, params: TicketRelationRemoveParams, _context: agen::tool::ToolExecutionContext, -) -> Result { +) -> Result { let output = backend .remove_ticket_relation( TicketIdOrSlug::Id(params.ticket), @@ -1453,7 +1470,7 @@ fn execute_ticket_relation_remove( TicketIdOrSlug::Id(params.target), ) .map_err(|error| backend_error("TicketRelationRemove", error))?; - Ok(json_output( + Ok(command_result( format!( "Removed ticket relation {} {} {}", output.ticket_id, output.kind, output.target @@ -1471,6 +1488,7 @@ impl Tool for TicketRelationQueryTool { ) -> Result { let params: TicketRelationQueryParams = parse_input("TicketRelationQuery", input_json)?; execute_ticket_relation_query(&self.backend, params, _ctx) + .map(TicketCommandResult::into_tool_output) } } @@ -1478,7 +1496,7 @@ fn execute_ticket_relation_query( backend: &TicketToolBackend, params: TicketRelationQueryParams, _context: agen::tool::ToolExecutionContext, -) -> Result { +) -> Result { let limit = bounded(params.limit, DEFAULT_LIST_LIMIT, MAX_LIST_LIMIT); let ticket = params.ticket.clone().map(TicketIdOrSlug::Id); let kind = params.kind.map(TicketRelationKindParam::into_kind); @@ -1492,7 +1510,7 @@ fn execute_ticket_relation_query( .take(limit) .map(|relation| ticket_relation_json(&relation)) .collect::>(); - Ok(json_output( + Ok(command_result( format!( "Found {} ticket relation(s){}", count, @@ -1517,6 +1535,7 @@ impl Tool for TicketOrchestrationPlanRecordTool { let params: TicketOrchestrationPlanRecordParams = parse_input("TicketOrchestrationPlanRecord", input_json)?; execute_ticket_orchestration_plan_record(&self.backend, params, _ctx) + .map(TicketCommandResult::into_tool_output) } } @@ -1524,7 +1543,7 @@ fn execute_ticket_orchestration_plan_record( backend: &TicketToolBackend, params: TicketOrchestrationPlanRecordParams, _context: agen::tool::ToolExecutionContext, -) -> Result { +) -> Result { let accepted_plan = params.accepted_plan.map(|plan| AcceptedOrchestrationPlan { summary: plan.summary, branch: plan.branch, @@ -1541,7 +1560,7 @@ fn execute_ticket_orchestration_plan_record( let output = backend .add_orchestration_plan_record(TicketIdOrSlug::Query(params.ticket.clone()), record) .map_err(|error| backend_error("TicketOrchestrationPlanRecord", error))?; - Ok(json_output( + Ok(command_result( format!( "Recorded orchestration plan {} for ticket {}", output.kind, params.ticket @@ -1560,6 +1579,7 @@ impl Tool for TicketOrchestrationPlanQueryTool { let params: TicketOrchestrationPlanQueryParams = parse_input("TicketOrchestrationPlanQuery", input_json)?; execute_ticket_orchestration_plan_query(&self.backend, params, _ctx) + .map(TicketCommandResult::into_tool_output) } } @@ -1567,7 +1587,7 @@ fn execute_ticket_orchestration_plan_query( backend: &TicketToolBackend, params: TicketOrchestrationPlanQueryParams, _context: agen::tool::ToolExecutionContext, -) -> Result { +) -> Result { let limit = bounded(params.limit, DEFAULT_LIST_LIMIT, MAX_LIST_LIMIT); let ticket = params.ticket.clone().map(TicketIdOrSlug::Query); let kind = params @@ -1583,7 +1603,7 @@ fn execute_ticket_orchestration_plan_query( .take(limit) .map(|record| serde_json::to_value(record).unwrap_or_else(|_| json!({}))) .collect::>(); - Ok(json_output( + Ok(command_result( format!( "Found {} orchestration plan record(s){}", count, @@ -1607,6 +1627,7 @@ impl Tool for TicketDoctorTool { ) -> Result { let params: TicketDoctorParams = parse_input("TicketDoctor", input_json)?; execute_ticket_doctor(&self.backend, params, _ctx) + .map(TicketCommandResult::into_tool_output) } } @@ -1614,13 +1635,13 @@ fn execute_ticket_doctor( backend: &TicketToolBackend, params: TicketDoctorParams, _context: agen::tool::ToolExecutionContext, -) -> Result { +) -> Result { let limit = bounded(params.limit, DEFAULT_DIAGNOSTIC_LIMIT, MAX_DIAGNOSTIC_LIMIT); let report = backend .doctor() .map_err(|error| backend_error("TicketDoctor", error))?; let output = doctor_output(report, limit); - Ok(json_output( + Ok(command_result( format!( "Ticket doctor: {} error(s), {} diagnostic(s){}", output.error_count, @@ -1640,6 +1661,7 @@ impl Tool for TicketDependencyCheckTool { ) -> Result { let params: TicketDependencyCheckParams = parse_input("TicketDependencyCheck", input_json)?; execute_ticket_dependency_check(&self.backend, params, _ctx) + .map(TicketCommandResult::into_tool_output) } } @@ -1647,11 +1669,11 @@ fn execute_ticket_dependency_check( backend: &TicketToolBackend, params: TicketDependencyCheckParams, _context: agen::tool::ToolExecutionContext, -) -> Result { +) -> Result { let check = backend .dependency_check(TicketIdOrSlug::Query(params.ticket.clone())) .map_err(|error| backend_error("TicketDependencyCheck", error))?; - Ok(json_output( + Ok(command_result( format!( "Ticket {} dependency check: {}", params.ticket, @@ -1676,6 +1698,30 @@ pub async fn execute_ticket_command( command: Value, context: agen::tool::ToolExecutionContext, ) -> Result { + execute_ticket_command_result(backend, operation, command, context) + .map(TicketCommandResult::into_tool_output) +} + +/// Execute a converted command and return domain data without rendering a Tool result. +/// +/// This shares validation, Backend calls and operation identities with ordinary Tools. +/// Selecting value output adds no retries or reference lookups. +pub async fn execute_ticket_command_value( + backend: TicketToolBackend, + operation: &str, + command: Value, + context: agen::tool::ToolExecutionContext, +) -> Result { + execute_ticket_command_result(backend, operation, command, context) + .map(TicketCommandResult::into_value) +} + +fn execute_ticket_command_result( + backend: TicketToolBackend, + operation: &str, + command: Value, + context: agen::tool::ToolExecutionContext, +) -> Result { fn convert(command: Value) -> Result { serde_json::from_value(command) .map_err(|error| ToolError::InvalidArgument(format!("invalid Ticket command: {error}"))) @@ -2060,11 +2106,87 @@ fn truncate_text(text: &str, max_bytes: usize) -> String { out } -fn json_output(summary: String, value: impl Serialize) -> ToolOutput { - ToolOutput { +// Retain typed payloads until presentation so ordinary Tool JSON keeps its field +// order while native callers never format or reparse Tool text. +#[derive(Serialize)] +#[serde(untagged)] +enum TicketCommandValue { + Json(Value), + Query(QueryTicketOutput), + RelationQuery(TicketRelationQueryOutput), + PlanRecord(OrchestrationPlanRecord), + PlanQuery(TicketOrchestrationPlanQueryOutput), + Doctor(TicketDoctorOutput), + Dependency(crate::TicketDependencyCheck), +} + +impl From for TicketCommandValue { + fn from(value: Value) -> Self { + Self::Json(value) + } +} + +impl From for TicketCommandValue { + fn from(value: QueryTicketOutput) -> Self { + Self::Query(value) + } +} + +impl From for TicketCommandValue { + fn from(value: TicketRelationQueryOutput) -> Self { + Self::RelationQuery(value) + } +} + +impl From for TicketCommandValue { + fn from(value: OrchestrationPlanRecord) -> Self { + Self::PlanRecord(value) + } +} + +impl From for TicketCommandValue { + fn from(value: TicketOrchestrationPlanQueryOutput) -> Self { + Self::PlanQuery(value) + } +} + +impl From for TicketCommandValue { + fn from(value: TicketDoctorOutput) -> Self { + Self::Doctor(value) + } +} + +impl From for TicketCommandValue { + fn from(value: crate::TicketDependencyCheck) -> Self { + Self::Dependency(value) + } +} + +struct TicketCommandResult { + summary: String, + value: TicketCommandValue, +} + +impl TicketCommandResult { + fn into_tool_output(self) -> ToolOutput { + ToolOutput { + summary: self.summary, + content: Some( + serde_json::to_string_pretty(&self.value).unwrap_or_else(|_| "{}".to_string()), + ), + attachments: Vec::new(), + } + } + + fn into_value(self) -> Value { + serde_json::to_value(self.value).unwrap_or_else(|_| json!({})) + } +} + +fn command_result(summary: String, value: impl Into) -> TicketCommandResult { + TicketCommandResult { summary, - content: Some(serde_json::to_string_pretty(&value).unwrap_or_else(|_| "{}".to_string())), - attachments: Vec::new(), + value: value.into(), } } @@ -2254,11 +2376,46 @@ mod tests { .expect("tool exists") } + async fn assert_value_read_matches_tools( + backend: &crate::SqliteTicketBackend, + operation: &str, + input: Value, + ) -> Value { + let value = execute_ticket_command_value( + backend.clone().into(), + operation, + input.clone(), + Default::default(), + ) + .await + .unwrap(); + let wrapped = execute_ticket_command( + backend.clone().into(), + operation, + input.clone(), + Default::default(), + ) + .await + .unwrap(); + let ordinary = tool_by_name(backend.clone(), operation) + .execute(&input.to_string(), Default::default()) + .await + .unwrap(); + assert_eq!(wrapped.summary, ordinary.summary); + assert_eq!(wrapped.content, ordinary.content); + assert!(ordinary.attachments.is_empty()); + assert_eq!( + value, + serde_json::from_str::(ordinary.content.as_deref().unwrap()).unwrap() + ); + value + } + #[tokio::test] async fn internal_commands_share_persisted_edits_post_cas_and_dependency_results_with_tools() { let temp = TempDir::new().unwrap(); let backend = backend(&temp); - let created = execute_ticket_command( + let created = execute_ticket_command_value( backend.clone().into(), "TicketCreate", json!({"title":"Shared execution", "body":"Original"}), @@ -2266,10 +2423,11 @@ mod tests { ) .await .unwrap(); - let id = serde_json::from_str::(created.content.as_deref().unwrap()).unwrap()["id"] - .as_str() - .unwrap() - .to_owned(); + let id = created["id"].as_str().unwrap().to_owned(); + assert_eq!(created["state"], "open"); + let listed = + assert_value_read_matches_tools(&backend, "QueryTicket", json!({"state":"all"})).await; + assert_eq!(listed["count"], 1, "creation must execute only once"); let edit = tool_by_name(backend.clone(), "TicketEditItem"); edit.execute( &json!({"ticket":id, "old_string":"Original", "new_string":"Tool edit"}).to_string(), @@ -2277,7 +2435,7 @@ mod tests { ) .await .unwrap(); - execute_ticket_command( + let edited = execute_ticket_command_value( backend.clone().into(), "TicketEditItem", json!({"ticket":id, "old_string":"Tool edit", "new_string":"Command edit"}), @@ -2285,7 +2443,8 @@ mod tests { ) .await .unwrap(); - execute_ticket_command( + assert_eq!(edited["body"], "Command edit"); + let appended = execute_ticket_command_value( backend.clone().into(), "TicketComment", json!({"ticket":id, "body":"Discussion"}), @@ -2293,7 +2452,13 @@ mod tests { ) .await .unwrap(); + assert_eq!(appended, json!({"ticket":id, "event":"comment", "ok":true})); let before = backend.show(id.clone().into()).unwrap(); + assert_eq!( + before.events.len(), + 2, + "one initial post and one appended comment" + ); assert_eq!(before.document.body.as_str(), "Command edit"); let post = before .events @@ -2301,7 +2466,7 @@ mod tests { .find(|event| !event.is_initial) .unwrap(); let input = json!({"ticket":id, "event_ref":post.event_ref, "expected_body":"Discussion", "body":"Updated"}); - execute_ticket_command( + let edited_post = execute_ticket_command_value( backend.clone().into(), "TicketEditPost", input.clone(), @@ -2309,7 +2474,7 @@ mod tests { ) .await .unwrap(); - let native_error = execute_ticket_command( + let native_error = execute_ticket_command_value( backend.clone().into(), "TicketEditPost", input.clone(), @@ -2323,6 +2488,20 @@ mod tests { .unwrap_err(); assert_eq!(native_error.to_string(), tool_error.to_string()); let after = backend.show(id.clone().into()).unwrap(); + assert_eq!( + after.events.len(), + 2, + "editing must not append or retry a comment" + ); + assert_eq!( + edited_post, + ticket_json( + &after, + DEFAULT_EVENT_LIMIT, + DEFAULT_ARTIFACT_LIMIT, + DEFAULT_BODY_MAX_BYTES + ) + ); assert_eq!( after .events @@ -2333,20 +2512,31 @@ mod tests { .as_str(), "Updated" ); - let input = json!({"ticket":id}); - let native = execute_ticket_command( + let dependency_output = execute_ticket_command( backend.clone().into(), "TicketDependencyCheck", - input.clone(), + json!({"ticket":id}), Default::default(), ) .await .unwrap(); - let ordinary = tool_by_name(backend, "TicketDependencyCheck") - .execute(&input.to_string(), Default::default()) - .await - .unwrap(); - assert_eq!(native.content, ordinary.content); + let check = backend.dependency_check(id.clone().into()).unwrap(); + assert_eq!( + dependency_output.content.as_deref(), + Some(serde_json::to_string_pretty(&check).unwrap().as_str()) + ); + assert_eq!( + dependency_output.summary, + format!("Ticket {id} dependency check: queueable") + ); + for (operation, input) in [ + ("TicketDependencyCheck", json!({"ticket":id})), + ("ShowTicket", json!({"id":id})), + ("QueryTicket", json!({"state":"all"})), + ("TicketDoctor", json!({})), + ] { + assert_value_read_matches_tools(&backend, operation, input).await; + } } #[tokio::test] @@ -2356,7 +2546,7 @@ mod tests { let source = backend.create(NewTicket::new("Source")).unwrap().id; let target = backend.create(NewTicket::new("Target")).unwrap().id; let relation = json!({"ticket":source, "kind":"related", "target":target}); - execute_ticket_command( + let recorded = execute_ticket_command_value( backend.clone().into(), "TicketRelationRecord", relation.clone(), @@ -2365,20 +2555,13 @@ mod tests { .await .unwrap(); let query = json!({"ticket":source}); - let native = execute_ticket_command( - backend.clone().into(), - "TicketRelationQuery", - query.clone(), - Default::default(), - ) - .await - .unwrap(); - let ordinary = tool_by_name(backend.clone(), "TicketRelationQuery") - .execute(&query.to_string(), Default::default()) - .await - .unwrap(); - assert_eq!(native.content, ordinary.content); - execute_ticket_command( + let queried = + assert_value_read_matches_tools(&backend, "TicketRelationQuery", query.clone()).await; + assert_eq!(queried["count"], 1); + assert_eq!(queried["relations"][0], recorded); + assert_eq!(recorded["ticket_id"], source); + assert_eq!(recorded["target"], target); + let removed = execute_ticket_command_value( backend.clone().into(), "TicketRelationRemove", relation, @@ -2386,8 +2569,12 @@ mod tests { ) .await .unwrap(); + assert_eq!(removed, recorded); + let empty = + assert_value_read_matches_tools(&backend, "TicketRelationQuery", query.clone()).await; + assert_eq!(empty["count"], 0); let plan = json!({"ticket":source, "kind":"accepted_plan", "accepted_plan":{"summary":"Shared execution plan", "branch":"work"}}); - execute_ticket_command( + let planned = execute_ticket_command_value( backend.clone().into(), "TicketOrchestrationPlanRecord", plan, @@ -2395,21 +2582,19 @@ mod tests { ) .await .unwrap(); - let native = execute_ticket_command( - backend.clone().into(), + let queried = assert_value_read_matches_tools( + &backend, "TicketOrchestrationPlanQuery", query.clone(), - Default::default(), ) - .await - .unwrap(); - let ordinary = tool_by_name(backend.clone(), "TicketOrchestrationPlanQuery") - .execute(&query.to_string(), Default::default()) - .await - .unwrap(); - assert_eq!(native.content, ordinary.content); + .await; + assert_eq!( + queried["count"], 1, + "one durable plan, without duplicate execution" + ); + assert_eq!(queried["records"][0], planned); let invalid = json!({"ticket":source, "kind":"accepted_plan"}); - let native = execute_ticket_command( + let native = execute_ticket_command_value( backend.clone().into(), "TicketOrchestrationPlanRecord", invalid.clone(), @@ -2423,7 +2608,7 @@ mod tests { .unwrap_err(); assert_eq!(native.to_string(), ordinary.to_string()); assert!(matches!( - execute_ticket_command( + execute_ticket_command_value( backend.clone().into(), "TicketRelationRecord", json!({"ticket":source,"target":target,"kind":"invalid"}), @@ -2433,7 +2618,7 @@ mod tests { Err(ToolError::InvalidArgument(_)) )); assert!( - execute_ticket_command(backend.into(), "Unknown", json!({}), Default::default()) + execute_ticket_command_value(backend.into(), "Unknown", json!({}), Default::default()) .await .unwrap_err() .to_string() diff --git a/crates/worker/src/checkout_race_tests.rs b/crates/worker/src/checkout_race_tests.rs index def2a253d..75ed4da61 100644 --- a/crates/worker/src/checkout_race_tests.rs +++ b/crates/worker/src/checkout_race_tests.rs @@ -282,6 +282,7 @@ fn deadline_provider(router: Arc) -> Arc { fn context() -> WipCallContext { WipCallContext { + result_namespaces: Default::default(), execution: Default::default(), security_context: "checkout-race-tests".into(), } diff --git a/crates/worker/src/checkout_wip_tests.rs b/crates/worker/src/checkout_wip_tests.rs index 0577b22f2..36929da6f 100644 --- a/crates/worker/src/checkout_wip_tests.rs +++ b/crates/worker/src/checkout_wip_tests.rs @@ -424,6 +424,7 @@ async fn checkout_wip_provider_validator_rejects_same_content_inode_substitution "write", &BTreeMap::from([("content".into(), Value::String("bad".into()))]), WipCallContext { + result_namespaces: Default::default(), execution: Default::default(), security_context: "checkout-test-worker".into(), }, @@ -966,6 +967,7 @@ async fn checkout_list_permission_cannot_be_bypassed_with_glob_or_command_identi "list", &BTreeMap::new(), WipCallContext { + result_namespaces: Default::default(), execution: Default::default(), security_context: "fixture".into(), }, @@ -1030,6 +1032,7 @@ async fn common_scope_rejects_wrong_namespace_scope_ref_and_old_target_authority .call( forged, WipCallContext { + result_namespaces: Default::default(), execution: Default::default(), security_context: "fixture".into(), }, @@ -1041,13 +1044,13 @@ async fn common_scope_rejects_wrong_namespace_scope_ref_and_old_target_authority } let forged = request(p.interface.clone(), Some("old".into())); assert!( - matches!(r.host.call(forged, WipCallContext { execution:Default::default(),security_context:"fixture".into() }).await, Err(WipOperationError::Protocol(e)) if e.code == ProtocolErrorCode::InterfaceMismatch) + matches!(r.host.call(forged, WipCallContext { result_namespaces: Default::default(), execution:Default::default(),security_context:"fixture".into() }).await, Err(WipOperationError::Protocol(e)) if e.code == ProtocolErrorCode::InterfaceMismatch) ); router.detach(&main).await.unwrap(); router.attach(main, session(&dir, true)).unwrap(); let forged = request(p.interface.clone(), None); assert!( - matches!(r.host.call(forged, WipCallContext { execution:Default::default(),security_context:"fixture".into() }).await, Err(WipOperationError::Protocol(e)) if e.code == ProtocolErrorCode::ValidatorMismatch) + matches!(r.host.call(forged, WipCallContext { result_namespaces: Default::default(), execution:Default::default(),security_context:"fixture".into() }).await, Err(WipOperationError::Protocol(e)) if e.code == ProtocolErrorCode::ValidatorMismatch) ); } @@ -1073,6 +1076,7 @@ fn observed_common_request( } fn file_context() -> WipCallContext { WipCallContext { + result_namespaces: Default::default(), execution: Default::default(), security_context: "file-contract-test".into(), } diff --git a/crates/worker/src/feature/builtin/drive/tests.rs b/crates/worker/src/feature/builtin/drive/tests.rs index df2f98ab8..06059946a 100644 --- a/crates/worker/src/feature/builtin/drive/tests.rs +++ b/crates/worker/src/feature/builtin/drive/tests.rs @@ -703,6 +703,7 @@ async fn oversized_images_and_unknown_arguments_fail_before_binary_transfer() { fn native_context() -> crate::wip::WipCallContext { crate::wip::WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "drive-test".into(), } diff --git a/crates/worker/src/feature/builtin/manage_workdir/wip.rs b/crates/worker/src/feature/builtin/manage_workdir/wip.rs index 004cb8093..216d8dda5 100644 --- a/crates/worker/src/feature/builtin/manage_workdir/wip.rs +++ b/crates/worker/src/feature/builtin/manage_workdir/wip.rs @@ -1471,6 +1471,7 @@ pub(crate) mod tests { } fn context() -> WipCallContext { WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::default(), security_context: "test".into(), } @@ -1839,6 +1840,7 @@ mod lifetime_tests { }; let args = BTreeMap::from([("limit".into(), Value::Integer(50))]); let context = || WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::default(), security_context: "test".into(), }; diff --git a/crates/worker/src/feature/builtin/merge_request.rs b/crates/worker/src/feature/builtin/merge_request.rs index 0ff51f03a..9c603172c 100644 --- a/crates/worker/src/feature/builtin/merge_request.rs +++ b/crates/worker/src/feature/builtin/merge_request.rs @@ -24,7 +24,9 @@ //! //! Read returns the existing bounded thread page, keeping linked Tickets as references. Successful //! item-read response hashes are observed; mutations discard observations. Receipts do not fabricate -//! complete representations or local mutation generations. Open results include the new item path. +//! complete representations or local mutation generations. Native results separate a typed subject +//! Entry and explicit related Entries from the bounded domain JSON; the private `result` module +//! owns that projection boundary. //! Item responses are checked against the bound MR; subject mismatches after mutations are not //! treated as unexecuted operations. Native omission and explicit nullable unions are distinct; //! native input changes do not change ordinary Tool omission/null behavior. @@ -199,6 +201,7 @@ impl Kind { } mod command; mod native; +mod result; #[async_trait] impl Tool for MergeRequestTool { @@ -497,6 +500,7 @@ pub fn mount_workspace_http_merge_request_wip( }), })?; registry.replace_compatibility_tools(&collection_route, claimed_tools)?; + registry.publish_result_namespace(namespace_route)?; Ok(()) } @@ -528,33 +532,26 @@ impl WipOperationHandler for MergeRequestCollectionWipHandler { .tools .get(operation) .ok_or_else(merge_request_operation_not_found)?; - let output = + let namespaces = context.result_namespaces.clone(); + let response = execute_native_merge_request_tool(tool, &self.permissions, arguments, None, context) .await?; - let mut response = merge_request_tool_output_json(output)?; - match tool.kind { - Kind::Open => { - let reference = merge_request_reference(&response) - .filter(|reference| is_merge_request_route_reference(reference)) - .ok_or_else(|| { - WipOperationError::OutcomeUnknown( - "OpenMergeRequest returned no routable Merge Request identity".into(), - ) - })? - .to_string(); - record_merge_request_observation(&self.observations, &reference, &response, true); - if let Some(object) = response.as_object_mut() { - object.insert( - "path".into(), - Value::String(format!("{}/{}", self.collection_route, reference)), - ); - } - } - _ => {} - } - Ok(WipOperationOutput::native( - json_to_wip(&response).map_err(WipOperationError::OutcomeUnknown)?, - )) + let reference = merge_request_reference(&response) + .filter(|reference| is_merge_request_route_reference(reference)) + .ok_or_else(|| { + WipOperationError::OutcomeUnknown( + "OpenMergeRequest returned no routable Merge Request identity".into(), + ) + })? + .to_string(); + record_merge_request_observation(&self.observations, &reference, &response, true); + Ok(WipOperationOutput::native(result::value( + tool.kind, + response, + &reference, + &self.collection_route, + namespaces.root("ticket"), + )?)) } } @@ -592,6 +589,7 @@ impl WipDynamicItemResolver for MergeRequestItemResolver { tools: self.tools.clone(), permissions: self.permissions.clone(), merge_request_id: item_reference.into(), + collection_route: self.collection_route.clone(), observations: Arc::clone(&self.observations), }), }) @@ -602,6 +600,7 @@ struct MergeRequestItemWipHandler { tools: HashMap, permissions: Option, merge_request_id: String, + collection_route: String, observations: MergeRequestObservations, } @@ -617,7 +616,8 @@ impl WipOperationHandler for MergeRequestItemWipHandler { .tools .get(operation) .ok_or_else(merge_request_operation_not_found)?; - let output = execute_native_merge_request_tool( + let namespaces = context.result_namespaces.clone(); + let response = execute_native_merge_request_tool( tool, &self.permissions, arguments, @@ -625,7 +625,6 @@ impl WipOperationHandler for MergeRequestItemWipHandler { context, ) .await?; - let response = merge_request_tool_output_json(output)?; ensure_merge_request_response_identity( &response, &self.merge_request_id, @@ -637,9 +636,13 @@ impl WipOperationHandler for MergeRequestItemWipHandler { &response, tool.kind.mutating(), ); - Ok(WipOperationOutput::native( - json_to_wip(&response).map_err(WipOperationError::OutcomeUnknown)?, - )) + Ok(WipOperationOutput::native(result::value( + tool.kind, + response, + &self.merge_request_id, + &self.collection_route, + namespaces.root("ticket"), + )?)) } } @@ -649,7 +652,7 @@ async fn execute_native_merge_request_tool( arguments: &BTreeMap, bound_merge_request: Option<&str>, _context: WipCallContext, -) -> Result { +) -> Result { if tool.kind == Kind::Review { let reviewer = tool.client.reviewer_context().ok_or_else(|| { merge_request_protocol_failure( @@ -666,10 +669,10 @@ async fn execute_native_merge_request_tool( } let input = native_merge_request_input(tool, arguments, bound_merge_request)?; // Legacy Tool permission patterns remain an additional fail-closed ceiling, evaluated on - // normalized domain fields. Backend authorization is always performed by command::execute. + // normalized domain fields. Backend authorization is always performed by command::execute_value. authorize_native_merge_request(permissions, &tool.name, &input)?; let command = native::command(tool.kind, input)?; - command::execute(tool.client.clone(), command) + command::execute_value(tool.client.clone(), command) .map_err(|error| map_merge_request_tool_error(error, tool.kind.mutating())) } @@ -761,14 +764,6 @@ fn merge_request_http_status(message: &str) -> Option { .ok() } -fn merge_request_tool_output_json(output: ToolOutput) -> Result { - match output.content { - Some(content) => serde_json::from_str(&content) - .map_err(|error| WipOperationError::OutcomeUnknown(error.to_string())), - None => Ok(json!({"summary": output.summary, "ok": true})), - } -} - fn ensure_merge_request_response_identity( response: &Value, bound_reference: &str, @@ -1363,6 +1358,7 @@ mod tests { &input, Some("MR-2"), WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "reviewer".into(), }, @@ -1383,6 +1379,7 @@ mod tests { &input, Some("MR-1"), WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "reviewer".into(), }, @@ -1480,14 +1477,24 @@ mod tests { ), WipOperationError::OutcomeUnknown(_) )); + let invalid_json = Arc::new(RecordingWorkspaceClient::new(vec![WorkspaceResponse { + status: 200, + body: "not-json".into(), + }])); + let error = command::execute_value( + invalid_json.clone(), + command::Command::Read { + merge_request_id: "MR-1".into(), + after: None, + limit: None, + }, + ) + .unwrap_err(); assert!(matches!( - merge_request_tool_output_json(ToolOutput { - summary: "OpenMergeRequest".into(), - content: Some("not-json".into()), - attachments: vec![], - }), - Err(WipOperationError::OutcomeUnknown(_)) + map_merge_request_tool_error(error, false), + WipOperationError::OutcomeUnknown(_) )); + assert_eq!(invalid_json.requests.lock().unwrap().len(), 1); assert!(matches!( ensure_merge_request_response_identity( &json!({"merge_request_id": "MR-2"}), @@ -1665,6 +1672,7 @@ mod tests { &BTreeMap::new(), Some("MR-1"), WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "test".into(), }, diff --git a/crates/worker/src/feature/builtin/merge_request/command.rs b/crates/worker/src/feature/builtin/merge_request/command.rs index 4e6e7ea3a..9ac78ea1c 100644 --- a/crates/worker/src/feature/builtin/merge_request/command.rs +++ b/crates/worker/src/feature/builtin/merge_request/command.rs @@ -104,13 +104,28 @@ impl Command { } } -/// The single domain dispatch boundary. Backend authorization precedes every data request. +/// Ordinary Tool output serializes the shared domain value only at this presentation boundary. pub(super) fn execute( client: Arc, command: Command, ) -> Result { let kind = command.kind(); - let executor = Executor { client, kind }; + let value = execute_value(client, command)?; + Ok(ToolOutput { + summary: kind.name().into(), + content: Some(value.to_string()), + attachments: vec![], + }) +} + +/// The single domain dispatch boundary. Decode the documented Workspace JSON response once; +/// native consumers never recover domain references by reparsing a formatted ToolOutput. +/// Backend authorization precedes every data request. +pub(super) fn execute_value( + client: Arc, + command: Command, +) -> Result { + let executor = Executor { client }; let ws = executor.client.workspace_id().ok_or_else(|| { ToolError::ExecutionFailed("Merge Request operations require Workspace identity".into()) })?; @@ -240,16 +255,11 @@ pub(super) fn execute( if !res.is_success() { return Err(api_error("Merge Request API", &res)); } - Ok(ToolOutput { - summary: kind.name().into(), - content: Some(res.body), - attachments: vec![], - }) + response_value(&res.body) } struct Executor { client: Arc, - kind: Kind, } impl Executor { @@ -267,7 +277,7 @@ impl Executor { merge_request_id: &str, after: Option, limit: Option, - ) -> Result { + ) -> Result { self.authorize( "merge_request.read", PermissionResource::MergeRequest { @@ -295,10 +305,12 @@ impl Executor { if !response.is_success() { return Err(api_error("Merge Request API", &response)); } - Ok(ToolOutput { - summary: self.kind.name().into(), - content: Some(response.body), - attachments: vec![], - }) + response_value(&response.body) } } + +fn response_value(body: &str) -> Result { + serde_json::from_str(body).map_err(|_| { + ToolError::ExecutionFailed("Merge Request API returned invalid JSON after dispatch".into()) + }) +} diff --git a/crates/worker/src/feature/builtin/merge_request/native.rs b/crates/worker/src/feature/builtin/merge_request/native.rs index 980a6f666..aec8ffaa3 100644 --- a/crates/worker/src/feature/builtin/merge_request/native.rs +++ b/crates/worker/src/feature/builtin/merge_request/native.rs @@ -198,7 +198,9 @@ impl ReferenceRoots { } pub(super) fn declaration(contract: &Contract, roots: &ReferenceRoots) -> OperationDeclaration { - contract.declaration(&roots.contract_roots()) + let mut declaration = contract.declaration(&roots.contract_roots()); + declaration.returns = result::declaration(); + declaration } pub(super) fn convert( diff --git a/crates/worker/src/feature/builtin/merge_request/native/domain_tests.rs b/crates/worker/src/feature/builtin/merge_request/native/domain_tests.rs index 87586056e..de6c333e7 100644 --- a/crates/worker/src/feature/builtin/merge_request/native/domain_tests.rs +++ b/crates/worker/src/feature/builtin/merge_request/native/domain_tests.rs @@ -59,6 +59,7 @@ async fn native_backend_conflict_is_a_domain_rejection_not_a_type_rejection_or_r &input, Some("MR-1"), WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "test".into(), }, diff --git a/crates/worker/src/feature/builtin/merge_request/result.rs b/crates/worker/src/feature/builtin/merge_request/result.rs new file mode 100644 index 000000000..63961245a --- /dev/null +++ b/crates/worker/src/feature/builtin/merge_request/result.rs @@ -0,0 +1,139 @@ +//! Native MR results separate navigation from bounded domain evidence. +//! +//! `entry` is the route-bound subject (the response identity for open). `data` preserves the +//! Backend response, including display IDs, body/findings, repository selectors, page/state and +//! exact source/event IDs. In particular, a review's full linked-MR snapshot must remain intact: +//! its `subject_ref` strings are evidence, not navigable Objects. `links` selects only the +//! documented PublicMergeRequest.ticket_ids, Detail.linked_tickets[].ticket_id and +//! Review{Requested,}Event.ticket_merge_request_subjects[].merge_request_id fields. The latter +//! occur in bounded thread pages, readiness.review and review receipts; no arbitrary JSON, body, +//! finding path, or textual ID is searched recursively. +//! +//! References are coordinates, not existence observations or authority. Related Entries are +//! emitted only for roots in the final Host registry publication snapshot supplied on each call, +//! never from allocated input namespaces or an early mount-time snapshot. Missing publication or an +//! unroutable identity omits the optional Entry but retains already-disclosed domain IDs. There is +//! no prefetch, all-item registration, post-mutation target read, or retry: selected Entry Inspect +//! and read use the target's existing permission/deletion boundary independently of this success. + +use super::*; +use std::collections::BTreeSet; +use wip_protocol::{FieldDeclaration, ReturnDeclaration, TypeExpr}; + +fn field(name: &str, required: bool, ty: TypeExpr) -> FieldDeclaration { + FieldDeclaration { + name: name.into(), + required, + documentation: None, + r#type: ty, + } +} + +pub(super) fn declaration() -> ReturnDeclaration { + let references = || TypeExpr::List { + items: Box::new(TypeExpr::Record { + fields: vec![ + field("id", true, TypeExpr::String), + field("entry", false, TypeExpr::Entry), + ], + }), + }; + ReturnDeclaration { + documentation: Some(Documentation { + summary: "Bounded MR result: entry is the subject; data retains domain evidence; links contains typed navigation only.".into(), + details: Some("IDs, source refs, bodies, state and page metadata remain data, not inferred references. Related entry is omitted for unpublished spaces or unroutable IDs; publication is not existence, permission, or observed state. Inspect only selected Entries, then invoke their read operations. Failed target resolution never retries the originating mutation.".into()), + }), + r#type: TypeExpr::Record { fields: vec![ + field("entry", true, TypeExpr::Entry), + field("data", true, TypeExpr::Json), + field("links", true, TypeExpr::Record { fields: vec![ + field("tickets", true, references()), + field("merge_requests", true, references()), + ] }), + ] }, + } +} + +fn ticket_reference(id: &str) -> bool { + // Match the Ticket item's public route grammar, not MR's broader single-segment grammar. + id.strip_prefix("T-").is_some_and(|sequence| { + !sequence.is_empty() && sequence.bytes().all(|byte| byte.is_ascii_digit()) + }) || ((8..=128).contains(&id.len()) && id.bytes().all(|byte| byte.is_ascii_alphanumeric())) +} + +fn reference(id: &str, root: Option<&str>, routable: bool) -> Value { + let mut value = json!({"id": id}); + if let Some(root) = root.filter(|_| routable) { + value["entry"] = Value::String(format!("{root}/{id}")); + } + value +} + +fn snapshot_ids(event: &Value, ids: &mut BTreeSet) { + if let Some(subjects) = event + .get("ticket_merge_request_subjects") + .and_then(Value::as_array) + { + for subject in subjects { + if let Some(id) = subject.get("merge_request_id").and_then(Value::as_str) { + ids.insert(id.into()); + } + } + } +} + +pub(super) fn value( + kind: Kind, + data: Value, + subject: &str, + collection: &str, + ticket_root: Option<&str>, +) -> Result { + let mut tickets = BTreeSet::new(); + let mut merge_requests = BTreeSet::new(); + match kind { + Kind::Open | Kind::Show => { + if let Some(ids) = data.get("ticket_ids").and_then(Value::as_array) { + tickets.extend(ids.iter().filter_map(Value::as_str).map(str::to_string)); + } + if kind == Kind::Show { + if let Some(linked) = data.get("linked_tickets").and_then(Value::as_array) { + tickets.extend( + linked + .iter() + .filter_map(|ticket| ticket.get("ticket_id").and_then(Value::as_str)) + .map(str::to_string), + ); + } + } + if let Some(thread) = data.get("thread").and_then(Value::as_array) { + for event in thread { + if matches!( + event.get("kind").and_then(Value::as_str), + Some("review_requested" | "review") + ) { + snapshot_ids(event, &mut merge_requests); + } + } + } + } + Kind::Readiness => { + if let Some(review) = data.get("review").filter(|review| !review.is_null()) { + snapshot_ids(review, &mut merge_requests); + } + } + Kind::Review => snapshot_ids(&data, &mut merge_requests), + Kind::Complete => {} + } + json_to_wip(&json!({ + "entry": format!("{collection}/{subject}"), + "data": data, + "links": { + "tickets": tickets.iter().map(|id| reference(id, ticket_root, ticket_reference(id))).collect::>(), + "merge_requests": merge_requests.iter().map(|id| reference(id, Some(collection), is_merge_request_route_reference(id))).collect::>(), + } + })).map_err(WipOperationError::OutcomeUnknown) +} + +#[cfg(test)] +mod tests; diff --git a/crates/worker/src/feature/builtin/merge_request/result/tests.rs b/crates/worker/src/feature/builtin/merge_request/result/tests.rs new file mode 100644 index 000000000..d9c8f44e5 --- /dev/null +++ b/crates/worker/src/feature/builtin/merge_request/result/tests.rs @@ -0,0 +1,438 @@ +//! Connection tests run the production model-facing Inspect/Invoke, Host, Client, native result +//! validation and domain dispatch; only Workspace HTTP is scripted. They do not assert Backend +//! transactions or persistence. Unit cases pin the explicit response-field selection boundary. + +use super::super::tests::{RecordingWorkspaceClient, response}; +use super::*; +use crate::wip::{WipRuntime, install_wip_mode_with_mounts}; +use agen::Engine; +use agen::llm_client::client::LlmClient; +use agen::llm_client::{ClientError, Request as LlmRequest, ResponseStream}; +use agen::state::Mutable; + +#[derive(Clone)] +struct NoLlm; +#[async_trait] +impl LlmClient for NoLlm { + async fn stream(&self, _: LlmRequest) -> Result { + panic!("the model-facing tools are driven directly") + } + fn clone_boxed(&self) -> Box { + Box::new(self.clone()) + } +} + +struct Model { + engine: Engine, + runtime: Arc, +} +impl Model { + fn new(registry: WipMountRegistry) -> Self { + let mut engine = Engine::new_annotated(NoLlm); + let runtime = + install_wip_mode_with_mounts(&mut engine, None, "mr-model-tools".into(), registry) + .unwrap(); + engine.tool_server_handle().flush_pending(); + Self { engine, runtime } + } + async fn execute(&self, name: &str, input: Value) -> Result { + let output = self + .engine + .tool_server_handle() + .call_tool(name, &input.to_string(), ToolExecutionContext::direct()) + .await + .map_err(|error| ToolError::ExecutionFailed(error.to_string()))?; + Ok(serde_json::from_str(&output.content.unwrap()).unwrap()) + } + async fn inspect(&self, path: &Value) -> Value { + self.execute("Inspect", json!({"path":path})).await.unwrap() + } + async fn invoke( + &self, + object: &Value, + operation: &str, + arguments: Value, + ) -> Result { + self.execute("Invoke", json!({"path":object["path"], "interface":object["interfaces"][0]["reference"], "operation":operation, "arguments":arguments})).await + } +} + +fn projected(kind: Kind, data: Value, published: bool) -> Value { + crate::wip::wip_to_json( + &value( + kind, + data, + "MR-1", + "/changes", + published.then_some("/cases"), + ) + .unwrap_or_else(|_| panic!("result mapping")), + ) + .unwrap() +} + +#[test] +fn explicit_domain_reference_fields_preserve_full_snapshot_evidence_without_recursive_inference() { + let snapshot = json!([ + {"merge_request_id":"MR-1", "subject_ref":"source-1"}, + {"merge_request_id":"MR-2", "subject_ref":"source-2"}, + {"merge_request_id":"MR-2", "subject_ref":"older-source-2"}, + {"merge_request_id":"unroutable/id", "subject_ref":"source-3"} + ]); + let event = json!({"kind":"review", "event_id":"event-1", "subject_ref":"source-1", + "ticket_content_digest":"digest", "ticket_merge_request_subjects":snapshot, + "body":"/cases/T-FAKE", "findings":[{"path":"/changes/MR-FAKE"}]}); + let data = json!({"merge_request_id":"MR-1", "ticket_ids":["ticket-internal", "T-2"], + "linked_tickets":[{"ticket_id":"ticket-internal", "key":"T-1"}, {"ticket_id":"T-3","key":null}], + "thread":[event, {"kind":"comment", "ticket_merge_request_subjects":[{"merge_request_id":"MR-FAKE"}]}], + "state":"open", "page":{"next_cursor":"cursor-MR-FAKE"}, + "body":{"merge_request_id":"MR-FAKE","ticket_id":"T-FAKE"}}); + let result = projected(Kind::Show, data.clone(), true); + assert_eq!(result["data"], data); + assert_eq!(result["entry"], "/changes/MR-1"); + assert_eq!( + result["links"]["tickets"], + json!([ + {"id":"T-2","entry":"/cases/T-2"}, + {"id":"T-3","entry":"/cases/T-3"}, + {"id":"ticket-internal"} + ]) + ); + assert_eq!( + result["links"]["merge_requests"], + json!([ + {"id":"MR-1","entry":"/changes/MR-1"}, + {"id":"MR-2","entry":"/changes/MR-2"}, + {"id":"unroutable/id"} + ]) + ); + for (kind, response) in [ + (Kind::Readiness, json!({"ready":false, "review":event})), + (Kind::Review, event), + ] { + let result = projected(kind, response.clone(), false); + assert_eq!(result["data"], response); + assert_eq!( + result["links"]["merge_requests"].as_array().unwrap().len(), + 3 + ); + } +} + +#[test] +fn absent_null_and_empty_references_have_empty_lists_and_unpublished_entries_are_omitted() { + for kind in ALL_KINDS { + for data in [ + json!({}), + json!({"review":null, "thread":[], "ticket_ids":[], "linked_tickets":[]}), + ] { + let result = projected(kind, data.clone(), false); + assert_eq!(result["data"], data); + assert_eq!(result["links"], json!({"tickets":[],"merge_requests":[]})); + } + } + let result = projected(Kind::Open, json!({"ticket_ids":["T-1"]}), false); + assert_eq!(result["links"]["tickets"], json!([{"id":"T-1"}])); +} + +fn registry(client: Arc, publish_ticket: bool) -> WipMountRegistry { + registry_with_order(client, publish_ticket, false) +} + +fn registry_with_order( + client: Arc, + publish_ticket: bool, + ticket_first: bool, +) -> WipMountRegistry { + let mut registry = WipMountRegistry::new(); + let ticket = registry.allocate_namespace("ticket", "cases").unwrap(); + let mr = registry + .allocate_namespace("merge-request", "changes") + .unwrap(); + let mount_ticket = |registry: &mut WipMountRegistry| { + crate::feature::builtin::ticket::mount_workspace_http_ticket_wip( + registry, + client.clone(), + crate::feature::builtin::ticket::TicketFeatureAccess::read_only(), + None, + &ticket, + ) + .unwrap(); + }; + if publish_ticket && ticket_first { + mount_ticket(&mut registry); + } + mount_workspace_http_merge_request_wip( + &mut registry, + client.clone(), + MergeRequestFeatureConfig { + show: true, + open: true, + readiness_check: true, + ..Default::default() + }, + None, + &mr, + ) + .unwrap(); + if publish_ticket && !ticket_first { + mount_ticket(&mut registry); + } + registry +} + +async fn inspect(runtime: &WipRuntime, path: &str) -> Value { + serde_json::from_str( + &runtime + .inspect(path.into(), false) + .await + .unwrap() + .content + .unwrap(), + ) + .unwrap() +} + +async fn invoke( + runtime: &WipRuntime, + object: &Value, + operation: &str, + arguments: Value, +) -> Result { + let interface = &object["interfaces"][0]["reference"]; + runtime + .invoke( + object["path"].as_str().unwrap().into(), + wip_protocol::InterfaceReference { + scope: interface["scope"].as_str().unwrap().into(), + name: interface["name"].as_str().unwrap().into(), + }, + operation.into(), + arguments, + ToolExecutionContext::direct(), + ) + .await + .map(|output| serde_json::from_str(&output.content.unwrap()).unwrap()) +} + +#[tokio::test] +async fn inspect_open_entry_read_and_linked_ticket_use_published_contract_without_prefetch_or_indexing() + { + for ticket_first in [true, false] { + let detail = json!({"resource_key":"T-1", "title":"Intent", "body":"Requirements", "body_truncated":false, + "content_digest":"observed", "state":"open", "targets":[], "events":[], "linked_objectives":[], + "initial_post":{"sequence":0,"event_ref":"initial","is_initial":true,"kind":"comment","body":"Requirements","body_truncated":false,"body_digest":"a".repeat(64),"editable":true,"author":null,"at":null,"edited_at":null,"edited_by":null}, + "closure_operation_key":null,"current_closure":null,"legacy_handoff":null,"closure_history":[], + "assignment_tracking":[],"tags":[],"assignments":[],"implementation_reports":[],"merge_requests":[]}); + let page = json!({"merge_request_id":"MR-1", "ticket_ids":["T-1"], "linked_tickets":[{"ticket_id":"T-1","key":"T-1"}], + "thread":[{"kind":"review_requested", "subject_ref":"sha-1", "ticket_merge_request_subjects":[ + {"merge_request_id":"MR-1","subject_ref":"sha-1"}, {"merge_request_id":"MR-2","subject_ref":"sha-2"}]}]}); + let client = Arc::new(RecordingWorkspaceClient::new(vec![ + response(json!({"merge_request_id":"MR-1", "ticket_ids":["T-1"]})), + response(page.clone()), + response(detail), + response(json!({"ready":false,"review":null})), + ])); + let model = Model::new(registry_with_order(client.clone(), true, ticket_first)); + let runtime = &model.runtime; + let collection = model.inspect(&json!("/changes")).await; + let definition = model.inspect(&collection["interfaces"][0]["path"]).await; + let signature = definition["interface_signature"].as_str().unwrap(); + assert!(signature.contains("`entry`: entry"), "{signature}"); + assert!(signature.contains("data: json"), "{signature}"); + assert!(signature.contains("`entry`?: entry"), "{signature}"); + assert!(client.requests.lock().unwrap().is_empty()); + let before = runtime.metrics(); + let opened = model.invoke(&collection, "open", json!({"ticket":"/cases/T-1", "repository":{"key":"main","source":"work","target":"develop"}})).await.unwrap(); + assert_eq!(opened["entry"], "/changes/MR-1"); + assert_eq!(client.requests.lock().unwrap().len(), 1); + assert_eq!( + runtime.metrics().inspect_round_trips, + before.inspect_round_trips, + "Entry values must not be inspected eagerly" + ); + let tree = runtime + .tree("/changes".into(), 3, false) + .await + .unwrap() + .content + .unwrap(); + assert!(!tree.contains("MR-1"), "dynamic items remain nonindexable"); + let item = model.inspect(&opened["entry"]).await; + let item_definition = model.inspect(&item["interfaces"][0]["path"]).await; + assert!( + item_definition["interface_signature"] + .as_str() + .unwrap() + .contains("`entry`: entry") + ); + assert_eq!( + client.requests.lock().unwrap().len(), + 1, + "Inspect must not read domain data" + ); + let read = model.invoke(&item, + "read", + json!({"page":{"after":{"$case":"value","value":7},"limit":{"$case":"value","value":25}}}), + ) + .await + .unwrap(); + assert_eq!(read["data"], page); + assert_eq!(read["links"]["merge_requests"][1]["entry"], "/changes/MR-2"); + assert_eq!( + client.requests.lock().unwrap().len(), + 2, + "linked snapshot must not be prefetched" + ); + let ticket = model.inspect(&read["links"]["tickets"][0]["entry"]).await; + let ticket_definition = model.inspect(&ticket["interfaces"][0]["path"]).await; + assert!( + ticket_definition["interface_signature"] + .as_str() + .unwrap() + .contains("operation read(") + ); + let ticket_read = model.invoke(&ticket, "read", json!({})).await.unwrap(); + assert!(ticket_read.to_string().contains("Intent")); + // Reading updates the MR validator: refresh the observation for this separate operation. + runtime + .inspect(read["entry"].as_str().unwrap().into(), true) + .await + .unwrap(); + let readiness = model.invoke(&item, "readiness", json!({})).await.unwrap(); + assert_eq!(readiness["entry"], read["entry"]); + assert_eq!( + readiness["links"], + json!({"tickets":[],"merge_requests":[]}) + ); + let requests = client.requests.lock().unwrap(); + assert_eq!(requests.len(), 4); + assert_eq!( + requests[1].path, + "/api/w/ws/merge-requests/MR-1?after=7&limit=25" + ); + assert!(requests[2].path.contains("/tickets/T-1")); + } +} + +#[tokio::test] +async fn selected_denied_or_deleted_entry_never_retries_successful_open() { + for status in [403, 404] { + let client = Arc::new(RecordingWorkspaceClient::new(vec![ + response(json!({"merge_request_id":"MR-1","ticket_ids":["T-1"]})), + crate::worker::WorkspaceResponse { + status, + body: "{}".into(), + }, + ])); + let runtime = + WipRuntime::from_mounts(registry(client.clone(), true), format!("negative-{status}")) + .unwrap(); + let collection = inspect(&runtime, "/changes").await; + let opened = invoke(&runtime, &collection, "open", json!({"ticket":"/cases/T-1","repository":{"key":"main","source":"work","target":"develop"}})).await.unwrap(); + let ticket = inspect( + &runtime, + opened["links"]["tickets"][0]["entry"].as_str().unwrap(), + ) + .await; + let failure = invoke(&runtime, &ticket, "read", json!({})) + .await + .unwrap_err(); + assert!( + failure.to_string().contains(if status == 403 { + "PermissionDenied" + } else { + "NotFound" + }), + "{failure}" + ); + assert_eq!(opened["data"]["merge_request_id"], "MR-1"); + let requests = client.requests.lock().unwrap(); + assert_eq!(requests.len(), 2); + assert_eq!( + requests + .iter() + .filter(|request| request.path.ends_with("/merge-request")) + .count(), + 1 + ); + } +} + +#[tokio::test] +async fn unpublished_ticket_reference_omits_entry_but_open_remains_successful() { + let client = Arc::new(RecordingWorkspaceClient::new(vec![response( + json!({"merge_request_id":"MR-1","ticket_ids":["T-1"]}), + )])); + let runtime = + WipRuntime::from_mounts(registry(client.clone(), false), "unpublished-result".into()) + .unwrap(); + let collection = inspect(&runtime, "/changes").await; + let opened = invoke(&runtime, &collection, "open", json!({"ticket":"/cases/T-1","repository":{"key":"main","source":"work","target":"develop"}})).await.unwrap(); + assert_eq!(opened["links"]["tickets"], json!([{"id":"T-1"}])); + assert!(runtime.inspect("/cases/T-1".into(), false).await.is_err()); + assert_eq!(client.requests.lock().unwrap().len(), 1); +} + +#[tokio::test] +async fn successful_open_with_unroutable_response_identity_is_unknown_without_retry() { + let client = Arc::new(RecordingWorkspaceClient::new(vec![response(json!({ + "merge_request_id":"not/a/route", "ticket_ids":["T-1"] + }))])); + let model = Model::new(registry(client.clone(), true)); + let collection = model.inspect(&json!("/changes")).await; + let failure = model.invoke(&collection, "open", json!({"ticket":"/cases/T-1", "repository":{"key":"main","source":"work","target":"develop"}})).await.unwrap_err(); + assert!( + failure + .to_string() + .contains("outcome unknown; do not retry automatically"), + "{failure}" + ); + assert_eq!(client.requests.lock().unwrap().len(), 1); +} + +#[tokio::test] +async fn malformed_domain_response_after_open_is_unknown_without_replaying_mutation() { + let client = Arc::new(RecordingWorkspaceClient::new(vec![ + crate::worker::WorkspaceResponse { + status: 200, + body: "not-json".into(), + }, + ])); + let model = Model::new(registry(client.clone(), true)); + let collection = model.inspect(&json!("/changes")).await; + let failure = model.invoke(&collection, "open", json!({"ticket":"/cases/T-1", "repository":{"key":"main","source":"work","target":"develop"}})).await.unwrap_err(); + assert!( + failure + .to_string() + .contains("outcome unknown; do not retry automatically"), + "{failure}" + ); + assert_eq!(client.requests.lock().unwrap().len(), 1); +} + +#[test] +fn shared_domain_value_and_ordinary_tool_boundary_preserve_the_same_response_data() { + let data = json!({"merge_request_id":"MR-1","ticket_ids":["T-1"],"body":"Do not infer /cases/T-9", "thread":[{"kind":"review", "ticket_merge_request_subjects":[{"merge_request_id":"MR-2","subject_ref":"source-id"}]}]}); + let client = Arc::new(RecordingWorkspaceClient::new(vec![ + response(data.clone()), + response(data.clone()), + ])); + let read = || command::Command::Read { + merge_request_id: "MR-1".into(), + after: None, + limit: Some(25), + }; + let domain = command::execute_value(client.clone(), read()).unwrap(); + assert_eq!(domain, data); + let tool = command::execute(client.clone(), read()).unwrap(); + assert_eq!(tool.summary, "ShowMergeRequest"); + assert_eq!( + serde_json::from_str::(&tool.content.unwrap()).unwrap(), + domain + ); + let requests = client.requests.lock().unwrap(); + assert_eq!(requests.len(), 2); + assert_eq!(requests[0].path, requests[1].path); + assert_eq!(client.authorizations.lock().unwrap().len(), 2); +} diff --git a/crates/worker/src/feature/builtin/objective.rs b/crates/worker/src/feature/builtin/objective.rs index 321dcfcc6..3730a4053 100644 --- a/crates/worker/src/feature/builtin/objective.rs +++ b/crates/worker/src/feature/builtin/objective.rs @@ -17,6 +17,10 @@ //! logic. Query/detail uses canonical references and bounded bodies/events, omitting internal //! storage IDs from output. +#[cfg(test)] +mod result_tests; +mod results; + use std::collections::BTreeMap; use std::sync::Arc; #[cfg(test)] @@ -633,13 +637,16 @@ pub fn mount_workspace_http_objective_wip( ) -> Result<(), WipMountError> { let collection_route = namespace_route.root().to_string(); let backend = WorkspaceHttpObjectiveBackend::new(client); + let result_routes = results::Routes { + objective: collection_route.clone(), + }; let collection_descriptor = objective_collection_descriptor(); let collection_validator = descriptor_validator(&collection_descriptor); let collection_handler = Arc::new(ObjectiveCollectionWipHandler { backend: backend.clone(), permissions: permissions.clone(), - collection_route: collection_route.clone(), + result_routes: result_routes.clone(), }); registry.mount(WipProjection { route: collection_route.clone(), @@ -677,16 +684,18 @@ pub fn mount_workspace_http_objective_wip( backend, permissions, collection_route: collection_route.clone(), + result_routes, }), })?; registry.replace_compatibility_tools(&collection_route, OBJECTIVE_TOOL_NAMES)?; + registry.publish_result_namespace(namespace_route)?; Ok(()) } struct ObjectiveCollectionWipHandler { backend: WorkspaceHttpObjectiveBackend, permissions: Option, - collection_route: String, + result_routes: results::Routes, } #[async_trait] @@ -695,7 +704,7 @@ impl WipOperationHandler for ObjectiveCollectionWipHandler { &self, operation: &str, arguments: &BTreeMap, - _context: WipCallContext, + context: WipCallContext, ) -> Result { let value = match operation { "query" => { @@ -725,34 +734,13 @@ impl WipOperationHandler for ObjectiveCollectionWipHandler { }; let permission_input = native_permission_input(arguments, None)?; authorize_native(&self.permissions, "QueryObjective", &permission_input)?; - let mut response = self + let response = self .backend .query_value(&input) .await .map_err(map_backend_error)?; - if let Some(objectives) = - response.get_mut("objectives").and_then(Json::as_array_mut) - { - for objective in objectives { - if let Some(reference) = objective - .get("objective") - .and_then(Json::as_str) - .map(ToOwned::to_owned) - { - objective - .as_object_mut() - .expect("projected Objective is an object") - .insert( - "path".into(), - Json::String(format!( - "{}/{}", - self.collection_route, reference - )), - ); - } - } - } - response + self.result_routes + .query(&response, context.result_namespaces.root("ticket"))? } "create" => { let input = ObjectiveCreateInput { @@ -765,32 +753,17 @@ impl WipOperationHandler for ObjectiveCollectionWipHandler { }; let permission_input = native_permission_input(arguments, None)?; authorize_native(&self.permissions, "ObjectiveCreate", &permission_input)?; - let mut response = self + let response = self .backend .create_value(&input) .await .map_err(map_backend_error)?; - - if let Some(reference) = response - .get("objective") - .and_then(Json::as_str) - .map(ToOwned::to_owned) - { - response - .as_object_mut() - .expect("projected Objective is an object") - .insert( - "path".into(), - Json::String(format!("{}/{}", self.collection_route, reference)), - ); - } - response + self.result_routes + .detail(&response, context.result_namespaces.root("ticket"))? } _ => return Err(operation_not_found()), }; - Ok(WipOperationOutput::native( - json_to_wip(&value).map_err(WipOperationError::OutcomeUnknown)?, - )) + Ok(WipOperationOutput::native(value)) } } @@ -798,6 +771,7 @@ struct ObjectiveItemResolver { backend: WorkspaceHttpObjectiveBackend, permissions: Option, collection_route: String, + result_routes: results::Routes, } impl WipDynamicItemResolver for ObjectiveItemResolver { @@ -820,6 +794,7 @@ impl WipDynamicItemResolver for ObjectiveItemResolver { backend: self.backend.clone(), permissions: self.permissions.clone(), objective_reference: item_reference.to_string(), + result_routes: self.result_routes.clone(), }), }) } @@ -829,6 +804,7 @@ struct ObjectiveItemWipHandler { backend: WorkspaceHttpObjectiveBackend, permissions: Option, objective_reference: String, + result_routes: results::Routes, } #[async_trait] @@ -837,7 +813,7 @@ impl WipOperationHandler for ObjectiveItemWipHandler { &self, operation: &str, arguments: &BTreeMap, - _context: WipCallContext, + context: WipCallContext, ) -> Result { let permission_input = native_permission_input(arguments, Some(&self.objective_reference))?; let response = match operation { @@ -899,9 +875,10 @@ impl WipOperationHandler for ObjectiveItemWipHandler { _ => return Err(operation_not_found()), }; - Ok(WipOperationOutput::native( - json_to_wip(&response).map_err(WipOperationError::OutcomeUnknown)?, - )) + Ok(WipOperationOutput::native(self.result_routes.detail( + &response, + context.result_namespaces.root("ticket"), + )?)) } } @@ -1129,7 +1106,7 @@ fn objective_collection_descriptor() -> InterfaceDescriptor { documentation: Some(Documentation { summary: "Search and create authoritative Workspace Objectives".into(), details: Some( - "Query results are bounded and include canonical item paths. Ticket references are data only and confer no Ticket authority." + "Query returns bounded items with data: Json, entry: Entry, links.tickets, and separate next_cursor/has_more. Inspect a selected entry and its Interface before read; create returns the same item envelope. Linked Ticket Entries are emitted only for committed published result namespaces; allocation alone omits the optional Entry. Publication does not attest existence or authority: selected reads may be missing or denied. No related resources are fetched eagerly." .into(), ), }), @@ -1151,7 +1128,7 @@ fn objective_collection_descriptor() -> InterfaceDescriptor { parameter("limit", false, TypeExpr::Integer), parameter("cursor", false, TypeExpr::String), ], - returns: json_return("Bounded Objective query with cursor metadata and item paths"), + returns: results::declaration(true), }, OperationDeclaration { name: "create".into(), @@ -1165,7 +1142,7 @@ fn objective_collection_descriptor() -> InterfaceDescriptor { parameter("state", false, TypeExpr::String), parameter("linked_tickets", false, list_of(TypeExpr::String)), ], - returns: json_return("Created Objective with bounded context"), + returns: results::declaration(false), }, ], } @@ -1177,7 +1154,7 @@ fn objective_item_descriptor() -> InterfaceDescriptor { documentation: Some(Documentation { summary: "Read and mutate the Objective bound to this object route".into(), details: Some( - "The Objective identity is taken exclusively from the target path; operations do not accept a second Objective id." + "The Objective identity is taken exclusively from the target path; operations do not accept a second Objective id. Results separate bounded data from the subject Entry and linked Ticket coordinates. Inspect the chosen Entry and its Interface before read; unavailable or denied references do not invalidate a successful mutation and must not cause replay." .into(), ), }), @@ -1232,7 +1209,7 @@ fn operation( details: None, }), parameters, - returns: json_return("Authoritative Objective detail with bounded context"), + returns: results::declaration(false), } } @@ -1251,16 +1228,6 @@ fn list_of(items: TypeExpr) -> TypeExpr { } } -fn json_return(summary: &str) -> ReturnDeclaration { - ReturnDeclaration { - documentation: Some(Documentation { - summary: summary.into(), - details: None, - }), - r#type: TypeExpr::Json, - } -} - #[derive(Clone, Copy)] enum ObjectiveOperation { List, @@ -1584,15 +1551,18 @@ mod tests { } #[derive(Debug)] - struct MockWorkspaceClient { - auth_denied: std::sync::atomic::AtomicBool, - authorizations: Mutex>, + pub(super) struct MockWorkspaceClient { + pub(super) auth_denied: std::sync::atomic::AtomicBool, + pub(super) authorizations: + Mutex>, responses: Mutex>, requests: Mutex>, } impl MockWorkspaceClient { - fn new(responses: impl IntoIterator) -> Self { + pub(super) fn new( + responses: impl IntoIterator, + ) -> Self { Self { auth_denied: std::sync::atomic::AtomicBool::new(false), authorizations: Mutex::new(Vec::new()), @@ -1601,7 +1571,7 @@ mod tests { } } - fn requests(&self) -> Vec { + pub(super) fn requests(&self) -> Vec { self.requests .lock() .unwrap_or_else(|error| error.into_inner()) @@ -1657,7 +1627,7 @@ mod tests { } } - fn objective_response(title: &str) -> crate::worker::WorkspaceResponse { + pub(super) fn objective_response(title: &str) -> crate::worker::WorkspaceResponse { crate::worker::WorkspaceResponse { status: 200, body: json!({ @@ -1902,6 +1872,9 @@ mod tests { )), permissions: None, collection_route: "/objectives".into(), + result_routes: results::Routes { + objective: "/objectives".into(), + }, }; let initial = resolver.resolve("O-3").unwrap(); assert_eq!(initial.object.name, "O-3"); @@ -2031,6 +2004,9 @@ mod tests { rules: Vec::new(), }), objective_reference: "O-3".into(), + result_routes: results::Routes { + objective: "/objectives".into(), + }, }; let result = denied .call( @@ -2039,6 +2015,7 @@ mod tests { WipCallContext { execution: ToolExecutionContext::direct(), security_context: "test".into(), + result_namespaces: Default::default(), }, ) .await; @@ -2053,7 +2030,9 @@ mod tests { let collection = ObjectiveCollectionWipHandler { backend, permissions: None, - collection_route: "/objectives".into(), + result_routes: results::Routes { + objective: "/objectives".into(), + }, }; let result = collection .call( @@ -2062,6 +2041,7 @@ mod tests { WipCallContext { execution: ToolExecutionContext::direct(), security_context: "test".into(), + result_namespaces: Default::default(), }, ) .await; @@ -2094,11 +2074,14 @@ mod tests { let collection = ObjectiveCollectionWipHandler { backend: backend.clone(), permissions: None, - collection_route: "/objectives".into(), + result_routes: results::Routes { + objective: "/objectives".into(), + }, }; let context = || WipCallContext { execution: ToolExecutionContext::direct(), security_context: "test".into(), + result_namespaces: Default::default(), }; assert!( collection @@ -2115,6 +2098,9 @@ mod tests { backend, permissions: None, objective_reference: "O-3".into(), + result_routes: results::Routes { + objective: "/objectives".into(), + }, }; assert!(item.call("read", &BTreeMap::new(), context()).await.is_ok()); assert!( @@ -2179,6 +2165,9 @@ mod tests { backend: WorkspaceHttpObjectiveBackend::new(client), permissions: None, objective_reference: "O-404".into(), + result_routes: results::Routes { + objective: "/objectives".into(), + }, }; for (operation, arguments) in [ ("read", BTreeMap::new()), @@ -2194,6 +2183,7 @@ mod tests { WipCallContext { execution: ToolExecutionContext::direct(), security_context: "test".into(), + result_namespaces: Default::default(), }, ) .await; @@ -2245,7 +2235,9 @@ mod tests { crate::worker::TestWorkspaceHttpClient::new("workspace", base_url), )), permissions: None, - collection_route: "/objectives".into(), + result_routes: results::Routes { + objective: "/objectives".into(), + }, }; let output = match handler .call( @@ -2254,6 +2246,7 @@ mod tests { WipCallContext { execution: ToolExecutionContext::direct(), security_context: "test".into(), + result_namespaces: Default::default(), }, ) .await @@ -2265,8 +2258,12 @@ mod tests { let output = crate::wip::wip_to_json(&output.value).unwrap(); assert_eq!(output["next_cursor"], "next-page"); assert_eq!(output["has_more"], true); - assert_eq!(output["objectives"][0]["path"], "/objectives/O-3"); - assert_eq!(output["objectives"][0]["linked_tickets"][0], "T-7"); + assert_eq!(output["items"][0]["entry"], "/objectives/O-3"); + assert_eq!(output["items"][0]["data"]["linked_tickets"][0], "T-7"); + assert_eq!( + output["items"][0]["links"]["tickets"], + json!([{"id":"T-7"}]) + ); assert!(!output.to_string().contains("00001OBJECTIVE")); assert!(!output.to_string().contains("00001TICKET")); } @@ -2312,12 +2309,18 @@ mod tests { backend: backend.clone(), permissions: None, collection_route: "/objectives".into(), + result_routes: results::Routes { + objective: "/objectives".into(), + }, }; let before = resolver.resolve("O-3").unwrap().object.validator; let handler = ObjectiveItemWipHandler { backend, permissions: None, objective_reference: "O-3".into(), + result_routes: results::Routes { + objective: "/objectives".into(), + }, }; let output = match handler .call( @@ -2326,6 +2329,7 @@ mod tests { WipCallContext { execution: ToolExecutionContext::direct(), security_context: "test".into(), + result_namespaces: Default::default(), }, ) .await @@ -2335,8 +2339,8 @@ mod tests { }; server.join().unwrap(); let output = crate::wip::wip_to_json(&output.value).unwrap(); - assert_eq!(output["objective"], "O-3"); - assert_eq!(output["title"], "Changed"); + assert_eq!(output["data"]["objective"], "O-3"); + assert_eq!(output["data"]["title"], "Changed"); let after = resolver.resolve("O-3").unwrap().object.validator; assert_eq!(before, after); } @@ -2400,6 +2404,9 @@ mod tests { backend, permissions: None, objective_reference: "O-3".into(), + result_routes: results::Routes { + objective: "/objectives".into(), + }, }; let result = item .call( @@ -2408,6 +2415,7 @@ mod tests { WipCallContext { execution: ToolExecutionContext::direct(), security_context: "test".into(), + result_namespaces: Default::default(), }, ) .await; diff --git a/crates/worker/src/feature/builtin/objective/result_tests.rs b/crates/worker/src/feature/builtin/objective/result_tests.rs new file mode 100644 index 000000000..e6842a528 --- /dev/null +++ b/crates/worker/src/feature/builtin/objective/result_tests.rs @@ -0,0 +1,567 @@ +//! Model-facing integration of production Inspect/Text View, Invoke, native Objective projection, +//! Client result validation and selected Ticket navigation. Only Workspace HTTP/authorization replies +//! are scripted (no server policy or persistence claim). Normal crate tests include this suite. + +use super::tests::{MockWorkspaceClient, objective_response}; +use super::*; +use crate::wip::{WipRuntime, install_wip_mode_with_mounts}; +use agen::Engine; +use agen::llm_client::client::LlmClient; +use agen::llm_client::{ClientError, Request as LlmRequest, ResponseStream}; +use agen::state::Mutable; +use std::sync::atomic::Ordering; + +#[derive(Clone)] +struct NoLlm; +#[async_trait] +impl LlmClient for NoLlm { + async fn stream(&self, _: LlmRequest) -> Result { + panic!("model-facing tools are driven explicitly, not by an LLM fixture") + } + fn clone_boxed(&self) -> Box { + Box::new(self.clone()) + } +} + +struct Model { + engine: Engine, + runtime: Arc, +} + +impl Model { + // allocated and mounted are deliberately independent; Objective never enables Ticket itself. + fn new( + client: Arc, + allocated: bool, + mounted: bool, + objective_permissions: Option, + ) -> Self { + let mut registry = WipMountRegistry::new(); + let ticket = allocated.then(|| registry.allocate_namespace("ticket", "cases").unwrap()); + let objective = registry.allocate_namespace("objective", "goals").unwrap(); + mount_workspace_http_objective_wip( + &mut registry, + client.clone(), + objective_permissions, + &objective, + ) + .unwrap(); + // Mount after Objective: final Host publication, not mount-time allocation, supplies links. + if mounted { + super::super::ticket::mount_workspace_http_ticket_wip( + &mut registry, + client, + super::super::ticket::TicketFeatureAccess::workspace_authoring(), + None, + ticket.as_ref().unwrap(), + ) + .unwrap(); + } + let mut engine = Engine::new_annotated(NoLlm); + let runtime = + install_wip_mode_with_mounts(&mut engine, None, "objective-model".into(), registry) + .unwrap(); + engine.tool_server_handle().flush_pending(); + Self { engine, runtime } + } + + async fn execute(&self, name: &str, input: Json) -> Result { + let output = self + .engine + .tool_server_handle() + .call_tool(name, &input.to_string(), ToolExecutionContext::direct()) + .await + .map_err(|error| ToolError::ExecutionFailed(error.to_string()))?; + // Parse only the actual model ToolOutput envelope, never Text View or arbitrary domain JSON. + Ok(serde_json::from_str(output.content.as_deref().unwrap()).unwrap()) + } + + async fn inspect(&self, entry: &Json) -> Result { + self.execute("Inspect", json!({"path":entry})).await + } + + async fn contract(&self, object: &Json) -> Json { + self.inspect(&object["interfaces"][0]["path"]) + .await + .unwrap() + } + + async fn invoke( + &self, + object: &Json, + operation: &str, + arguments: Json, + ) -> Result { + self.execute( + "Invoke", + json!({ + "path":object["path"], "interface":object["interfaces"][0]["reference"], + "operation":operation, "arguments":arguments, + }), + ) + .await + } +} + +fn response(body: Json) -> crate::worker::WorkspaceResponse { + crate::worker::WorkspaceResponse { + status: 200, + body: body.to_string(), + } +} + +fn query_item(id: &str, tickets: Json) -> Json { + json!({"resource_key":id,"title":"Intent","snippet":"O-999 /cases/T-999 are text only", + "state":"active","linked_ticket_keys":tickets}) +} + +fn query(items: Json, cursor: Json, more: bool) -> crate::worker::WorkspaceResponse { + response(json!({"items":items,"page":{"next_cursor":cursor,"has_more":more}})) +} + +fn detail(tickets: Json) -> crate::worker::WorkspaceResponse { + let mut body: Json = serde_json::from_str(&objective_response("Intent").body).unwrap(); + body["linked_ticket_summaries"] = tickets; + body["body"] = json!("T-999 /cases/T-999 {\"objective\":\"O-999\"} remain content"); + body["events"] = json!([{"kind":"edited","created_at":"now","body":"T-888"}]); + body["event_page"] = json!({"next_cursor":"events-next","has_more":true}); + response(body) +} + +fn linked_ticket() -> Json { + json!([{"resource_key":"T-7","title":"Linked","state":"open"}]) +} + +fn ticket_detail() -> crate::worker::WorkspaceResponse { + response(json!({ + "resource_key":"T-7","title":"Linked","body":"Requirements","body_truncated":false, + "content_digest":"observed","state":"open","targets":[],"events":[],"linked_objectives":[], + "initial_post":{"sequence":0,"event_ref":"initial","is_initial":true,"kind":"comment", + "body":"Requirements","body_truncated":false,"body_digest":"a".repeat(64), + "editable":true,"author":null,"at":null,"edited_at":null,"edited_by":null}, + "closure_operation_key":null,"current_closure":null,"legacy_handoff":null,"closure_history":[], + "assignment_tracking":[],"tags":[],"assignments":[],"implementation_reports":[],"merge_requests":[] + })) +} + +fn error_response(status: u16) -> crate::worker::WorkspaceResponse { + crate::worker::WorkspaceResponse { + status, + body: "unavailable".into(), + } +} + +fn validate(descriptor: &InterfaceDescriptor, operation: &str, output: &Json) { + descriptor + .validate_result(operation, &json_to_wip(output).unwrap()) + .unwrap(); +} + +#[tokio::test] +async fn model_query_selected_entry_read_paging_and_empty_are_lazy_nonindexable() { + let client = Arc::new(MockWorkspaceClient::new([ + query( + json!([ + query_item("O-3", json!(["T-7"])), + query_item("O-4", json!([])) + ]), + json!("next-page"), + true, + ), + detail(linked_ticket()), + query(json!([]), Json::Null, false), + ])); + let model = Model::new(client.clone(), true, true, None); + let collection = model.inspect(&json!("/goals")).await.unwrap(); + let contract = model.contract(&collection).await; + let signature = contract["interface_signature"].as_str().unwrap(); + assert!(signature.contains("data: json"), "{signature}"); + assert!(signature.contains("`entry`: entry"), "{signature}"); + assert!(signature.contains("`entry`?: entry"), "{signature}"); + assert!(signature.contains("next_cursor?: string"), "{signature}"); + let before = model.runtime.metrics(); + let found = model + .invoke(&collection, "query", json!({"limit":2})) + .await + .unwrap(); + validate(&objective_collection_descriptor(), "query", &found); + assert_eq!(found["next_cursor"], "next-page"); + assert_eq!(found["has_more"], true); + assert_eq!(found["items"].as_array().unwrap().len(), 2); + assert!( + found.get("data").is_none(), + "query items must not be duplicated" + ); + assert_eq!(found["items"][0]["data"]["linked_tickets"], json!(["T-7"])); + assert_eq!( + found["items"][0]["links"]["tickets"], + json!([{"id":"T-7","entry":"/cases/T-7"}]) + ); + assert_eq!(found["items"][1]["links"]["tickets"], json!([])); + assert_eq!(client.requests().len(), 1, "no eager item or Ticket reads"); + assert_eq!( + model.runtime.metrics().inspect_round_trips, + before.inspect_round_trips + ); + assert_eq!( + model.runtime.metrics().discover_round_trips, + before.discover_round_trips + ); + // Select the declared Entry field from the Invoke envelope, not a path inferred from data. + let selected = model.inspect(&found["items"][0]["entry"]).await.unwrap(); + let item_contract = model.contract(&selected).await; + assert!( + item_contract["interface_signature"] + .as_str() + .unwrap() + .contains("`entry`: entry") + ); + assert_eq!( + client.requests().len(), + 1, + "fixed signatures do not read content" + ); + let read = model + .invoke( + &selected, + "read", + json!({"event_limit":1,"event_cursor":"events-page"}), + ) + .await + .unwrap(); + validate(&objective_item_descriptor(), "read", &read); + assert_eq!(read["entry"], found["items"][0]["entry"]); + assert_eq!( + read["data"]["event_page"], + json!({"next_cursor":"events-next","has_more":true}) + ); + assert_eq!(read["data"]["events"][0]["body"], "T-888"); + assert_eq!( + read["links"]["tickets"].as_array().unwrap().len(), + 1, + "body/events/snippets cannot become links" + ); + let empty = model + .invoke( + &collection, + "query", + json!({"limit":2,"cursor":found["next_cursor"]}), + ) + .await + .unwrap(); + validate(&objective_collection_descriptor(), "query", &empty); + assert_eq!(empty, json!({"items":[],"has_more":false})); + let tree = model + .execute( + "Tree", + json!({"path":collection["path"],"depth":2,"refresh":true}), + ) + .await + .unwrap(); + assert_eq!( + tree["tree"]["children"], + json!([]), + "selected items never become indexable edges" + ); + let requests = client.requests(); + assert_eq!(requests.len(), 3); + assert!(requests[1].path.ends_with("/objectives/O-3/show")); + assert_eq!( + serde_json::from_str::(requests[1].body.as_deref().unwrap()).unwrap(), + json!({"event_limit":1,"event_cursor":"events-page"}) + ); + assert_eq!( + serde_json::from_str::(requests[2].body.as_deref().unwrap()).unwrap()["cursor"], + found["next_cursor"] + ); + assert!(!requests.iter().any(|r| r.path.contains("O-4"))); +} + +#[tokio::test] +async fn model_query_omits_ticket_entries_for_unallocated_or_unpublished_namespaces() { + for allocated in [false, true] { + let client = Arc::new(MockWorkspaceClient::new([query( + json!([query_item("O-3", json!(["T-7"]))]), + Json::Null, + false, + )])); + let model = Model::new(client.clone(), allocated, false, None); + let collection = model.inspect(&json!("/goals")).await.unwrap(); + model.contract(&collection).await; + let found = model.invoke(&collection, "query", json!({})).await.unwrap(); + validate(&objective_collection_descriptor(), "query", &found); + assert_eq!(found["items"][0]["links"]["tickets"], json!([{"id":"T-7"}])); + assert_eq!(found["items"][0]["data"]["linked_tickets"], json!(["T-7"])); + assert_eq!(found["items"][0]["entry"], "/goals/O-3"); + assert_eq!(client.requests().len(), 1); + } +} + +#[tokio::test] +async fn model_create_subject_and_related_ticket_entries_are_inspected_then_read() { + let client = Arc::new(MockWorkspaceClient::new([ + detail(linked_ticket()), + detail(linked_ticket()), + ticket_detail(), + ])); + let model = Model::new(client.clone(), true, true, None); + let collection = model.inspect(&json!("/goals")).await.unwrap(); + model.contract(&collection).await; + let created = model + .invoke( + &collection, + "create", + json!({"title":"Intent","linked_tickets":["T-7"]}), + ) + .await + .unwrap(); + validate(&objective_collection_descriptor(), "create", &created); + assert_eq!( + client.requests().len(), + 1, + "creating a result does not fetch any references" + ); + let selected = model.inspect(&created["entry"]).await.unwrap(); + model.contract(&selected).await; + let read = model.invoke(&selected, "read", json!({})).await.unwrap(); + assert_eq!(read["data"]["objective"], created["data"]["objective"]); + let ticket = model + .inspect(&created["links"]["tickets"][0]["entry"]) + .await + .unwrap(); + let ticket_contract = model.contract(&ticket).await; + assert!( + ticket_contract["interface_signature"] + .as_str() + .unwrap() + .contains("read(") + ); + let linked = model.invoke(&ticket, "read", json!({})).await.unwrap(); + assert_eq!(linked["data"]["ticket"], "T-7"); // Existing Ticket Model data, not an inferred coordinate. + let requests = client.requests(); + assert_eq!(requests.len(), 3); + assert_eq!( + requests + .iter() + .filter(|r| r.path == "/api/w/workspace/objectives") + .count(), + 1 + ); + assert!(requests[2].path.ends_with("/tickets/T-7/show")); +} + +#[tokio::test] +async fn model_allocation_is_not_publication_and_followup_failure_never_replays_create() { + let client = Arc::new(MockWorkspaceClient::new([ + detail(linked_ticket()), + error_response(404), + ])); + let model = Model::new(client.clone(), true, false, None); + let collection = model.inspect(&json!("/goals")).await.unwrap(); + model.contract(&collection).await; + let created = model + .invoke(&collection, "create", json!({"title":"Intent"})) + .await + .unwrap(); + assert_eq!(created["links"]["tickets"], json!([{"id":"T-7"}])); + assert!( + created["links"]["tickets"][0].get("entry").is_none(), + "allocated but unpublished spaces must not supply navigable entries" + ); + assert_eq!( + client.requests().len(), + 1, + "building references does not fetch Tickets" + ); + let selected = model.inspect(&created["entry"]).await.unwrap(); + model.contract(&selected).await; + let deleted = model + .invoke(&selected, "read", json!({})) + .await + .unwrap_err(); + assert!(deleted.to_string().contains("NotFound"), "{deleted}"); + assert_eq!(model.runtime.metrics().operation_successes, 1); + assert_eq!(client.requests().len(), 2); + assert_eq!( + client + .requests() + .iter() + .filter(|r| r.path == "/api/w/workspace/objectives") + .count(), + 1 + ); + validate(&objective_collection_descriptor(), "create", &created); +} + +#[tokio::test] +async fn model_unallocated_ticket_retains_id_and_backend_read_denial_is_not_empty_success() { + let client = Arc::new(MockWorkspaceClient::new([ + detail(linked_ticket()), + error_response(403), + ])); + let model = Model::new(client.clone(), false, false, None); + let collection = model.inspect(&json!("/goals")).await.unwrap(); + let created = model + .invoke(&collection, "create", json!({"title":"Intent"})) + .await + .unwrap(); + assert_eq!(created["links"]["tickets"], json!([{"id":"T-7"}])); + validate(&objective_collection_descriptor(), "create", &created); + let selected = model.inspect(&created["entry"]).await.unwrap(); + let denied = model + .invoke(&selected, "read", json!({})) + .await + .unwrap_err(); + assert!(denied.to_string().contains("PermissionDenied"), "{denied}"); + assert_eq!(client.requests().len(), 2); + assert_eq!(model.runtime.metrics().operation_successes, 1); +} + +#[tokio::test] +async fn model_selected_related_ticket_denied_or_deleted_does_not_repeat_objective_mutation() { + for status in [403, 404] { + let client = Arc::new(MockWorkspaceClient::new([ + detail(linked_ticket()), + error_response(status), + ])); + let model = Model::new(client.clone(), true, true, None); + let collection = model.inspect(&json!("/goals")).await.unwrap(); + let created = model + .invoke(&collection, "create", json!({"title":"Intent"})) + .await + .unwrap(); + let related = model + .inspect(&created["links"]["tickets"][0]["entry"]) + .await + .unwrap(); + model.contract(&related).await; + let error = model.invoke(&related, "read", json!({})).await.unwrap_err(); + assert!( + error.to_string().contains(if status == 403 { + "PermissionDenied" + } else { + "NotFound" + }), + "{error}" + ); + assert_eq!(client.requests().len(), 2); + assert_eq!(model.runtime.metrics().operation_successes, 1); + validate(&objective_collection_descriptor(), "create", &created); + } +} + +#[tokio::test] +async fn model_permissions_remain_call_time_and_pre_dispatch() { + let client = Arc::new(MockWorkspaceClient::new([detail(linked_ticket())])); + let model = Model::new(client.clone(), true, false, None); + let collection = model.inspect(&json!("/goals")).await.unwrap(); + let created = model + .invoke(&collection, "create", json!({"title":"Intent"})) + .await + .unwrap(); + client.auth_denied.store(true, Ordering::SeqCst); + let selected = model.inspect(&created["entry"]).await.unwrap(); + let error = model + .invoke(&selected, "read", json!({})) + .await + .unwrap_err(); + assert!(error.to_string().contains("PermissionDenied"), "{error}"); + assert_eq!(client.requests().len(), 1); + let checked = client.authorizations.lock().unwrap(); + assert_eq!(checked.last().unwrap().operation, "objective.read"); + drop(checked); + let denied_model = Model::new( + client.clone(), + true, + false, + Some(ToolPermissionConfig { + default_action: ToolPermissionAction::Deny, + rules: vec![], + }), + ); + let target = denied_model.inspect(&created["entry"]).await.unwrap(); + let error = denied_model + .invoke(&target, "read", json!({})) + .await + .unwrap_err(); + assert!(error.to_string().contains("PermissionDenied"), "{error}"); + assert_eq!(client.requests().len(), 1); +} + +#[tokio::test] +async fn model_item_mutations_share_descriptor_value_parity_and_preserve_domain_content() { + let lookup = || response(json!({"resource_key":"T-7"})); + let client = Arc::new(MockWorkspaceClient::new([ + detail(linked_ticket()), + detail(linked_ticket()), + lookup(), + detail(linked_ticket()), + lookup(), + detail(json!([])), + ])); + let model = Model::new(client.clone(), true, false, None); + let selected = model.inspect(&json!("/goals/O-3")).await.unwrap(); + model.contract(&selected).await; + for (operation, arguments) in [ + ("edit", json!({"title":"Edited"})), + ("set_state", json!({"state":"active"})), + ("link_ticket", json!({"ticket_id":"T-7"})), + ("unlink_ticket", json!({"ticket_id":"T-7"})), + ] { + let result = model.invoke(&selected, operation, arguments).await.unwrap(); + validate(&objective_item_descriptor(), operation, &result); + assert_eq!(result["entry"], "/goals/O-3"); + assert!(result["data"]["body"].as_str().unwrap().contains("T-999")); + assert_eq!(result["data"]["event_page"]["next_cursor"], "events-next"); + assert_eq!( + result["links"]["tickets"].as_array().unwrap().len(), + if operation == "unlink_ticket" { 0 } else { 1 } + ); + } + assert_eq!( + client.requests().len(), + 6, + "only existing link/unlink preflight reads, no result reference fetches" + ); +} + +#[test] +fn result_contract_rejects_wrong_entry_types_and_does_not_guess_links_from_json() { + let projected = json!({"objective":"O-3","linked_tickets":[{"ticket":"T-7"},{"ticket":"../hidden"}], + "body":"T-90 /cases/T-90", "events":[{"ticket":"T-91"}],"merge_request_id":"MR-92"}); + let routes = results::Routes { + objective: "/goals".into(), + }; + let value = routes + .detail(&projected, Some("/cases")) + .unwrap_or_else(|_| panic!("valid projected detail")); + let descriptor = objective_item_descriptor(); + descriptor.validate_result("read", &value).unwrap(); + let output = wip_to_json(&value).unwrap(); + assert_eq!( + output["links"]["tickets"], + json!([{"id":"T-7","entry":"/cases/T-7"},{"id":"../hidden"}]) + ); + assert_eq!(output["data"], projected); + let mut invalid = output.clone(); + invalid["entry"] = json!(7); + assert!( + descriptor + .validate_result("read", &json_to_wip(&invalid).unwrap()) + .is_err() + ); + invalid = output; + invalid["links"]["tickets"][0]["entry"] = json!("not-an-absolute-coordinate"); + assert!( + descriptor + .validate_result("read", &json_to_wip(&invalid).unwrap()) + .is_err() + ); + let unavailable = routes + .detail(&projected, None) + .unwrap_or_else(|_| panic!("valid projected detail without published Ticket namespace")); + assert_eq!( + wip_to_json(&unavailable).unwrap()["links"]["tickets"], + json!([{"id":"T-7"},{"id":"../hidden"}]) + ); +} diff --git a/crates/worker/src/feature/builtin/objective/results.rs b/crates/worker/src/feature/builtin/objective/results.rs new file mode 100644 index 000000000..fc47008e2 --- /dev/null +++ b/crates/worker/src/feature/builtin/objective/results.rs @@ -0,0 +1,164 @@ +//! Native result references are coordinates, not observations or authority. +//! +//! `data` preserves the bounded Model projection, including bodies/events and their identifiers; +//! only the explicit subject and linked-Ticket fields supply navigable references. Query paging is +//! separate from item data, avoiding duplicate copies of the same bounded items. Related Entries use +//! only the current call context's committed published result namespaces, not mount-time allocations. +//! Unpublished spaces retain display IDs but omit Entries. No result construction resolves a related +//! resource, so a later denied/missing read cannot turn a successful mutation into a failed operation +//! or cause a replay. + +use super::*; +use wip_protocol::FieldDeclaration; + +#[derive(Clone)] +pub(super) struct Routes { + pub objective: String, +} + +fn field(name: &str, required: bool, ty: TypeExpr) -> FieldDeclaration { + FieldDeclaration { + name: name.into(), + required, + documentation: None, + r#type: ty, + } +} + +pub(super) fn item_type() -> TypeExpr { + TypeExpr::Record { + fields: vec![ + field("data", true, TypeExpr::Json), + field("entry", true, TypeExpr::Entry), + field( + "links", + true, + TypeExpr::Record { + fields: vec![field( + "tickets", + true, + list_of(TypeExpr::Record { + fields: vec![ + field("id", true, TypeExpr::String), + field("entry", false, TypeExpr::Entry), + ], + }), + )], + }, + ), + ], + } +} + +pub(super) fn query_type() -> TypeExpr { + TypeExpr::Record { + fields: vec![ + field("items", true, list_of(item_type())), + field("next_cursor", false, TypeExpr::String), + field("has_more", true, TypeExpr::Boolean), + ], + } +} + +pub(super) fn declaration(query: bool) -> ReturnDeclaration { + ReturnDeclaration { + documentation: Some(Documentation { + summary: if query { + "Bounded Objective items with typed references and separate paging metadata" + } else { + "Objective data, subject Entry, and explicit linked-Ticket coordinates" + } + .into(), + details: Some( + "Inspect entry, then its Interface before read. data is bounded domain JSON, not a reference contract. links.tickets[].entry is omitted when the Ticket result namespace is unpublished or the ID is unroutable. Namespace allocation alone is not publication. Publication does not attest existence, visibility or permission: selected reads may be denied or return NotFound. Follow-up failures never require replay of a successful mutation." + .into(), + ), + }), + r#type: if query { query_type() } else { item_type() }, + } +} + +impl Routes { + pub(super) fn query( + &self, + response: &Json, + ticket_root: Option<&str>, + ) -> Result { + let items = response["objectives"] + .as_array() + .ok_or_else(|| invalid_projection("missing Model Objective query items"))? + .iter() + .map(|item| self.item(item, true, ticket_root)) + .collect::, _>>()?; + let mut fields = BTreeMap::from([ + ("items".into(), Value::List(items)), + ( + "has_more".into(), + Value::Boolean(response["has_more"].as_bool().ok_or_else(|| { + invalid_projection("missing Model Objective query paging metadata") + })?), + ), + ]); + if let Some(cursor) = response["next_cursor"].as_str() { + fields.insert("next_cursor".into(), Value::String(cursor.into())); + } + Ok(Value::Record(fields)) + } + + pub(super) fn detail( + &self, + response: &Json, + ticket_root: Option<&str>, + ) -> Result { + self.item(response, false, ticket_root) + } + + fn item( + &self, + data: &Json, + query: bool, + ticket_root: Option<&str>, + ) -> Result { + let id = objective_ref(data).map_err(invalid_projection)?; + let tickets = data["linked_tickets"] + .as_array() + .ok_or_else(|| invalid_projection("missing Model Objective linked Tickets"))? + .iter() + .filter_map(|ticket| { + // These are the two explicit Model fields, never arbitrary nested JSON/paths/text. + if query { + ticket.as_str() + } else { + ticket["ticket"].as_str() + } + }) + .map(|id| { + let mut fields = BTreeMap::from([("id".into(), Value::String(id.into()))]); + if let Some(root) = ticket_root + && is_canonical_resource_key(id, "T-") + { + fields.insert("entry".into(), Value::String(format!("{root}/{id}"))); + } + Value::Record(fields) + }) + .collect(); + Ok(Value::Record(BTreeMap::from([ + ( + "data".into(), + json_to_wip(data).map_err(WipOperationError::OutcomeUnknown)?, + ), + ( + "entry".into(), + Value::String(format!("{}/{id}", self.objective)), + ), + ( + "links".into(), + Value::Record(BTreeMap::from([("tickets".into(), Value::List(tickets))])), + ), + ]))) + } +} + +fn invalid_projection(message: impl Into) -> WipOperationError { + WipOperationError::OutcomeUnknown(message.into()) +} diff --git a/crates/worker/src/feature/builtin/ticket.rs b/crates/worker/src/feature/builtin/ticket.rs index 366b9d257..dc4d49c5d 100644 --- a/crates/worker/src/feature/builtin/ticket.rs +++ b/crates/worker/src/feature/builtin/ticket.rs @@ -26,8 +26,8 @@ //! //! # Bounded responses and observation invalidation //! -//! Query returns bounded snippets and canonical item paths; read returns bounded Backend details. -//! Objectives and Merge Requests remain references, without expanding their content or native routes. +//! Query/create/read separate typed subject and relationship Entries from bounded domain data. +//! The result boundary owns this contract; it does not eagerly expand or observe related Objects. //! Successful item-read response hashes feed route validators; canonical/internal aliases share an //! observation group. Mutation receipts are not complete representations, so updates discard //! observation hashes. Related targets and targets in queue results are also invalidated; no mutation @@ -44,6 +44,7 @@ mod assignment_public; mod assignment_tracking; mod native; mod notification_diagnostics; +mod results; mod tags; use std::collections::{BTreeMap, BTreeSet, HashMap}; @@ -270,6 +271,18 @@ enum WorkspaceTicketReadCommand { } impl WorkspaceTicketReadTool { fn execute_domain(&self, command: WorkspaceTicketReadCommand) -> Result { + let value = self.execute_value(command)?; + Ok(ToolOutput { + summary: self.kind.name().to_string(), + content: Some( + serde_json::to_string(&value) + .map_err(|error| ToolError::Internal(error.to_string()))?, + ), + attachments: Vec::new(), + }) + } + + fn execute_value(&self, command: WorkspaceTicketReadCommand) -> Result { let workspace_id = self.client.workspace_id().ok_or_else(|| { ToolError::InvalidArgument("Workspace Ticket reads require workspace identity".into()) })?; @@ -331,20 +344,15 @@ impl WorkspaceTicketReadTool { "Workspace Ticket API returned invalid JSON: {error}" )) })?; - let content = match self.kind { - WorkspaceTicketReadKind::Query => serde_json::to_string( + match self.kind { + WorkspaceTicketReadKind::Query => serde_json::to_value( &project_ticket_query(response_value).map_err(ToolError::ExecutionFailed)?, ), - WorkspaceTicketReadKind::Show => serde_json::to_string( + WorkspaceTicketReadKind::Show => serde_json::to_value( &project_ticket_detail(response_value).map_err(ToolError::ExecutionFailed)?, ), } - .map_err(|error| ToolError::Internal(error.to_string()))?; - Ok(ToolOutput { - summary: self.kind.name().to_string(), - content: Some(content), - attachments: Vec::new(), - }) + .map_err(|error| ToolError::Internal(error.to_string())) } } @@ -440,6 +448,18 @@ impl WorkspaceTicketDecisionTool { &self, value: WorkspaceTicketCaseUpdateInput, ) -> Result { + let value = self.execute_value(value)?; + Ok(ToolOutput { + summary: self.kind.name().into(), + content: Some( + serde_json::to_string(&value) + .map_err(|error| ToolError::Internal(error.to_string()))?, + ), + attachments: Vec::new(), + }) + } + + fn execute_value(&self, value: WorkspaceTicketCaseUpdateInput) -> Result { self.client.workspace_id().ok_or_else(|| { ToolError::InvalidArgument("Ticket decisions require Workspace identity".into()) })?; @@ -491,14 +511,7 @@ impl WorkspaceTicketDecisionTool { .map_err(|error| ToolError::InvalidArgument(error.to_string()))?, ) .map_err(|error| ToolError::ExecutionFailed(error.to_string()))?; - Ok(ToolOutput { - summary: self.kind.name().into(), - content: Some( - serde_json::to_string(&ticket) - .map_err(|error| ToolError::Internal(error.to_string()))?, - ), - attachments: Vec::new(), - }) + serde_json::to_value(ticket).map_err(|error| ToolError::Internal(error.to_string())) } } @@ -2049,7 +2062,7 @@ pub fn mount_workspace_http_ticket_wip( &collection_tools, &roots, "Search and create authoritative Workspace Tickets", - "Search results remain bounded and carry canonical item paths. Operations are exactly the Ticket tools enabled for this Worker.", + "Search results remain bounded and separate typed Entries from domain data. Operations are exactly the Ticket tools enabled for this Worker.", )?; registry.mount(WipProjection { route: collection_route.clone(), @@ -2103,6 +2116,7 @@ pub fn mount_workspace_http_ticket_wip( }), })?; registry.replace_compatibility_tools(&collection_route, claimed_tools)?; + registry.publish_result_namespace(namespace_route)?; Ok(()) } @@ -2140,26 +2154,19 @@ impl WipOperationHandler for TicketCollectionWipHandler { &self.merge_request_route, &self.objective_route, )?; - let output = execute_native_ticket_tool(tool, &self.permissions, input, context).await?; - let mut response = ticket_tool_output_json(output)?; - match operation { - "query" => add_ticket_paths(&mut response, &self.collection_route), - "create" => { - record_ticket_observation(&self.observations, None, &response, true); - if let Some(reference) = ticket_reference(&response).map(ToOwned::to_owned) { - response.as_object_mut().map(|object| { - object.insert( - "path".into(), - Value::String(format!("{}/{}", self.collection_route, reference)), - ) - }); - } - } - _ => {} + let result_namespaces = context.result_namespaces.clone(); + let response = execute_native_ticket_tool(tool, &self.permissions, input, context).await?; + if operation == "create" { + record_ticket_observation(&self.observations, None, &response, true); } - Ok(WipOperationOutput::native( - json_to_wip(&response).map_err(WipOperationError::OutcomeUnknown)?, - )) + Ok(WipOperationOutput::native(results::project( + operation, + response, + None, + &self.collection_route, + result_namespaces.root("merge-request").unwrap_or_default(), + result_namespaces.root("objective").unwrap_or_default(), + )?)) } } @@ -2236,8 +2243,8 @@ impl WipOperationHandler for TicketItemWipHandler { &self.merge_request_route, &self.objective_route, )?; - let output = execute_native_ticket_tool(tool, &self.permissions, input, context).await?; - let response = ticket_tool_output_json(output)?; + let result_namespaces = context.result_namespaces.clone(); + let response = execute_native_ticket_tool(tool, &self.permissions, input, context).await?; // Classification has its own membership CAS and never changes item content. if operation != "tags_set" { record_ticket_observation( @@ -2250,9 +2257,14 @@ impl WipOperationHandler for TicketItemWipHandler { if tool.projection.mutating && operation != "tags_set" { record_affected_ticket_mutations(&self.observations, &self.ticket_reference, &response); } - Ok(WipOperationOutput::native( - json_to_wip(&response).map_err(WipOperationError::OutcomeUnknown)?, - )) + Ok(WipOperationOutput::native(results::project( + operation, + response, + Some(&self.ticket_reference), + &self.collection_route, + result_namespaces.root("merge-request").unwrap_or_default(), + result_namespaces.root("objective").unwrap_or_default(), + )?)) } } @@ -2261,13 +2273,98 @@ async fn execute_native_ticket_tool( permissions: &Option, input: Value, context: WipCallContext, -) -> Result { +) -> Result { authorize_native_ticket(permissions, &tool.name, &input)?; - execute_ticket_domain(tool.client.clone(), &tool.name, input, context.execution) + if let Some(value) = assignment_public::execute_native_value( + tool.client.clone(), + &tool.name, + input.clone(), + context.execution.clone(), + ) + .await + { + return value.map_err(map_ticket_tool_error); + } + if let Some(value) = assignment_tracking::execute_native_value( + tool.client.clone(), + &tool.name, + input.clone(), + context.execution.clone(), + ) + .await + { + return value.map_err(map_ticket_tool_error); + } + if let Some(value) = notification_diagnostics::execute_native_value( + tool.client.clone(), + &tool.name, + input.clone(), + context.execution.clone(), + ) + .await + { + return value.map_err(map_ticket_tool_error); + } + // Classification values contain no independently published object references. + if let Some(output) = tags::execute_native( + tool.client.clone(), + &tool.name, + input.clone(), + context.execution.clone(), + ) + .await + { + return ticket_tool_output_json(output.map_err(map_ticket_tool_error)?); + } + let value = match tool.name.as_str() { + "QueryTicket" | "ShowTicket" => { + let (kind, command) = if tool.name == "QueryTicket" { + ( + WorkspaceTicketReadKind::Query, + WorkspaceTicketReadCommand::Query(serde_json::from_value(input).map_err( + |error| { + map_ticket_tool_error(ToolError::InvalidArgument(error.to_string())) + }, + )?), + ) + } else { + ( + WorkspaceTicketReadKind::Show, + WorkspaceTicketReadCommand::Show(serde_json::from_value(input).map_err( + |error| { + map_ticket_tool_error(ToolError::InvalidArgument(error.to_string())) + }, + )?), + ) + }; + WorkspaceTicketReadTool { + client: tool.client.clone(), + kind, + } + .execute_value(command) + .map_err(map_ticket_tool_error)? + } + "TicketCaseUpdate" => WorkspaceTicketDecisionTool { + client: tool.client.clone(), + kind: WorkspaceTicketDecisionKind::CaseUpdate, + } + .execute_value(serde_json::from_value(input).map_err(|error| { + map_ticket_tool_error(ToolError::InvalidArgument(error.to_string())) + })?) + .map_err(map_ticket_tool_error)?, + _ => ticket::tool::execute_ticket_command_value( + TicketToolBackend::new(WorkspaceHttpTicketBackend::new(tool.client.clone())), + &tool.name, + input, + context.execution, + ) .await - .map_err(map_ticket_tool_error) + .map_err(map_ticket_tool_error)?, + }; + Ok(value) } +#[cfg(test)] async fn execute_ticket_domain( client: Arc, name: &str, @@ -2457,29 +2554,6 @@ fn ticket_tool_output_json(output: ToolOutput) -> Result Option<&str> { response .get("ticket") @@ -2623,7 +2697,14 @@ fn ticket_operation_declaration( tool: &NativeTicketTool, roots: &crate::feature::builtin::native_contract::ReferenceRoots<'_>, ) -> Result { - Ok(native::contract(tool.projection.operation).declaration(roots)) + let mut declaration = native::contract(tool.projection.operation).declaration(roots); + if let Some(returns) = results::declaration( + tool.projection.operation, + tool.projection.surface == NativeTicketSurface::Item, + ) { + declaration.returns = returns; + } + Ok(declaration) } fn protocol_failure(code: ProtocolErrorCode, message: impl Into) -> WipOperationError { @@ -2789,6 +2870,7 @@ mod tests { &None, decision_input(), WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: String::new(), }, @@ -3248,6 +3330,7 @@ mod tests { &None, decision_input(), WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "self-reported-coder".into(), }, @@ -3274,6 +3357,7 @@ mod tests { &permissions, decision_input(), WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "author".into(), }, @@ -3365,6 +3449,7 @@ mod tests { "case_update", &arguments, WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "coder".into(), }, @@ -4475,6 +4560,7 @@ mod tests { json_to_wip(&json!({"title":"Create"})).unwrap(), )]), WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "test".into(), }, @@ -4892,6 +4978,7 @@ mod tests { json_to_wip(&json!({"title":"Authorized create"})).unwrap(), )]), WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "test".into(), }, @@ -5223,6 +5310,7 @@ mod tests { "read", &BTreeMap::new(), WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "test".into(), }, diff --git a/crates/worker/src/feature/builtin/ticket/assignment_public.rs b/crates/worker/src/feature/builtin/ticket/assignment_public.rs index 23939d959..6b395a780 100644 --- a/crates/worker/src/feature/builtin/ticket/assignment_public.rs +++ b/crates/worker/src/feature/builtin/ticket/assignment_public.rs @@ -263,13 +263,26 @@ impl Tool for PublicAssignmentTool { } } -/// Convert an adapter-owned internal command; never invokes Tool execution or its schema. +/// Preserve the regular Tool presentation for internal callers. +#[cfg(test)] pub(super) async fn execute_native( client: Arc, tool_name: &str, command: Value, ctx: ToolExecutionContext, ) -> Option> { + execute_native_value(client, tool_name, command, ctx) + .await + .map(|result| result.map(|content| tool_output(tool_name, content))) +} + +/// Return the shared domain projection before Tool presentation serialization. +pub(super) async fn execute_native_value( + client: Arc, + tool_name: &str, + command: Value, + ctx: ToolExecutionContext, +) -> Option> { let parse = |error: serde_json::Error| ToolError::InvalidArgument(error.to_string()); let command = match tool_name { "TicketOrchestratorAssign" => serde_json::from_value(command).map(Command::Assign), @@ -286,7 +299,7 @@ pub(super) async fn execute_native( _ => return None, }; Some(match command { - Ok(command) => execute_domain(client, command, ctx).await, + Ok(command) => execute_value(client, command, ctx).await, Err(error) => Err(parse(error)), }) } @@ -296,6 +309,25 @@ async fn execute_domain( command: Command, ctx: ToolExecutionContext, ) -> Result { + let name = command.kind().name(); + execute_value(client, command, ctx) + .await + .map(|content| tool_output(name, content)) +} + +fn tool_output(name: &str, content: Value) -> ToolOutput { + ToolOutput { + summary: name.into(), + content: Some(content.to_string()), + attachments: vec![], + } +} + +async fn execute_value( + client: Arc, + command: Command, + ctx: ToolExecutionContext, +) -> Result { let kind = command.kind(); client .workspace_id() @@ -518,11 +550,7 @@ async fn execute_domain( ), } .map_err(|error| ToolError::Internal(error.to_string()))?; - Ok(ToolOutput { - summary: kind.name().into(), - content: Some(content.to_string()), - attachments: vec![], - }) + Ok(content) } pub(super) fn definition( @@ -549,7 +577,10 @@ mod native_command_tests { #[tokio::test] async fn internal_commands_share_exact_authorization_requests_and_host_retry_identity() { - for (kind, input, response) in super::tests::cases() { + for (kind, input, mut response) in super::tests::cases() { + if matches!(kind, PublicAssignmentKind::Diagnostics) { + response["backend_private_field"] = json!("must be redacted"); + } let operation = match kind { PublicAssignmentKind::Assign => "ticket.assignment.assign", PublicAssignmentKind::Clear => "ticket.assignment.clear", @@ -577,9 +608,24 @@ mod native_command_tests { .await .unwrap() .unwrap(); + let value = + execute_native_value(client.clone(), kind.name(), input.clone(), ctx.clone()) + .await + .unwrap() + .unwrap(); + assert_eq!( + value, + serde_json::from_str::(ordinary.content.as_deref().unwrap()).unwrap() + ); + assert!(value.get("backend_private_field").is_none()); assert_eq!(native.content, ordinary.content); + assert_eq!(native.summary, ordinary.summary); + assert_eq!(ordinary.summary, kind.name()); + assert!(native.attachments.is_empty()); + assert_eq!(client.requests.lock().unwrap().len(), 3); { let requests = client.requests.lock().unwrap(); + assert_eq!(requests[1], requests[2]); assert_eq!( requests[0], requests[1], @@ -598,21 +644,21 @@ mod native_command_tests { let mut other = input.clone(); other["ticket"] = json!("T-other"); assert!( - execute_native(client.clone(), kind.name(), other, ctx.clone()) + execute_native_value(client.clone(), kind.name(), other, ctx.clone()) .await .unwrap() .is_err() ); client.grants.lock().unwrap().clear(); assert!( - execute_native(client.clone(), kind.name(), input, ctx) + execute_native_value(client.clone(), kind.name(), input, ctx) .await .unwrap() .is_err() ); - assert_eq!(client.requests.lock().unwrap().len(), 2); + assert_eq!(client.requests.lock().unwrap().len(), 3); assert!( - execute_native( + execute_native_value( client.clone(), "Unknown", Value::Null, diff --git a/crates/worker/src/feature/builtin/ticket/assignment_public/authorization_tests.rs b/crates/worker/src/feature/builtin/ticket/assignment_public/authorization_tests.rs index 05d2f424e..0ec125d57 100644 --- a/crates/worker/src/feature/builtin/ticket/assignment_public/authorization_tests.rs +++ b/crates/worker/src/feature/builtin/ticket/assignment_public/authorization_tests.rs @@ -65,6 +65,7 @@ async fn public_operations_require_exact_ticket_grants_on_tool_and_wip() { projection.operation, &args, WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "worker".into(), }, @@ -79,6 +80,7 @@ async fn public_operations_require_exact_ticket_grants_on_tool_and_wip() { projection.operation, &args, WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "worker".into() } diff --git a/crates/worker/src/feature/builtin/ticket/assignment_public/tests.rs b/crates/worker/src/feature/builtin/ticket/assignment_public/tests.rs index 7381a667c..1dc8fa329 100644 --- a/crates/worker/src/feature/builtin/ticket/assignment_public/tests.rs +++ b/crates/worker/src/feature/builtin/ticket/assignment_public/tests.rs @@ -779,6 +779,7 @@ async fn implementation_result_wip_and_native_dispatch_preserve_trusted_call_con "implementation_result", &args, WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::new(call_id, "wip", 3), security_context: "worker".into(), }, @@ -803,6 +804,7 @@ async fn implementation_result_wip_and_native_dispatch_preserve_trusted_call_con "implementation_result", &args, WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::new("", "wip", 0), security_context: "worker".into(), }, @@ -827,6 +829,7 @@ async fn implementation_result_wip_and_native_dispatch_preserve_trusted_call_con "implementation_result", &forged, WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::new("call-c", "wip", 0), security_context: "worker".into(), }, @@ -868,6 +871,7 @@ async fn assignment_wip_binds_ticket_and_preserves_normal_permission_and_cas_inp operation, &args, WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "worker".into(), }, @@ -883,6 +887,7 @@ async fn assignment_wip_binds_ticket_and_preserves_normal_permission_and_cas_inp operation, &forged, WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "worker".into() } @@ -919,6 +924,7 @@ async fn assignment_wip_binds_ticket_and_preserves_normal_permission_and_cas_inp "assignment_answer", &args, WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "worker".into() } diff --git a/crates/worker/src/feature/builtin/ticket/assignment_tracking.rs b/crates/worker/src/feature/builtin/ticket/assignment_tracking.rs index 712d0852d..00040414b 100644 --- a/crates/worker/src/feature/builtin/ticket/assignment_tracking.rs +++ b/crates/worker/src/feature/builtin/ticket/assignment_tracking.rs @@ -179,13 +179,26 @@ impl Tool for TrackingTool { } } -/// Convert an adapter-owned internal command; never invokes Tool execution or its schema. +/// Preserve the regular Tool presentation for internal callers. +#[cfg(test)] pub(super) async fn execute_native( client: Arc, tool_name: &str, command: Value, ctx: ToolExecutionContext, ) -> Option> { + execute_native_value(client, tool_name, command, ctx) + .await + .map(|result| result.map(|content| tool_output(tool_name, content))) +} + +/// Return the shared domain projection before Tool presentation serialization. +pub(super) async fn execute_native_value( + client: Arc, + tool_name: &str, + command: Value, + ctx: ToolExecutionContext, +) -> Option> { let parse = |error: serde_json::Error| ToolError::InvalidArgument(error.to_string()); let command = match tool_name { "TicketAssignmentTrackingQuery" => serde_json::from_value(command).map(Command::Query), @@ -194,7 +207,7 @@ pub(super) async fn execute_native( _ => return None, }; Some(match command { - Ok(command) => execute_domain(client, command, ctx).await, + Ok(command) => execute_value(client, command, ctx).await, Err(error) => Err(parse(error)), }) } @@ -202,8 +215,27 @@ pub(super) async fn execute_native( async fn execute_domain( client: Arc, command: Command, - _ctx: ToolExecutionContext, + ctx: ToolExecutionContext, ) -> Result { + let name = command.kind().name(); + execute_value(client, command, ctx) + .await + .map(|content| tool_output(name, content)) +} + +fn tool_output(name: &str, content: Value) -> ToolOutput { + ToolOutput { + summary: name.into(), + content: Some(content.to_string()), + attachments: vec![], + } +} + +async fn execute_value( + client: Arc, + command: Command, + _ctx: ToolExecutionContext, +) -> Result { let kind = command.kind(); let workspace = client .workspace_id() @@ -306,11 +338,7 @@ async fn execute_domain( "Backend returned invalid assignment tracking JSON: {error}" )) })?; - Ok(ToolOutput { - summary: kind.name().into(), - content: Some(content.to_string()), - attachments: Vec::new(), - }) + Ok(content) } pub(super) fn definition( @@ -624,14 +652,32 @@ mod native_command_tests { .await .unwrap() .unwrap(); + let value = execute_native_value( + client.clone(), + kind.name(), + input.clone(), + ToolExecutionContext::direct(), + ) + .await + .unwrap() + .unwrap(); + assert_eq!( + value, + serde_json::from_str::(ordinary.content.as_deref().unwrap()).unwrap() + ); assert_eq!(native.content, ordinary.content); + assert_eq!(native.summary, ordinary.summary); + assert_eq!(ordinary.summary, kind.name()); + assert!(native.attachments.is_empty()); + assert_eq!(client.requests.lock().unwrap().len(), 3); { let requests = client.requests.lock().unwrap(); assert_eq!(requests[0], requests[1]); + assert_eq!(requests[1], requests[2]); } client.grants.lock().unwrap().clear(); assert!( - execute_native( + execute_native_value( client.clone(), kind.name(), input, @@ -641,9 +687,9 @@ mod native_command_tests { .unwrap() .is_err() ); - assert_eq!(client.requests.lock().unwrap().len(), 2); + assert_eq!(client.requests.lock().unwrap().len(), 3); assert!( - execute_native( + execute_native_value( client, "Unknown", Value::Null, diff --git a/crates/worker/src/feature/builtin/ticket/assignment_tracking/authorization_tests.rs b/crates/worker/src/feature/builtin/ticket/assignment_tracking/authorization_tests.rs index 7b19dd4ce..a869b5acb 100644 --- a/crates/worker/src/feature/builtin/ticket/assignment_tracking/authorization_tests.rs +++ b/crates/worker/src/feature/builtin/ticket/assignment_tracking/authorization_tests.rs @@ -87,17 +87,17 @@ async fn tracking_operations_authorize_exact_ticket_before_dispatch_and_recheck_ ( TrackingToolKind::Query, "ticket.assignment.query", - json!({"ticket":"T-A"}), + json!({"ticket":"T-1"}), ), ( TrackingToolKind::History, "ticket.assignment.history", - json!({"ticket":"T-A","assignment_id":"a"}), + json!({"ticket":"T-1","assignment_id":"a"}), ), ( TrackingToolKind::Update, "ticket.assignment.update", - json!({"ticket":"T-A","assignment_id":"a","expected_last_event_id":"before","operation_key":"report","tracking":{"status":"done","waiting_reason":null,"progress":null,"next_action":null,"result":"Returned options"}}), + json!({"ticket":"T-1","assignment_id":"a","expected_last_event_id":"before","operation_key":"report","tracking":{"status":"done","waiting_reason":null,"progress":null,"next_action":null,"result":"Returned options"}}), ), ] { let client = AuthorizedClient::new(json!({"assignment_id":"a","last_event_id":"after"})); @@ -110,7 +110,7 @@ async fn tracking_operations_authorize_exact_ticket_before_dispatch_and_recheck_ .contains("HTTP status 403") ); assert!(client.requests.lock().unwrap().is_empty()); - client.grant(operation, ticket("T-A")); + client.grant(operation, ticket("T-1")); let output = tool .execute(&input.to_string(), ToolExecutionContext::direct()) .await @@ -121,7 +121,7 @@ async fn tracking_operations_authorize_exact_ticket_before_dispatch_and_recheck_ ); assert_eq!(client.requests.lock().unwrap().len(), 1); let mut other = input.clone(); - other["ticket"] = json!("T-B"); + other["ticket"] = json!("T-2"); assert!( tool.execute(&other.to_string(), ToolExecutionContext::direct()) .await @@ -137,8 +137,8 @@ async fn tracking_operations_authorize_exact_ticket_before_dispatch_and_recheck_ let auth = client.authorizations.lock().unwrap(); assert_eq!(auth.len(), 4); assert!(auth.iter().all(|request| request.operation == operation)); - assert_eq!(auth[0].resource, ticket("T-A")); - assert_eq!(auth[2].resource, ticket("T-B")); + assert_eq!(auth[0].resource, ticket("T-1")); + assert_eq!(auth[2].resource, ticket("T-2")); drop(auth); let tools = native_ticket_tools(client.clone(), TicketFeatureAccess::workflow()).unwrap(); let handler = TicketItemWipHandler { @@ -149,7 +149,7 @@ async fn tracking_operations_authorize_exact_ticket_before_dispatch_and_recheck_ .collect(), ), permissions: None, - ticket_reference: "T-A".into(), + ticket_reference: "T-1".into(), collection_route: "/tickets".into(), merge_request_route: "/merge-requests".into(), objective_route: "/objectives".into(), @@ -198,6 +198,7 @@ async fn tracking_operations_authorize_exact_ticket_before_dispatch_and_recheck_ projection.operation, &args, WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "worker".into() } @@ -209,12 +210,13 @@ async fn tracking_operations_authorize_exact_ticket_before_dispatch_and_recheck_ })) )); assert_eq!(client.requests.lock().unwrap().len(), 1); - client.grant(operation, ticket("T-A")); + client.grant(operation, ticket("T-1")); handler .call( projection.operation, &args, WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "worker".into(), }, diff --git a/crates/worker/src/feature/builtin/ticket/native.rs b/crates/worker/src/feature/builtin/ticket/native.rs index ea5e9b16d..06caabc81 100644 --- a/crates/worker/src/feature/builtin/ticket/native.rs +++ b/crates/worker/src/feature/builtin/ticket/native.rs @@ -9,11 +9,11 @@ //! The contract tree generates both the public declaration and closed input conversion. The //! converted command is private transport to shared domain execution, never a second public API. //! Backend validation owns string bounds, conditional business requirements, exact observations, -//! authorization and replay. Native type rejection precedes that boundary. Results deliberately -//! retain their bounded domain JSON: typed result traversal belongs to the separate result seam. +//! authorization and replay. Native type rejection precedes that boundary. The result projection +//! separates typed navigation from bounded domain JSON, independently of these input contracts. //! //! For example, Inspect the collection and its Interface, then invoke `query` with -//! `{"filter":{"states":["open"]},"page":{"limit":10}}`. Inspect a returned item path and +//! `{"filter":{"states":["open"]},"page":{"limit":10}}`. Inspect a returned typed item Entry and //! its Interface before `read` with `{"page":{"limit":5}}`, or `comment` with //! `{"body":"Confirmed requirement"}`. A relation target is an Entry, not a Tool Ticket ID. //! v2 replaces v1 native arguments; refresh Object/Interface observations, do not replay mutations. diff --git a/crates/worker/src/feature/builtin/ticket/notification_diagnostics.rs b/crates/worker/src/feature/builtin/ticket/notification_diagnostics.rs index 7d32501a2..79a078f44 100644 --- a/crates/worker/src/feature/builtin/ticket/notification_diagnostics.rs +++ b/crates/worker/src/feature/builtin/ticket/notification_diagnostics.rs @@ -19,18 +19,31 @@ impl Tool for DiagnosticsTool { } } -/// Convert an adapter-owned internal command without Tool execution or schema validation. +/// Preserve the regular Tool presentation for internal callers. +#[cfg(test)] pub(super) async fn execute_native( client: Arc, tool_name: &str, command: Value, - _ctx: ToolExecutionContext, + ctx: ToolExecutionContext, ) -> Option> { + execute_native_value(client, tool_name, command, ctx) + .await + .map(|result| result.map(|content| tool_output(tool_name, content))) +} + +/// Return the shared domain projection before Tool presentation serialization. +pub(super) async fn execute_native_value( + client: Arc, + tool_name: &str, + command: Value, + _ctx: ToolExecutionContext, +) -> Option> { if tool_name != NAME { return None; } Some(match serde_json::from_value(command) { - Ok(query) => execute_domain(client, query).await, + Ok(query) => execute_value(client, query).await, Err(error) => Err(ToolError::InvalidArgument(error.to_string())), }) } @@ -39,6 +52,23 @@ async fn execute_domain( client: Arc, query: server_api::NotificationDiagnosticsQuery, ) -> Result { + execute_value(client, query) + .await + .map(|content| tool_output(NAME, content)) +} + +fn tool_output(name: &str, content: Value) -> ToolOutput { + ToolOutput { + summary: name.into(), + content: Some(content.to_string()), + attachments: vec![], + } +} + +async fn execute_value( + client: Arc, + query: server_api::NotificationDiagnosticsQuery, +) -> Result { let workspace_id = client .workspace_id() .filter(|s| !s.trim().is_empty()) @@ -147,13 +177,7 @@ async fn execute_domain( "Notification response exceeds 100 items".into(), )); } - Ok(ToolOutput { - summary: NAME.into(), - content: Some( - serde_json::to_string(&page).map_err(|e| ToolError::Internal(e.to_string()))?, - ), - attachments: vec![], - }) + serde_json::to_value(page).map_err(|e| ToolError::Internal(e.to_string())) } #[cfg(test)] mod authorization_tests { @@ -284,6 +308,7 @@ mod authorization_tests { "notification_diagnostics", &args, WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "worker".into(), }, @@ -297,6 +322,7 @@ mod authorization_tests { "notification_diagnostics", &args, WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "worker".into() } @@ -355,6 +381,7 @@ mod authorization_tests { "notification_diagnostics", &args, WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "worker".into(), }, @@ -376,6 +403,7 @@ mod authorization_tests { "notification_diagnostics", &args, WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "worker".into() } @@ -492,8 +520,9 @@ mod native_command_tests { #[tokio::test] async fn internal_notification_command_requires_all_origins_and_shares_tool_request() { - let client = - AuthorizedClient::new(json!({"items":[],"next_cursor":null,"source_checks":[]})); + let client = AuthorizedClient::new(json!({ + "items":[], "next_cursor":"source/8?", "source_checks":[] + })); let input = json!({"ticket":"T-A","job_id":"Job-A","cursor":"source/7?","limit":2}); client.grant("notification.diagnostics.read", ticket("T-A")); assert!( @@ -529,21 +558,40 @@ mod native_command_tests { .await .unwrap() .unwrap(); + let value = execute_native_value( + client.clone(), + NAME, + input.clone(), + ToolExecutionContext::direct(), + ) + .await + .unwrap() + .unwrap(); + assert_eq!( + value, + serde_json::from_str::(ordinary.content.as_deref().unwrap()).unwrap() + ); + assert_eq!(value["next_cursor"], "source/8?"); assert_eq!(native.content, ordinary.content); + assert_eq!(native.summary, ordinary.summary); + assert_eq!(ordinary.summary, NAME); + assert!(native.attachments.is_empty()); + assert_eq!(client.requests.lock().unwrap().len(), 3); { let requests = client.requests.lock().unwrap(); assert_eq!(requests[0], requests[1]); + assert_eq!(requests[1], requests[2]); } client.grants.lock().unwrap().clear(); assert!( - execute_native(client.clone(), NAME, input, ToolExecutionContext::direct()) + execute_native_value(client.clone(), NAME, input, ToolExecutionContext::direct()) .await .unwrap() .is_err() ); - assert_eq!(client.requests.lock().unwrap().len(), 2); + assert_eq!(client.requests.lock().unwrap().len(), 3); assert!( - execute_native( + execute_native_value( client.clone(), "Unknown", Value::Null, diff --git a/crates/worker/src/feature/builtin/ticket/notification_diagnostics/tests.rs b/crates/worker/src/feature/builtin/ticket/notification_diagnostics/tests.rs index f065a8d5e..17e1fbcb3 100644 --- a/crates/worker/src/feature/builtin/ticket/notification_diagnostics/tests.rs +++ b/crates/worker/src/feature/builtin/ticket/notification_diagnostics/tests.rs @@ -34,6 +34,7 @@ async fn notification_wip_executes_same_read_and_honors_profile_denial() { "notification_diagnostics", &args, WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "worker".into(), }, diff --git a/crates/worker/src/feature/builtin/ticket/results.rs b/crates/worker/src/feature/builtin/ticket/results.rs new file mode 100644 index 000000000..13072cd84 --- /dev/null +++ b/crates/worker/src/feature/builtin/ticket/results.rs @@ -0,0 +1,370 @@ +//! Native result navigation is separate from bounded Ticket data. +//! +//! Only explicit domain reference fields and the route-bound subject produce Entries. IDs in +//! bodies, tracking prose, events, closure evidence, cursors, selectors and Worker/assignment +//! identities remain data; they are not independently published Ticket/Objective/MR Objects. +//! `data` retains the evolving bounded domain representation as Json, while `entry` and `links` +//! are declared Records/Entries. Consumers select typed fields, never parse data or signatures to +//! recover routes. Query pagination is declared separately; a missing cursor is not an empty ID. +//! +//! Cross-domain roots come from the final committed Host publication snapshot at dispatch, never +//! namespace allocation or intermediate mount order. Unpublished domains omit optional Entries +//! but keep already-visible IDs. References attest neither existence nor permission: Inspect may +//! describe the route-bound interface, and read still performs current authorization and can return +//! PermissionDenied/NotFound. No target is fetched here, indexed into Tree, or used to retry a +//! completed mutation. Historical dependencies and saved evidence are navigation, not start rights. +//! +//! Refresh native descriptors after upgrading. Query returns `items` containing `entry`, `data` +//! and `links`; creation and item operations return one such envelope. Notification diagnostics +//! have no independently published diagnostic item, so return only `data` and `links`. Tag catalog +//! operations remain Json because tags are classification data, not native Objects. Tool mode is +//! unchanged. Value dispatch precedes presentation; result mapping never reparses a ToolOutput. + +use super::*; +use wip_protocol::{FieldDeclaration, ReturnDeclaration, TypeExpr}; + +fn field(name: &str, required: bool, r#type: TypeExpr) -> FieldDeclaration { + FieldDeclaration { + name: name.into(), + documentation: None, + required, + r#type, + } +} +fn record(fields: Vec) -> TypeExpr { + TypeExpr::Record { fields } +} +fn list(items: TypeExpr) -> TypeExpr { + TypeExpr::List { + items: Box::new(items), + } +} +fn link_type() -> TypeExpr { + record(vec![ + field("id", true, TypeExpr::String), + field("entry", false, TypeExpr::Entry), + ]) +} +fn item_type(subject: bool) -> TypeExpr { + let mut fields = vec![ + field("data", true, TypeExpr::Json), + field( + "links", + true, + record(vec![ + field("tickets", true, list(link_type())), + field("objectives", true, list(link_type())), + field("merge_requests", true, list(link_type())), + ]), + ), + ]; + if subject { + fields.insert(0, field("entry", true, TypeExpr::Entry)); + } + record(fields) +} +pub(super) fn declaration(operation: &str, item: bool) -> Option { + let ty = match operation { + "query" => record(vec![ + field("items", true, list(item_type(true))), + field("next_cursor", false, TypeExpr::String), + field("has_more", true, TypeExpr::Boolean), + ]), + "create" => item_type(true), + "notification_diagnostics" => item_type(false), + _ if item => item_type(true), + _ => return None, + }; + Some(ReturnDeclaration{documentation:Some(Documentation{ + summary:"Typed subject/related Entries separate from bounded domain data and paging. Entries are lazy coordinates, never authority or existence attestations.".into(), + details:Some("Inspect the selected entry, then its Interface, and read. IDs, cursors and body/event/evidence JSON in data are not navigation. Missing cross-domain publication omits link.entry; unpublished, deleted, or denied targets fail only when selected, without replaying the successful operation.".into()), + }),r#type:ty}) +} +fn failure(message: &str) -> WipOperationError { + WipOperationError::OutcomeUnknown(format!("Ticket result projection: {message}")) +} +fn leaf(value: &Value, name: &str) -> Option { + value.get(name).and_then(Value::as_str).map(str::to_owned) +} +fn array<'a>(value: &'a Value, name: &str) -> &'a [Value] { + value + .get(name) + .and_then(Value::as_array) + .map(Vec::as_slice) + .unwrap_or_default() +} +fn add(ids: &mut BTreeSet, value: &Value, name: &str) { + if let Some(id) = leaf(value, name) { + ids.insert(id); + } +} +fn strings(ids: &mut BTreeSet, value: &Value, name: &str) { + ids.extend( + array(value, name) + .iter() + .filter_map(Value::as_str) + .map(str::to_owned), + ); +} +fn ticket_leaf(id: &str) -> bool { + is_ticket_route_reference(id) +} +fn mr_leaf(id: &str) -> bool { + !id.is_empty() + && id.len() <= 128 + && id != "." + && id != ".." + && id + .bytes() + .all(|b| b.is_ascii_alphanumeric() || matches!(b, b'-' | b'_' | b'.' | b'~')) +} +fn objective_leaf(id: &str) -> bool { + id.strip_prefix("O-") + .is_some_and(|n| !n.is_empty() && n.bytes().all(|b| b.is_ascii_digit())) + || ((8..=128).contains(&id.len()) && id.bytes().all(|b| b.is_ascii_alphanumeric())) +} +fn links(ids: BTreeSet, root: &str, valid: fn(&str) -> bool) -> WipValue { + WipValue::List( + ids.into_iter() + .map(|id| { + let entry = (!root.is_empty() && valid(&id)).then(|| format!("{root}/{id}")); + let mut fields = BTreeMap::from([("id".into(), WipValue::String(id))]); + if let Some(entry) = entry { + fields.insert("entry".into(), WipValue::String(entry)); + } + WipValue::Record(fields) + }) + .collect(), + ) +} +#[derive(Default)] +struct References { + tickets: BTreeSet, + objectives: BTreeSet, + mrs: BTreeSet, +} +impl References { + fn select(operation: &str, data: &Value) -> Self { + let mut refs = Self::default(); + match operation { + "query" | "read" => { + // ModelTicketQueryItem/ModelTicketDetail contain redacted, bounded relationships. + if let Some(relations) = data.get("relations") { + for kind in ["outgoing", "incoming", "blockers"] { + for relation in array(relations, kind) { + add(&mut refs.tickets, relation, "ticket"); + } + } + } + for objective in array(data, "linked_objectives") { + if let Some(id) = objective.as_str() { + refs.objectives.insert(id.into()); + } else { + add(&mut refs.objectives, objective, "objective"); + } + } + for mr in array(data, "merge_requests") { + add(&mut refs.mrs, mr, "merge_request_id"); + } + if let Some(mr) = data.get("merge_request") { + add(&mut refs.mrs, mr, "merge_request_id"); + } + } + "edit" | "edit_post" | "case_update" => { + // Shared domain Ticket/detail results use a distinct, explicit relation schema. + if let Some(relations) = data.get("relations") { + for relation in array(relations, "outgoing") { + add(&mut refs.tickets, relation, "ticket_id"); + add(&mut refs.tickets, relation, "target"); + } + for (kind, field) in [ + ("incoming", "source_ticket"), + ("blockers", "blocking_ticket"), + ("notices", "related_ticket"), + ] { + for relation in array(relations, kind) { + add(&mut refs.tickets, relation, field); + } + } + } + } + "record_relation" | "remove_relation" => { + add(&mut refs.tickets, data, "ticket_id"); + add(&mut refs.tickets, data, "target"); + } + "record_orchestration_plan" => { + add(&mut refs.tickets, data, "ticket_id"); + add(&mut refs.tickets, data, "related_ticket"); + } + "query_orchestration_plans" => { + for plan in array(data, "records") { + add(&mut refs.tickets, plan, "ticket_id"); + add(&mut refs.tickets, plan, "related_ticket"); + } + } + "dependency_check" => { + if let Some(ticket) = data.get("ticket") { + if let Some(id) = leaf(ticket, "resource_key").or_else(|| leaf(ticket, "id")) { + refs.tickets.insert(id); + } + } + for blocker in array(data, "blockers") { + add(&mut refs.tickets, blocker, "blocking_ticket"); + } + strings(&mut refs.tickets, data, "queue_tickets"); + } + "implementation_result" | "implementation_result_query" => { + add(&mut refs.tickets, data, "ticket_id"); + if let Some(mr) = data + .get("request") + .and_then(|request| request.get("merge_request")) + { + add(&mut refs.mrs, mr, "merge_request_id"); + } + } + "assignment_tracking_query" => { + // The domain's current/retained tracking lookup is a list, not an Object per row. + if let Some(rows) = data.as_array() { + for row in rows { + add(&mut refs.tickets, row, "ticket_id"); + } + } + } + "assignment_tracking_update" => { + add(&mut refs.tickets, data, "ticket_id"); + } + "assignments_query" => { + for row in array(data, "items") { + add(&mut refs.tickets, row, "ticket_id"); + } + } + "orchestrator_assign" | "orchestrator_clear" => { + add(&mut refs.tickets, data, "ticket_id"); + if let Some(row) = data.get("assignment") { + add(&mut refs.tickets, row, "ticket_id"); + } + } + "notification_diagnostics" => { + for row in array(data, "items") { + add(&mut refs.tickets, row, "ticket"); + add(&mut refs.mrs, row, "merge_request_id"); + } + } + _ => {} // Tag/answer/tracking-history/delivery identifiers and prose are domain data. + } + refs + } +} +fn envelope( + operation: &str, + data: Value, + subject: Option<&str>, + ticket_root: &str, + mr_root: &str, + objective_root: &str, +) -> Result { + let refs = References::select(operation, &data); + let mut fields = BTreeMap::from([ + ( + "data".into(), + json_to_wip(&data).map_err(WipOperationError::OutcomeUnknown)?, + ), + ( + "links".into(), + WipValue::Record(BTreeMap::from([ + ( + "tickets".into(), + links(refs.tickets, ticket_root, ticket_leaf), + ), + ( + "objectives".into(), + links(refs.objectives, objective_root, objective_leaf), + ), + ("merge_requests".into(), links(refs.mrs, mr_root, mr_leaf)), + ])), + ), + ]); + if let Some(id) = subject { + if !ticket_leaf(id) { + return Err(failure("invalid subject identity")); + } + fields.insert( + "entry".into(), + WipValue::String(format!("{ticket_root}/{id}")), + ); + } + Ok(WipValue::Record(fields)) +} +pub(super) fn project( + operation: &str, + data: Value, + bound: Option<&str>, + ticket_root: &str, + mr_root: &str, + objective_root: &str, +) -> Result { + match operation { + "query" => { + let items = data + .get("tickets") + .and_then(Value::as_array) + .ok_or_else(|| failure("missing bounded items"))? + .iter() + .map(|data| { + let id = leaf(data, "ticket") + .filter(|id| is_canonical_ticket_resource_key(id)) + .ok_or_else(|| failure("missing canonical subject"))?; + envelope( + "query", + data.clone(), + Some(&id), + ticket_root, + mr_root, + objective_root, + ) + }) + .collect::>()?; + let has_more = data + .get("has_more") + .and_then(Value::as_bool) + .ok_or_else(|| failure("missing paging state"))?; + let mut fields = BTreeMap::from([ + ("items".into(), WipValue::List(items)), + ("has_more".into(), WipValue::Boolean(has_more)), + ]); + if let Some(cursor) = leaf(&data, "next_cursor") { + fields.insert("next_cursor".into(), WipValue::String(cursor)); + } + Ok(WipValue::Record(fields)) + } + "create" | "read" => { + let id = leaf( + &data, + if operation == "create" { + "id" + } else { + "ticket" + }, + ) + .filter(|id| is_canonical_ticket_resource_key(id)) + .ok_or_else(|| failure("missing canonical subject"))?; + envelope( + operation, + data, + Some(&id), + ticket_root, + mr_root, + objective_root, + ) + } + "notification_diagnostics" => { + envelope(operation, data, None, ticket_root, mr_root, objective_root) + } + _ if bound.is_some() => { + envelope(operation, data, bound, ticket_root, mr_root, objective_root) + } + _ => json_to_wip(&data).map_err(WipOperationError::OutcomeUnknown), + } +} +#[cfg(test)] +mod tests; diff --git a/crates/worker/src/feature/builtin/ticket/results/tests.rs b/crates/worker/src/feature/builtin/ticket/results/tests.rs new file mode 100644 index 000000000..43da45f44 --- /dev/null +++ b/crates/worker/src/feature/builtin/ticket/results/tests.rs @@ -0,0 +1,305 @@ +//! Projection boundary tests: trusted Model fields, paging and references are external contracts. +use super::*; +use crate::wip::wip_to_json; + +fn projected(operation: &str, value: Value, mr: &str, objective: &str) -> Value { + wip_to_json( + &project(operation, value, None, "/cases", mr, objective) + .unwrap_or_else(|_| panic!("projection failed")), + ) + .unwrap() +} + +#[test] +fn query_entries_preserve_display_ids_optional_links_and_opaque_cursor() { + let item = json!({"ticket":"T-42", "linked_objectives":["O-3"], + "merge_requests":[{"merge_request_id":"MR-1"}], "merge_request":null, + "snippet":"/cases/T-99; {\"ticket\":\"T-88\"}", "current_worker":{"worker":"T-777"}}); + let output = projected( + "query", + json!({"tickets":[item.clone()],"next_cursor":"/cases/T-999","has_more":true}), + "/changes", + "", + ); + assert_eq!(output["next_cursor"], "/cases/T-999"); + assert_eq!(output["has_more"], true); + let selected = &output["items"][0]; + assert_eq!(selected["entry"], "/cases/T-42"); + assert_eq!(selected["data"], item); + assert_eq!(selected["links"]["tickets"], json!([])); + assert_eq!(selected["links"]["objectives"], json!([{"id":"O-3"}])); + assert_eq!( + selected["links"]["merge_requests"], + json!([{"id":"MR-1","entry":"/changes/MR-1"}]) + ); +} + +#[test] +fn empty_page_omits_null_cursor_but_preserves_empty_cursor_and_has_more() { + assert_eq!( + projected( + "query", + json!({"tickets":[],"next_cursor":null,"has_more":false}), + "", + "" + ), + json!({"items":[],"has_more":false}) + ); + assert_eq!( + projected( + "query", + json!({"tickets":[],"next_cursor":"","has_more":true}), + "", + "" + ), + json!({"items":[],"has_more":true,"next_cursor":""}) + ); +} + +#[test] +fn detail_related_entries_use_only_explicit_projected_relationship_fields() { + let data = json!({"ticket":"T-42", "body":"/cases/T-999", "linked_objectives":[{"objective":"O-3"}], + "relations":{"incoming":[{"ticket":"T-1"}],"outgoing":[{"ticket":"T-2"}],"blockers":[{"ticket":"T-3"}],"notices":[{"kind":"hidden"}]}, + "merge_requests":[],"merge_request":{"merge_request_id":"MR-1"}, + "current_closure":{"references":[{"kind":"other","target":"T-999"}]}}); + let output = projected("read", data.clone(), "/changes", "/goals"); + assert_eq!(output["data"], data); + assert_eq!( + output["links"]["tickets"], + json!([ + {"id":"T-1","entry":"/cases/T-1"},{"id":"T-2","entry":"/cases/T-2"},{"id":"T-3","entry":"/cases/T-3"} + ]) + ); + assert_eq!( + output["links"]["objectives"], + json!([{"id":"O-3","entry":"/goals/O-3"}]) + ); + assert_eq!( + output["links"]["merge_requests"], + json!([{"id":"MR-1","entry":"/changes/MR-1"}]) + ); +} + +#[test] +fn creation_receipt_is_not_reinterpreted_as_a_complete_item_observation() { + let output = projected("create", json!({"id":"T-42","state":"open"}), "", ""); + assert_eq!(output["entry"], "/cases/T-42"); + assert_eq!(output["data"], json!({"id":"T-42","state":"open"})); + assert_eq!( + output["links"], + json!({"tickets":[],"objectives":[],"merge_requests":[]}) + ); + assert!( + project( + "create", + json!({"id":"/cases/T-42"}), + None, + "/cases", + "", + "" + ) + .is_err() + ); +} + +#[test] +fn declared_ticket_results_validate_navigation_and_do_not_type_domain_strings_as_entries() { + let descriptor = InterfaceDescriptor { + format: INTERFACE_FORMAT_V1.into(), + documentation: None, + types: vec![], + operations: ["query", "create", "read"] + .into_iter() + .map(|op| OperationDeclaration { + name: op.into(), + documentation: None, + parameters: vec![], + returns: declaration(op, op == "read").unwrap(), + }) + .collect(), + }; + descriptor.validate().unwrap(); + for (operation, data) in [ + ("create", json!({"id":"T-42","state":"open"})), + ( + "read", + json!({"ticket":"T-42","linked_objectives":["O-3"],"body":"not-an-entry","event_page":{"next_cursor":"/cases/T-99","has_more":true}}), + ), + ( + "query", + json!({"tickets":[{"ticket":"T-42"}],"next_cursor":"/cases/T-99","has_more":true}), + ), + ] { + let output = projected(operation, data, "/changes", "/goals"); + descriptor + .validate_result(operation, &json_to_wip(&output).unwrap()) + .unwrap(); + let mut invalid = output.clone(); + if operation == "query" { + invalid["items"][0]["entry"] = json!(42); + } else { + invalid["entry"] = json!("not-an-absolute-coordinate"); + } + assert!( + descriptor + .validate_result(operation, &json_to_wip(&invalid).unwrap()) + .is_err() + ); + } +} + +#[test] +fn lookup_and_record_results_publish_only_their_explicit_domain_reference_fields() { + let cases = [ + ( + "implementation_result_query", + json!({"ticket_id":"T-42","request":{"merge_request":{"merge_request_id":"MR-1","subject_ref":"sha"},"body":"T-999"}}), + vec!["T-42"], + vec!["MR-1"], + ), + ( + "implementation_result", + json!({"ticket_id":"T-42","request":{"merge_request":null}}), + vec!["T-42"], + vec![], + ), + ( + "assignment_tracking_query", + json!([{"ticket_id":"T-42","tracking":{"result":"T-999","progress":"/cases/T-999"}}]), + vec!["T-42"], + vec![], + ), + ( + "assignment_tracking_update", + json!({"ticket_id":"T-42","tracking":{"result":"T-999"}}), + vec!["T-42"], + vec![], + ), + ( + "assignments_query", + json!({"items":[{"ticket_id":"T-42"}],"next_assignment_id":"T-999"}), + vec!["T-42"], + vec![], + ), + ( + "query_orchestration_plans", + json!({"records":[{"ticket_id":"T-42","related_ticket":"T-7","accepted_plan":{"branch":"T-999","worktree":"/cases/T-999"}}],"truncated":true,"count":2,"returned":1}), + vec!["T-42", "T-7"], + vec![], + ), + ( + "record_orchestration_plan", + json!({"ticket_id":"T-42","related_ticket":"T-7","note":"T-999"}), + vec!["T-42", "T-7"], + vec![], + ), + ( + "dependency_check", + json!({"ticket":{"id":"internal42","resource_key":"T-42"},"blockers":[{"blocking_ticket":"T-7"}],"queue_tickets":["T-8"],"recommended_action":"wait_for_orchestrator"}), + vec!["T-42", "T-7", "T-8"], + vec![], + ), + ( + "record_relation", + json!({"ticket_id":"T-42","target":"T-7"}), + vec!["T-42", "T-7"], + vec![], + ), + ( + "remove_relation", + json!({"ticket_id":"T-42","target":"T-7"}), + vec!["T-42", "T-7"], + vec![], + ), + ( + "edit", + json!({"meta":{"id":"T-42"},"relations":{"outgoing":[{"ticket_id":"T-42","target":"T-7"}],"incoming":[{"source_ticket":"T-8"}],"blockers":[{"blocking_ticket":"T-9"}],"notices":[{"related_ticket":"T-10"}]}}), + vec!["T-10", "T-42", "T-7", "T-8", "T-9"], + vec![], + ), + ( + "case_update", + json!({"meta":{"id":"T-42"},"relations":{"outgoing":[{"ticket_id":"T-42","target":"T-7"}]},"current_closure":{"references":[{"kind":"other","target":"T-999"}]}}), + vec!["T-42", "T-7"], + vec![], + ), + ( + "notification_diagnostics", + json!({"items":[{"ticket":"T-42","merge_request_id":"MR-1","job_id":"T-999"},{"ticket":null,"merge_request_id":null}],"next_cursor":"/cases/T-999","source_checks":[],"limit":2}), + vec!["T-42"], + vec!["MR-1"], + ), + ]; + for (operation, data, tickets, mrs) in cases { + let notification = operation == "notification_diagnostics"; + let output = project( + operation, + data.clone(), + (!notification).then_some("T-42"), + "/cases", + "/changes", + "/goals", + ) + .unwrap_or_else(|_| panic!("{operation}")); + let descriptor = InterfaceDescriptor { + format: INTERFACE_FORMAT_V1.into(), + documentation: None, + types: vec![], + operations: vec![OperationDeclaration { + name: operation.into(), + documentation: None, + parameters: vec![], + returns: declaration(operation, !notification).unwrap(), + }], + }; + descriptor.validate_result(operation, &output).unwrap(); + let output = wip_to_json(&output).unwrap(); + assert_eq!(output["data"], data, "{operation}"); + assert_eq!( + output.get("entry"), + (!notification).then_some(&json!("/cases/T-42")), + "{operation}" + ); + let ids = |domain: &str| { + output["links"][domain] + .as_array() + .unwrap() + .iter() + .map(|link| link["id"].as_str().unwrap()) + .collect::>() + }; + assert_eq!(ids("tickets"), tickets, "{operation}"); + assert_eq!(ids("merge_requests"), mrs, "{operation}"); + } +} + +#[test] +fn assignment_event_and_tag_identifiers_do_not_create_child_objects_or_text_links() { + for operation in [ + "assignment_tracking_history", + "assignment_answer", + "assignment_diagnostics", + "tags_set", + "comment", + ] { + let data = json!({"ticket":"T-42","assignment_id":"T-99","result_id":"T-98","event_ref":"/cases/T-97","items":[{"ticket":"T-96","source_context":{"merge_request_id":"MR-95"}}]}); + let output = wip_to_json( + &project( + operation, + data.clone(), + Some("T-42"), + "/cases", + "/changes", + "/goals", + ) + .unwrap_or_else(|_| panic!("{operation}")), + ) + .unwrap(); + assert_eq!(output["data"], data); + assert_eq!(output["entry"], "/cases/T-42"); + assert_eq!( + output["links"], + json!({"tickets":[],"objectives":[],"merge_requests":[]}) + ); + } +} diff --git a/crates/worker/src/feature/builtin/ticket/tags/tests.rs b/crates/worker/src/feature/builtin/ticket/tags/tests.rs index 31cdadd1a..527b2aa36 100644 --- a/crates/worker/src/feature/builtin/ticket/tags/tests.rs +++ b/crates/worker/src/feature/builtin/ticket/tags/tests.rs @@ -335,6 +335,7 @@ async fn classification_wip_uses_normal_permissions_and_route_bound_ticket_ident &permissions, input, WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "worker".into(), }, @@ -380,6 +381,7 @@ async fn classification_wip_uses_normal_permissions_and_route_bound_ticket_ident "tags_set", &args, WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "worker".into(), }, @@ -408,6 +410,7 @@ async fn classification_wip_uses_normal_permissions_and_route_bound_ticket_ident "tags_set", &args, WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "worker".into() } diff --git a/crates/worker/src/feature/builtin/workspace_config_tests.rs b/crates/worker/src/feature/builtin/workspace_config_tests.rs index 1f5a1763f..e33f6c2b8 100644 --- a/crates/worker/src/feature/builtin/workspace_config_tests.rs +++ b/crates/worker/src/feature/builtin/workspace_config_tests.rs @@ -133,6 +133,7 @@ fn invocation(key: &str, access: Option<&str>) -> FeatureInvocation { } fn context() -> WipCallContext { WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "test-worker".into(), } diff --git a/crates/worker/src/wip.rs b/crates/worker/src/wip.rs index f1e989446..1d00fa0ab 100644 --- a/crates/worker/src/wip.rs +++ b/crates/worker/src/wip.rs @@ -156,6 +156,9 @@ pub enum WipMountError { pub struct WipCallContext { pub execution: ToolExecutionContext, pub security_context: String, + /// Host-supplied completed composition, overwritten at dispatch. Direct handler calls default + /// to no cross-domain publication; caller-provided coordinates cannot publish an Object. + pub result_namespaces: WipResultNamespaces, } /// Successful operation value. Native attachments and compatibility ToolOutputs @@ -313,6 +316,7 @@ pub trait WipDynamicItemResolver: Send + Sync { } } +#[derive(Clone)] pub struct WipDynamicMount { pub collection_route: String, pub capability: String, @@ -378,6 +382,7 @@ pub trait WipSubtreeProvider: Send + Sync { async fn children(&self, path: &str) -> Result, ProtocolError>; } +#[derive(Clone)] pub struct WipSubtreeMount { pub root: String, pub provider: Arc, @@ -395,6 +400,7 @@ pub struct WipDynamicOperationContribution { pub resolver: Arc, } +#[derive(Clone)] struct MountedDynamicOperationContribution { contributor: String, operations: Vec, @@ -412,6 +418,7 @@ impl WipNamespaceRoute { } } +#[derive(Clone)] struct MountedProjection { projection: WipProjection, operation_handlers: BTreeMap)>, @@ -497,7 +504,23 @@ impl WipOperationHandler for OperationDispatchHandler { } } -#[derive(Default)] +/// Optional result-navigation coordinates from one completed native mount composition. +/// +/// The Host builds a fresh immutable snapshot from the final registry, not a namespace reservation +/// or an intermediate factory state. Registry checkpoint clones own their registration markers by +/// value; no handler captures a shared mutable publication ledger. Rebuilding a Host cannot freeze +/// another registry's context. These coordinates carry no observations, existence or permissions; +/// current target visibility and authorization remain the provider's read/dispatch responsibility. +#[derive(Debug, Clone, Default)] +pub struct WipResultNamespaces(Arc>); + +impl WipResultNamespaces { + pub fn root(&self, owner: &str) -> Option<&str> { + self.0.get(owner).map(String::as_str) + } +} + +#[derive(Clone, Default)] pub struct WipMountRegistry { mounts: BTreeMap, additional_interfaces: BTreeMap>, @@ -505,6 +528,7 @@ pub struct WipMountRegistry { subtree_mounts: Vec, dynamic_operation_contributions: BTreeMap>, namespaces: BTreeMap, + result_namespace_routes: BTreeSet, replaced_compatibility_capabilities: BTreeMap, } @@ -577,6 +601,50 @@ impl WipMountRegistry { roots.next().is_none().then_some(root.as_str()) } + /// Complete a native collection/item provider's result-reference registration only after + /// all of its installation steps succeed. This marker is registry-owned checkpoint data, + /// not authority, and is rechecked against final mounts when a Host is built. + pub(crate) fn publish_result_namespace( + &mut self, + namespace: &WipNamespaceRoute, + ) -> Result<(), WipMountError> { + let root = namespace.root(); + if !self.result_namespace_is_mounted(root) { + return Err(WipMountError::InvalidProjection { + route: root.into(), + message: + "result references require a visible native collection and dynamic item mount" + .into(), + }); + } + self.result_namespace_routes.insert(root.into()); + Ok(()) + } + + fn result_namespace_is_mounted(&self, root: &str) -> bool { + self.mounts.get(root).is_some_and(|mount| { + mount.projection.kind == WipProjectionKind::Native + && mount.projection.handler.is_visible() + }) && self + .dynamic_mounts + .iter() + .any(|mount| mount.collection_route == root) + } + + fn result_namespace_snapshot(&self) -> WipResultNamespaces { + let roots = self + .result_namespace_routes + .iter() + .filter_map(|root| { + let owner = self.namespaces.get(root)?; + (self.namespace_root(owner) == Some(root.as_str()) + && self.result_namespace_is_mounted(root)) + .then(|| (owner.clone(), root.clone())) + }) + .collect(); + WipResultNamespaces(Arc::new(roots)) + } + pub fn mount( &mut self, projection: WipProjection, @@ -1250,6 +1318,7 @@ fn validate_projection(projection: &WipProjection) -> Result<(), WipMountError> struct WipHost { registry: WipMountRegistry, generation: Vec, + result_namespaces: WipResultNamespaces, } impl WipHost { @@ -1259,9 +1328,11 @@ impl WipHost { digest.update(route.as_bytes()); digest.update([0]); } + let result_namespaces = registry.result_namespace_snapshot(); Self { registry, generation: digest.finalize().to_vec(), + result_namespaces, } } @@ -1985,6 +2056,10 @@ impl WipHost { error.to_string(), )) })?; + let mut context = context; + // Publication is Host-owned. Never trust a caller's fabricated result coordinates or + // reuse a snapshot captured by a handler before composition/rollback finished. + context.result_namespaces = self.result_namespaces.clone(); projection .handler .call(&request.operation, &request.arguments, context) @@ -2584,6 +2659,7 @@ impl WipRuntime { } }; let context = WipCallContext { + result_namespaces: Default::default(), execution, security_context: self.security_context.as_str().to_string(), }; @@ -3584,6 +3660,7 @@ mod tests { arguments: BTreeMap::new(), }; let context = || WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "worker-a".into(), }; @@ -3702,6 +3779,7 @@ mod tests { fn direct_wip_context() -> WipCallContext { WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "worker-a".into(), } @@ -5775,6 +5853,7 @@ mod tests { .call( request.clone(), WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "worker-a".into(), }, @@ -5794,6 +5873,7 @@ mod tests { .call( request.clone(), WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "worker-a".into(), }, @@ -5814,6 +5894,7 @@ mod tests { .call( request.clone(), WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "worker-a".into(), }, @@ -5833,6 +5914,7 @@ mod tests { .call( request, WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "worker-a".into(), }, @@ -5980,6 +6062,7 @@ mod tests { )]), }, WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "worker-a".into(), }, @@ -6108,6 +6191,7 @@ mod tests { arguments: BTreeMap::new(), }, WipCallContext { + result_namespaces: Default::default(), execution: ToolExecutionContext::direct(), security_context: "worker-a".into(), }, diff --git a/crates/worker/src/wip/ticket_native_tests.rs b/crates/worker/src/wip/ticket_native_tests.rs index 6c7cd988e..ace7967de 100644 --- a/crates/worker/src/wip/ticket_native_tests.rs +++ b/crates/worker/src/wip/ticket_native_tests.rs @@ -63,6 +63,9 @@ async fn ticket_inspect_contract_drives_query_read_and_comment_without_tool_sche let signature = declaration["interface_signature"].as_str().unwrap(); assert!(signature.contains("query(")); assert!(signature.contains("filter?")); + assert!(signature.contains("`entry`: entry"), "{signature}"); + assert!(signature.contains("data: json")); + assert!(signature.contains("next_cursor?: string")); assert!(signature.contains("states?")); assert!(signature.contains("open")); assert!(!signature.contains("Tool schema")); @@ -81,7 +84,7 @@ async fn ticket_inspect_contract_drives_query_read_and_comment_without_tool_sche .await .unwrap(); let output: Json = serde_json::from_str(&output.content.unwrap()).unwrap(); - let path = output["tickets"][0]["path"].as_str().unwrap(); + let path = output["items"][0]["entry"].as_str().unwrap(); assert_eq!(path, "/cases/T-42"); let object = inspect(&runtime, path).await; let interface = &object["interfaces"][0]; @@ -422,13 +425,14 @@ async fn native_plan_kind_reaches_persisted_query_through_inspect_and_dispatch() .await .unwrap(); let result: Json = serde_json::from_str(&result.content.unwrap()).unwrap(); - assert_eq!(result["count"], expected); + assert_eq!(result["data"]["count"], expected); + assert_eq!(result["entry"], path); assert_eq!( - result["records"].as_array().unwrap().len(), + result["data"]["records"].as_array().unwrap().len(), expected as usize ); if let Some(kind) = kind { - assert_eq!(result["records"][0]["kind"], kind); + assert_eq!(result["data"]["records"][0]["kind"], kind); } assert_eq!( client.requests.lock().unwrap().last().unwrap()["kind"], @@ -560,3 +564,608 @@ async fn ticket_inspect_explains_unpublished_objective_and_mr_input_roots() { ); } } + +fn ticket_result_detail(key: &str) -> Json { + json!({ + "resource_key":key,"title":"Intent","body":"/cases/T-999 is untrusted text", "body_truncated":false, + "content_digest":"observed","state":"open","targets":[],"events":[],"linked_objectives":[], + "initial_post":{"sequence":0,"event_ref":"initial","is_initial":true,"kind":"comment","body":"Requirements","body_truncated":false,"body_digest":"a".repeat(64),"editable":true,"author":null,"at":null,"edited_at":null,"edited_by":null}, + "closure_operation_key":null,"current_closure":null,"legacy_handoff":null,"closure_history":[], + "assignment_tracking":[],"tags":[],"assignments":[],"implementation_reports":[],"merge_requests":[] + }) +} + +// The response transport is JSON, but navigation is selected by the declared WIP types, not by +// looking for path-like strings or property names inside opaque domain data or rendered signatures. +fn ticket_result_declared_entries(ty: &TypeExpr, value: &Json, entries: &mut Vec) { + match ty { + TypeExpr::Entry => entries.push(value.as_str().unwrap().into()), + TypeExpr::Record { fields } => { + for field in fields { + if let Some(value) = value.get(&field.name) { + ticket_result_declared_entries(&field.r#type, value, entries); + } + } + } + TypeExpr::List { items } => { + for value in value.as_array().unwrap() { + ticket_result_declared_entries(items, value, entries); + } + } + _ => {} // Json and strings are deliberately not traversed. + } +} + +async fn ticket_result_model_call( + tools: &agen::tool_server::ToolServerHandle, + name: &str, + input: Json, +) -> Json { + transport_output_json( + tools + .call_tool(name, &input.to_string(), ToolExecutionContext::direct()) + .await + .unwrap(), + ) +} +async fn ticket_result_model_inspect( + tools: &agen::tool_server::ToolServerHandle, + entry: &Json, +) -> Json { + ticket_result_model_call(tools, "Inspect", json!({"path":entry})).await +} +async fn ticket_result_model_invoke( + tools: &agen::tool_server::ToolServerHandle, + object: &Json, + operation: &str, + arguments: Json, +) -> Json { + ticket_result_model_call(tools, "Invoke", json!({"path":object["path"],"interface":object["interfaces"][0]["reference"],"operation":operation,"arguments":arguments})).await +} + +#[tokio::test] +async fn ticket_typed_results_drive_model_query_create_read_and_all_related_domain_transitions() { + for order in [[0, 1, 2], [2, 0, 1], [1, 2, 0]] { + let mut detail = ticket_result_detail("T-42"); + detail["linked_objectives"] = + json!([{"resource_key":"O-3","title":"Objective","state":"active"}]); + detail["relations"] = json!({"outgoing":[{"target_resource_key":"T-7","kind":"related","note":null,"at":null}],"incoming":[],"blockers":[],"notices":[]}); + detail["merge_requests"] = json!([{"merge_request_id":"MR-1","repository_key":"main","state":"open","selector_from":"work/source","selector_to":"develop","review_status":"pending","subject_ref":null,"source_ref_observation":{"status":"not_required"}}]); + let query_item = json!({"resource_key":"T-42","title":"Intent","state":"open","closure_operation_key":null,"current_closure":null,"legacy_handoff":null,"assignment_tracking":[],"tags":[],"matched_fields":[],"linked_objective_keys":["O-3"],"relation_count":1,"blocker_count":0,"unresolved_blocker_count":0,"unresolved_review_count":0,"merge_requests":detail["merge_requests"], "snippet":"/cases/T-999"}); + let mut second_item = query_item.clone(); + second_item["resource_key"] = json!("T-43"); + let response = |value: Json| crate::worker::WorkspaceResponse { + status: 200, + body: value.to_string(), + }; + let client = Arc::new(ScriptedWorkspaceClient::new([ + response(json!({"items":[query_item, second_item],"page":{"next_cursor":"opaque-next","has_more":true}})), + response(json!({"items":[],"page":{"next_cursor":null,"has_more":false}})), + response(detail), + response(ticket_result_detail("T-7")), + objective_detail_response("objective-observed"), + response(json!({"workspace_id":"workspace","merge_request_id":"MR-1","repository_key":"main","state":"open","selector_from":"work/source","selector_to":"develop","ticket_ids":[],"thread":[]})), + response(json!({"id":"internal123","resource_key":"T-9","slug":"new","status":"Open"})), + ]).with_authorizations([ + expected_authorization("ticket.query","workspace",None), + expected_authorization("ticket.query","workspace",None), + expected_authorization("ticket.read","ticket",Some(("ticket_id","T-42"))), + expected_authorization("ticket.read","ticket",Some(("ticket_id","T-7"))), + expected_authorization("objective.read","objective",Some(("objective_id","O-3"))), + expected_authorization("merge_request.read","merge_request",Some(("merge_request_id","MR-1"))), + expected_authorization("ticket.create","workspace",None), + ])); + let mut registry = WipMountRegistry::new(); + let tickets = registry.allocate_namespace("ticket", "cases").unwrap(); + let objectives = registry.allocate_namespace("objective", "goals").unwrap(); + let mrs = registry + .allocate_namespace("merge-request", "changes") + .unwrap(); + for domain in order { + match domain { + 0 => crate::feature::builtin::ticket::mount_workspace_http_ticket_wip( + &mut registry, + client.clone(), + crate::feature::builtin::ticket::TicketFeatureAccess::workspace_authoring(), + None, + &tickets, + ) + .unwrap(), + 1 => crate::feature::builtin::objective::mount_workspace_http_objective_wip( + &mut registry, + client.clone(), + None, + &objectives, + ) + .unwrap(), + 2 => { + crate::feature::builtin::merge_request::mount_workspace_http_merge_request_wip( + &mut registry, + client.clone(), + manifest::MergeRequestFeatureConfig { + show: true, + ..Default::default() + }, + None, + &mrs, + ) + .unwrap() + } + _ => unreachable!(), + } + } + let mut engine = Engine::<_, Mutable, ()>::new_annotated(DummyClient); + let runtime = install_wip_mode_with_mounts( + &mut engine, + None, + "ticket-model-results".into(), + registry, + ) + .unwrap(); + let tools = engine.tool_server_handle(); + tools.flush_pending(); + let collection = ticket_result_model_inspect(&tools, &json!("/cases")).await; + let interface = + ticket_result_model_inspect(&tools, &collection["interfaces"][0]["path"]).await; + let signature = interface["interface_signature"].as_str().unwrap(); + assert!(signature.contains("`entry`: entry"), "{signature}"); + assert!(signature.contains("`entry`?: entry")); + assert!(signature.contains("data: json")); + let query = + ticket_result_model_invoke(&tools, &collection, "query", json!({"page":{"limit":2}})) + .await; + assert_eq!(query["next_cursor"], "opaque-next"); + assert_eq!(query["has_more"], true); + assert_eq!( + client.requests().len(), + 1, + "query does not prefetch any returned entry" + ); + let declaration = runtime + .host + .projection("/cases") + .unwrap() + .descriptor + .operations + .into_iter() + .find(|op| op.name == "query") + .unwrap(); + let mut paths = vec![]; + ticket_result_declared_entries(&declaration.returns.r#type, &query, &mut paths); + assert!(paths.contains(&"/cases/T-42".into())); + assert!(!paths.contains(&"/cases/T-999".into())); + let page = ticket_result_model_invoke( + &tools, + &collection, + "query", + json!({"page":{"cursor":query["next_cursor"],"limit":2}}), + ) + .await; + assert_eq!(page, json!({"items":[],"has_more":false})); + let selected = ticket_result_model_inspect(&tools, &query["items"][0]["entry"]).await; + assert_eq!( + client.requests().len(), + 2, + "Inspect resolves only route metadata, not related reads" + ); + let read = + ticket_result_model_invoke(&tools, &selected, "read", json!({"page":{"limit":1}})) + .await; + assert_eq!(read["entry"], query["items"][0]["entry"]); + for domain in ["tickets", "objectives", "merge_requests"] { + let related = + ticket_result_model_inspect(&tools, &read["links"][domain][0]["entry"]).await; + let interface = + ticket_result_model_inspect(&tools, &related["interfaces"][0]["path"]).await; + assert!( + interface["interface_signature"] + .as_str() + .unwrap() + .contains("read(") + ); + ticket_result_model_invoke(&tools, &related, "read", json!({})).await; + } + let created = ticket_result_model_invoke( + &tools, + &collection, + "create", + json!({"content":{"title":"New"}}), + ) + .await; + let created_object = ticket_result_model_inspect(&tools, &created["entry"]).await; + assert_eq!(created_object["path"], created["entry"]); + assert_eq!(client.requests().len(), 7); + client.assert_authorizations_consumed(); + let tree = ticket_result_model_call(&tools, "Tree", json!({"path":"/","depth":3})).await; + assert!(!tree.to_string().contains("/cases/T-42")); + assert!(!tree.to_string().contains("/goals/O-3")); + assert!(!tree.to_string().contains("/changes/MR-1")); + assert!(!tree.to_string().contains("/cases/T-43")); + assert_eq!(client.requests().len(), 7); + } +} + +#[tokio::test] +async fn ticket_successful_create_is_never_replayed_after_selected_deleted_or_denied_read() { + let client = Arc::new( + ScriptedWorkspaceClient::new([ + crate::worker::WorkspaceResponse { + status: 200, + body: json!({"id":"internal123","resource_key":"T-9","slug":"new","status":"Open"}) + .to_string(), + }, + crate::worker::WorkspaceResponse { + status: 404, + body: json!({"error":"not_found","message":"deleted"}).to_string(), + }, + crate::worker::WorkspaceResponse { + status: 403, + body: json!({"error":"permission_denied","message":"denied"}).to_string(), + }, + ]) + .with_authorizations([ + expected_authorization("ticket.create", "workspace", None), + expected_authorization("ticket.read", "ticket", Some(("ticket_id", "T-9"))), + expected_authorization("ticket.read", "ticket", Some(("ticket_id", "T-9"))), + ]), + ); + let mut registry = WipMountRegistry::new(); + let route = registry.allocate_namespace("ticket", "cases").unwrap(); + crate::feature::builtin::ticket::mount_workspace_http_ticket_wip( + &mut registry, + client.clone(), + crate::feature::builtin::ticket::TicketFeatureAccess::workspace_authoring(), + None, + &route, + ) + .unwrap(); + let mut engine = Engine::<_, Mutable, ()>::new_annotated(DummyClient); + install_wip_mode_with_mounts( + &mut engine, + None, + "ticket-negative-results".into(), + registry, + ) + .unwrap(); + let tools = engine.tool_server_handle(); + tools.flush_pending(); + let collection = ticket_result_model_inspect(&tools, &json!("/cases")).await; + let created = ticket_result_model_invoke( + &tools, + &collection, + "create", + json!({"content":{"title":"Created once"}}), + ) + .await; + assert_eq!(client.requests().len(), 1); + let object = ticket_result_model_inspect(&tools, &created["entry"]).await; + for expected in ["NotFound", "PermissionDenied"] { + let error=tools.call_tool("Invoke",&json!({"path":created["entry"],"interface":object["interfaces"][0]["reference"],"operation":"read","arguments":{}}).to_string(),ToolExecutionContext::direct()).await.unwrap_err(); + assert!(error.to_string().contains(expected), "{error}"); + } + let requests = client.requests(); + assert_eq!(requests.len(), 3); + assert_eq!( + requests + .iter() + .filter(|request| request.path == "/api/w/workspace/tickets") + .count(), + 1 + ); + client.assert_authorizations_consumed(); +} + +#[test] +fn result_namespaces_follow_committed_registry_rollback_and_independent_host_builds() { + let client = Arc::new(ScriptedWorkspaceClient::new([])); + let mut registry = WipMountRegistry::new(); + let ticket = registry.allocate_namespace("ticket", "cases").unwrap(); + let objective = registry.allocate_namespace("objective", "goals").unwrap(); + let checkpoint = registry.clone(); + assert!(registry.publish_result_namespace(&ticket).is_err()); + assert!( + registry + .result_namespace_snapshot() + .root("ticket") + .is_none() + ); + crate::feature::builtin::ticket::mount_workspace_http_ticket_wip( + &mut registry, + client.clone(), + crate::feature::builtin::ticket::TicketFeatureAccess::read_only(), + None, + &ticket, + ) + .unwrap(); + let ticket_host = WipHost::new(registry.clone()); + assert_eq!(ticket_host.result_namespaces.root("ticket"), Some("/cases")); + assert_eq!(ticket_host.result_namespaces.root("objective"), None); + crate::feature::builtin::objective::mount_workspace_http_objective_wip( + &mut registry, + client, + None, + &objective, + ) + .unwrap(); + let both = WipHost::new(registry.clone()); + assert_eq!(both.result_namespaces.root("objective"), Some("/goals")); + assert_eq!( + ticket_host.result_namespaces.root("objective"), + None, + "building another Host cannot mutate or freeze a prior snapshot" + ); + let rolled_back = WipHost::new(checkpoint); + assert_eq!(rolled_back.result_namespaces.root("ticket"), None); + assert_eq!(rolled_back.result_namespaces.root("objective"), None); + assert_eq!(both.result_namespaces.root("ticket"), Some("/cases")); + registry + .dynamic_mounts + .retain(|mount| mount.collection_route != "/goals"); + let partial = WipHost::new(registry.clone()); + assert_eq!( + partial.result_namespaces.root("objective"), + None, + "a marker alone cannot publish a missing/rolled-back item mount" + ); + registry + .allocate_namespace("ticket", "ambiguous-cases") + .unwrap(); + assert_eq!( + WipHost::new(registry).result_namespaces.root("ticket"), + None + ); +} + +#[tokio::test] +async fn host_overwrites_forged_result_context_and_omits_unpublished_cross_domain_entries() { + let mut detail = ticket_result_detail("T-42"); + detail["linked_objectives"] = + json!([{"resource_key":"O-3","title":"Visible ID only","state":"active"}]); + let client = Arc::new( + ScriptedWorkspaceClient::new([crate::worker::WorkspaceResponse { + status: 200, + body: detail.to_string(), + }]) + .with_authorizations([expected_authorization( + "ticket.read", + "ticket", + Some(("ticket_id", "T-42")), + )]), + ); + let mut registry = WipMountRegistry::new(); + let route = registry.allocate_namespace("ticket", "cases").unwrap(); + registry + .allocate_namespace("objective", "reserved-goals") + .unwrap(); + crate::feature::builtin::ticket::mount_workspace_http_ticket_wip( + &mut registry, + client.clone(), + crate::feature::builtin::ticket::TicketFeatureAccess::read_only(), + None, + &route, + ) + .unwrap(); + let host = WipHost::new(registry); + let projection = host.projection("/cases/T-42").unwrap(); + let output = host + .call( + CallOperationRequest { + target: wip_protocol::Target { + path: "/cases/T-42".into(), + validator: projection.object.validator, + }, + interface: wip_protocol::InterfaceTarget { + reference: projection.interface, + scope_ref: None, + validator: projection.interface_validator, + }, + operation: "read".into(), + arguments: BTreeMap::new(), + }, + WipCallContext { + execution: ToolExecutionContext::direct(), + security_context: "forged".into(), + result_namespaces: WipResultNamespaces(Arc::new(BTreeMap::from([( + "objective".into(), + "/fake-goals".into(), + )]))), + }, + ) + .await + .unwrap_or_else(|_| panic!("read")); + let output = wip_to_json(&output.value).unwrap(); + assert_eq!(output["links"]["objectives"], json!([{"id":"O-3"}])); + assert_eq!(client.requests().len(), 1); + client.assert_authorizations_consumed(); +} + +struct HiddenResultNamespace; +#[async_trait] +impl WipOperationHandler for HiddenResultNamespace { + fn is_visible(&self) -> bool { + false + } + async fn call( + &self, + _: &str, + _: &BTreeMap, + _: WipCallContext, + ) -> Result { + unreachable!("hidden fixture must never dispatch") + } +} + +#[test] +fn result_namespace_registration_rejects_failed_partial_mounts_and_final_hidden_collections() { + let client = Arc::new(ScriptedWorkspaceClient::new([])); + let mut registry = WipMountRegistry::new(); + let route = registry.allocate_namespace("ticket", "cases").unwrap(); + let reference = root_reference("yoi.ticket/item/v2"); + registry + .mount(WipProjection { + route: "/tools/conflicting-item-interface".into(), + capability: "fixture:conflict".into(), + kind: WipProjectionKind::Native, + object: Object { + name: "conflicting-item-interface".into(), + description: None, + interfaces: vec![reference.clone()], + r#ref: None, + validator: None, + }, + interface: reference, + interface_validator: None, + descriptor: InterfaceDescriptor { + format: INTERFACE_FORMAT_V1.into(), + documentation: None, + types: vec![], + operations: vec![], + }, + handler: Arc::new(HiddenResultNamespace), + }) + .unwrap(); + let result = crate::feature::builtin::ticket::mount_workspace_http_ticket_wip( + &mut registry, + client.clone(), + crate::feature::builtin::ticket::TicketFeatureAccess::read_only(), + None, + &route, + ); + assert!(matches!( + result, + Err(WipMountError::InterfaceCollision { .. }) + )); + assert!( + registry.mounts.contains_key("/cases"), + "the failure happened after collection staging" + ); + assert_eq!( + WipHost::new(registry).result_namespaces.root("ticket"), + None + ); + let mut registry = WipMountRegistry::new(); + let route = registry.allocate_namespace("ticket", "cases").unwrap(); + crate::feature::builtin::ticket::mount_workspace_http_ticket_wip( + &mut registry, + client, + crate::feature::builtin::ticket::TicketFeatureAccess::read_only(), + None, + &route, + ) + .unwrap(); + registry + .mounts + .get_mut("/cases") + .unwrap() + .projection + .handler = Arc::new(HiddenResultNamespace); + assert_eq!( + WipHost::new(registry).result_namespaces.root("ticket"), + None, + "final visibility overrides a stale success marker" + ); +} + +#[tokio::test] +async fn retained_ticket_result_mr_reference_is_model_navigation_not_body_or_snapshot_authority() { + let receipt = json!({"result_id":"saved","ticket_id":"T-42","created_at":"now", + "request":{"operation_key":"saved-operation","kind":"result","body":"/changes/MR-999 is prose", + "merge_request":{"merge_request_id":"MR-1","subject_ref":"historical-source"}}}); + let client = Arc::new( + ScriptedWorkspaceClient::new([ + crate::worker::WorkspaceResponse { + status: 200, + body: receipt.to_string(), + }, + crate::worker::WorkspaceResponse { + status: 200, + body: json!({"merge_request_id":"MR-1","ticket_ids":[],"thread":[]}).to_string(), + }, + ]) + .with_authorizations([ + expected_authorization( + "ticket.implementation_result.read", + "ticket", + Some(("ticket_id", "T-42")), + ), + expected_authorization( + "merge_request.read", + "merge_request", + Some(("merge_request_id", "MR-1")), + ), + ]), + ); + let mut registry = WipMountRegistry::new(); + let tickets = registry.allocate_namespace("ticket", "cases").unwrap(); + let changes = registry + .allocate_namespace("merge-request", "changes") + .unwrap(); + crate::feature::builtin::ticket::mount_workspace_http_ticket_wip( + &mut registry, + client.clone(), + crate::feature::builtin::ticket::TicketFeatureAccess::read_only(), + None, + &tickets, + ) + .unwrap(); + crate::feature::builtin::merge_request::mount_workspace_http_merge_request_wip( + &mut registry, + client.clone(), + manifest::MergeRequestFeatureConfig { + show: true, + ..Default::default() + }, + None, + &changes, + ) + .unwrap(); + let mut engine = Engine::<_, Mutable, ()>::new_annotated(DummyClient); + let runtime = install_wip_mode_with_mounts( + &mut engine, + None, + "retained-result-entries".into(), + registry, + ) + .unwrap(); + let tools = engine.tool_server_handle(); + tools.flush_pending(); + let selected = ticket_result_model_inspect(&tools, &json!("/cases/T-42")).await; + let result = ticket_result_model_invoke( + &tools, + &selected, + "implementation_result_query", + json!({"result_id":"saved"}), + ) + .await; + assert_eq!(result["data"], receipt); + assert_eq!( + client.requests().len(), + 1, + "saved references do not prefetch current state" + ); + let declaration = runtime + .host + .projection("/cases/T-42") + .unwrap() + .descriptor + .operations + .into_iter() + .find(|op| op.name == "implementation_result_query") + .unwrap(); + let mut entries = vec![]; + ticket_result_declared_entries(&declaration.returns.r#type, &result, &mut entries); + assert!(entries.contains(&"/changes/MR-1".into())); + assert!(!entries.contains(&"/changes/MR-999".into())); + let related = + ticket_result_model_inspect(&tools, &result["links"]["merge_requests"][0]["entry"]).await; + let interface = ticket_result_model_inspect(&tools, &related["interfaces"][0]["path"]).await; + assert!( + interface["interface_signature"] + .as_str() + .unwrap() + .contains("read(") + ); + ticket_result_model_invoke(&tools, &related, "read", json!({})).await; + assert_eq!(client.requests().len(), 2); + client.assert_authorizations_consumed(); +}