diff --git a/crates/ticket/src/tool.rs b/crates/ticket/src/tool.rs index ddd18002b..7e383ba7a 100644 --- a/crates/ticket/src/tool.rs +++ b/crates/ticket/src/tool.rs @@ -924,50 +924,57 @@ impl Tool for TicketCreateTool { _ctx: agen::tool::ToolExecutionContext, ) -> Result { let params: TicketCreateParams = parse_input("TicketCreate", input_json)?; - if params - .state - .is_some_and(|state| !matches!(state.into_state(), TicketState::Open)) - { - return Err(backend_error( - "TicketCreate", - TicketError::InvalidWorkflowTransition { - from: "creation".to_owned(), - to: params - .state - .expect("checked non-open state") - .into_state() - .as_str() - .to_owned(), - }, - )); - } - let mut input = NewTicket::new(params.title); - if let Some(body) = params.body { - input.body = MarkdownText::new(body); - } - input.author = None; - input.assignee = params.assignee; - input.readiness = params.readiness; - input.risk_flags = params.risk_flags; - input.workflow_state = params.state.map(TicketStateParam::into_state); - input.queued_by = None; - input.queued_at = params.queued_at; - input.targets = params.targets; - - let created = self - .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(), - }), - )) + execute_ticket_create(&self.backend, params, _ctx) } } +fn execute_ticket_create( + backend: &TicketToolBackend, + params: TicketCreateParams, + _context: agen::tool::ToolExecutionContext, +) -> Result { + if params + .state + .is_some_and(|state| !matches!(state.into_state(), TicketState::Open)) + { + return Err(backend_error( + "TicketCreate", + TicketError::InvalidWorkflowTransition { + from: "creation".to_owned(), + to: params + .state + .expect("checked non-open state") + .into_state() + .as_str() + .to_owned(), + }, + )); + } + let mut input = NewTicket::new(params.title); + if let Some(body) = params.body { + input.body = MarkdownText::new(body); + } + input.author = None; + input.assignee = params.assignee; + input.readiness = params.readiness; + input.risk_flags = params.risk_flags; + input.workflow_state = params.state.map(TicketStateParam::into_state); + input.queued_by = None; + input.queued_at = params.queued_at; + input.targets = params.targets; + + let created = 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(), + }), + )) +} + #[async_trait] impl Tool for TicketEditItemTool { async fn execute( @@ -976,45 +983,52 @@ impl Tool for TicketEditItemTool { _ctx: agen::tool::ToolExecutionContext, ) -> Result { let params: TicketEditItemParams = parse_input("TicketEditItem", input_json)?; - let body_replacement = match (params.old_string, params.new_string) { - (Some(old_string), Some(new_string)) => Some(TicketBodyReplacement { - old_string, - new_string, - replace_all: params.replace_all, - }), - (None, None) => None, - _ => { - return Err(backend_error( - "TicketEditItem", - TicketError::Conflict( - "old_string and new_string must be provided together".to_string(), - ), - )); - } - }; - let edit = crate::TicketItemEdit { - title: params.title, - body: params.body.map(MarkdownText::new), - body_replacement, - targets: params.targets, - author: None, - }; - let ticket = self - .backend - .edit_item(TicketIdOrSlug::from(params.ticket), edit) - .map_err(|error| backend_error("TicketEditItem", error))?; - Ok(json_output( - format!("Edited ticket {}", ticket.meta.id), - ticket_json( - &ticket, - DEFAULT_EVENT_LIMIT, - DEFAULT_ARTIFACT_LIMIT, - 16 * 1024, - ), - )) + execute_ticket_edit_item(&self.backend, params, _ctx) } } +fn execute_ticket_edit_item( + backend: &TicketToolBackend, + params: TicketEditItemParams, + _context: agen::tool::ToolExecutionContext, +) -> Result { + let body_replacement = match (params.old_string, params.new_string) { + (Some(old_string), Some(new_string)) => Some(TicketBodyReplacement { + old_string, + new_string, + replace_all: params.replace_all, + }), + (None, None) => None, + _ => { + return Err(backend_error( + "TicketEditItem", + TicketError::Conflict( + "old_string and new_string must be provided together".to_string(), + ), + )); + } + }; + let edit = crate::TicketItemEdit { + title: params.title, + body: params.body.map(MarkdownText::new), + body_replacement, + targets: params.targets, + author: None, + }; + let ticket = backend + .edit_item(TicketIdOrSlug::from(params.ticket), edit) + .map_err(|error| backend_error("TicketEditItem", error))?; + Ok(json_output( + format!("Edited ticket {}", ticket.meta.id), + ticket_json( + &ticket, + DEFAULT_EVENT_LIMIT, + DEFAULT_ARTIFACT_LIMIT, + 16 * 1024, + ), + )) +} + #[async_trait] impl Tool for TicketEditPostTool { async fn execute( @@ -1023,30 +1037,37 @@ impl Tool for TicketEditPostTool { _ctx: agen::tool::ToolExecutionContext, ) -> Result { let params: TicketEditPostParams = parse_input("TicketEditPost", input_json)?; - let ticket = self - .backend - .edit_post( - TicketIdOrSlug::from(params.ticket), - crate::TicketPostEdit { - event_ref: params.event_ref, - expected_body: MarkdownText::new(params.expected_body), - body: MarkdownText::new(params.body), - author: default_author(), - }, - ) - .map_err(|error| backend_error("TicketEditPost", error))?; - Ok(json_output( - format!("Edited Ticket post in {}", ticket.meta.id), - ticket_json( - &ticket, - DEFAULT_EVENT_LIMIT, - DEFAULT_ARTIFACT_LIMIT, - DEFAULT_BODY_MAX_BYTES, - ), - )) + execute_ticket_edit_post(&self.backend, params, _ctx) } } +fn execute_ticket_edit_post( + backend: &TicketToolBackend, + params: TicketEditPostParams, + _context: agen::tool::ToolExecutionContext, +) -> Result { + let ticket = backend + .edit_post( + TicketIdOrSlug::from(params.ticket), + crate::TicketPostEdit { + event_ref: params.event_ref, + expected_body: MarkdownText::new(params.expected_body), + body: MarkdownText::new(params.body), + author: default_author(), + }, + ) + .map_err(|error| backend_error("TicketEditPost", error))?; + Ok(json_output( + format!("Edited Ticket post in {}", ticket.meta.id), + ticket_json( + &ticket, + DEFAULT_EVENT_LIMIT, + DEFAULT_ARTIFACT_LIMIT, + DEFAULT_BODY_MAX_BYTES, + ), + )) +} + #[async_trait] impl Tool for QueryTicketTool { async fn execute( @@ -1055,39 +1076,46 @@ impl Tool for QueryTicketTool { _ctx: agen::tool::ToolExecutionContext, ) -> Result { let params: QueryTicketParams = parse_input("QueryTicket", input_json)?; - let (filter, state_filter, params_limit) = params - .into_query() - .map_err(|error| backend_error("QueryTicket", error))?; - let limit = bounded(params_limit, DEFAULT_LIST_LIMIT, MAX_LIST_LIMIT); - let tickets = self - .backend - .list(filter) - .map_err(|error| backend_error("QueryTicket", error))?; - let count = tickets.len(); - let returned_tickets: Vec<_> = tickets - .into_iter() - .take(limit) - .map(ticket_summary_json) - .collect(); - let output = QueryTicketOutput { - state_filter: state_filter.to_string(), - count, - returned: returned_tickets.len(), - truncated: count > returned_tickets.len(), - limit, - tickets: returned_tickets, - }; - Ok(json_output( - format!( - "Listed {} ticket(s) for state {state_filter}{}", - output.returned, - if output.truncated { " (truncated)" } else { "" } - ), - output, - )) + execute_query_ticket(&self.backend, params, _ctx) } } +fn execute_query_ticket( + backend: &TicketToolBackend, + params: QueryTicketParams, + _context: agen::tool::ToolExecutionContext, +) -> Result { + let (filter, state_filter, params_limit) = params + .into_query() + .map_err(|error| backend_error("QueryTicket", error))?; + let limit = bounded(params_limit, DEFAULT_LIST_LIMIT, MAX_LIST_LIMIT); + let tickets = backend + .list(filter) + .map_err(|error| backend_error("QueryTicket", error))?; + let count = tickets.len(); + let returned_tickets: Vec<_> = tickets + .into_iter() + .take(limit) + .map(ticket_summary_json) + .collect(); + let output = QueryTicketOutput { + state_filter: state_filter.to_string(), + count, + returned: returned_tickets.len(), + truncated: count > returned_tickets.len(), + limit, + tickets: returned_tickets, + }; + Ok(json_output( + format!( + "Listed {} ticket(s) for state {state_filter}{}", + output.returned, + if output.truncated { " (truncated)" } else { "" } + ), + output, + )) +} + #[async_trait] impl Tool for ShowTicketTool { async fn execute( @@ -1096,41 +1124,47 @@ impl Tool for ShowTicketTool { _ctx: agen::tool::ToolExecutionContext, ) -> Result { let params: ShowTicketParams = parse_input("ShowTicket", input_json)?; - 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( - params.artifact_limit, - DEFAULT_ARTIFACT_LIMIT, - MAX_ARTIFACT_LIMIT, - ); - let body_max_bytes = bounded( - params.body_max_bytes, - DEFAULT_BODY_MAX_BYTES, - MAX_BODY_MAX_BYTES, - ); - let ticket = self - .backend - .show(query) - .map_err(|error| backend_error("ShowTicket", error))?; - let summary = format!( - "Ticket {} state {}", - ticket.meta.id, - ticket.meta.workflow_state.as_str() - ); - Ok(json_output( - summary, - ticket_json(&ticket, event_limit, artifact_limit, body_max_bytes), - )) + execute_show_ticket(&self.backend, params, _ctx) } } +fn execute_show_ticket( + backend: &TicketToolBackend, + params: ShowTicketParams, + _context: agen::tool::ToolExecutionContext, +) -> 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( + params.artifact_limit, + DEFAULT_ARTIFACT_LIMIT, + MAX_ARTIFACT_LIMIT, + ); + let body_max_bytes = bounded( + params.body_max_bytes, + DEFAULT_BODY_MAX_BYTES, + MAX_BODY_MAX_BYTES, + ); + let ticket = backend + .show(query) + .map_err(|error| backend_error("ShowTicket", error))?; + let summary = format!( + "Ticket {} state {}", + ticket.meta.id, + ticket.meta.workflow_state.as_str() + ); + Ok(json_output( + summary, + ticket_json(&ticket, event_limit, artifact_limit, body_max_bytes), + )) +} + fn execute_ticket_thread_event( backend: &TicketToolBackend, tool_name: &str, kind: TicketEventKind, - input_json: &str, + params: TicketThreadEventParams, ) -> Result { - let params: TicketThreadEventParams = parse_input(tool_name, input_json)?; let role = kind.as_str().to_string(); backend .add_event( @@ -1155,7 +1189,7 @@ impl Tool for TicketCommentTool { &self.backend, "TicketComment", TicketEventKind::Comment, - input_json, + parse_input("TicketComment", input_json)?, ) } } @@ -1169,7 +1203,12 @@ macro_rules! impl_ticket_thread_event_tool { input_json: &str, _ctx: agen::tool::ToolExecutionContext, ) -> Result { - execute_ticket_thread_event(&self.backend, $name, $kind, input_json) + execute_ticket_thread_event( + &self.backend, + $name, + $kind, + parse_input($name, input_json)?, + ) } } }; @@ -1195,30 +1234,37 @@ impl Tool for TicketMarkReadyTool { ctx: agen::tool::ToolExecutionContext, ) -> Result { let params: TicketMarkReadyParams = parse_input("TicketMarkReady", input_json)?; - let ticket = self - .backend - .mark_ready( - TicketIdOrSlug::Query(params.ticket.clone()), - TicketMarkReady { - operation_key: format!("ticket-mark-ready:{}", ctx.call_id), - reason: params.reason, - author: None, - intake_summary: None, - }, - ) - .map_err(|error| backend_error("TicketMarkReady", error))?; - Ok(json_output( - format!("Marked ticket {} routing readiness ready", params.ticket), - json!({ - "ticket": ticket.meta.id, - "state": ticket.meta.workflow_state.as_str(), - "targets": ticket.meta.targets, - "ok": true - }), - )) + execute_ticket_mark_ready(&self.backend, params, ctx) } } +fn execute_ticket_mark_ready( + backend: &TicketToolBackend, + params: TicketMarkReadyParams, + ctx: agen::tool::ToolExecutionContext, +) -> Result { + let ticket = backend + .mark_ready( + TicketIdOrSlug::Query(params.ticket.clone()), + TicketMarkReady { + operation_key: format!("ticket-mark-ready:{}", ctx.call_id), + reason: params.reason, + author: None, + intake_summary: None, + }, + ) + .map_err(|error| backend_error("TicketMarkReady", error))?; + Ok(json_output( + format!("Marked ticket {} routing readiness ready", params.ticket), + json!({ + "ticket": ticket.meta.id, + "state": ticket.meta.workflow_state.as_str(), + "targets": ticket.meta.targets, + "ok": true + }), + )) +} + #[async_trait] impl Tool for TicketIntakeReadyTool { async fn execute( @@ -1227,33 +1273,40 @@ impl Tool for TicketIntakeReadyTool { ctx: agen::tool::ToolExecutionContext, ) -> Result { let params: TicketIntakeReadyParams = parse_input("TicketIntakeReady", input_json)?; - let ticket = self - .backend - .mark_ready( - TicketIdOrSlug::Query(params.ticket.clone()), - TicketMarkReady { - operation_key: format!("ticket-intake-ready:{}", ctx.call_id), - reason: params.reason, - author: None, - intake_summary: Some(TicketIntakeSummary::new(params.intake_summary)), - }, - ) - .map_err(|error| backend_error("TicketIntakeReady", error))?; - Ok(json_output( - format!( - "Marked ticket {} routing readiness ready after intake", - params.ticket - ), - json!({ - "ticket": ticket.meta.id, - "state": ticket.meta.workflow_state.as_str(), - "targets": ticket.meta.targets, - "ok": true - }), - )) + execute_ticket_intake_ready(&self.backend, params, ctx) } } +fn execute_ticket_intake_ready( + backend: &TicketToolBackend, + params: TicketIntakeReadyParams, + ctx: agen::tool::ToolExecutionContext, +) -> Result { + let ticket = backend + .mark_ready( + TicketIdOrSlug::Query(params.ticket.clone()), + TicketMarkReady { + operation_key: format!("ticket-intake-ready:{}", ctx.call_id), + reason: params.reason, + author: None, + intake_summary: Some(TicketIntakeSummary::new(params.intake_summary)), + }, + ) + .map_err(|error| backend_error("TicketIntakeReady", error))?; + Ok(json_output( + format!( + "Marked ticket {} routing readiness ready after intake", + params.ticket + ), + json!({ + "ticket": ticket.meta.id, + "state": ticket.meta.workflow_state.as_str(), + "targets": ticket.meta.targets, + "ok": true + }), + )) +} + #[async_trait] impl Tool for TicketQueueTool { async fn execute( @@ -1262,34 +1315,41 @@ impl Tool for TicketQueueTool { _ctx: agen::tool::ToolExecutionContext, ) -> Result { let params: TicketQueueParams = parse_input("TicketQueue", input_json)?; - let queued_by = default_author(); - let mut outcome = self - .backend - .queue_ready(TicketIdOrSlug::Query(params.ticket.clone()), &queued_by) - .map_err(|error| backend_error("TicketQueue", error))?; - outcome.requested_ticket = - model_ticket_reference(&self.backend, &outcome.requested_ticket, "TicketQueue")?; - outcome.queued_tickets = outcome - .queued_tickets - .into_iter() - .map(|ticket| model_ticket_reference(&self.backend, &ticket, "TicketQueue")) - .collect::, _>>()?; - Ok(json_output( - format!( - "Queued {} ticket(s) for Orchestrator", - outcome.queued_tickets.len() - ), - json!({ - "ticket": outcome.requested_ticket, - "queued_tickets": outcome.queued_tickets, - "state": "queued", - "queued_by": queued_by, - "ok": true - }), - )) + execute_ticket_queue(&self.backend, params, _ctx) } } +fn execute_ticket_queue( + backend: &TicketToolBackend, + params: TicketQueueParams, + _context: agen::tool::ToolExecutionContext, +) -> Result { + let queued_by = default_author(); + let mut outcome = backend + .queue_ready(TicketIdOrSlug::Query(params.ticket.clone()), &queued_by) + .map_err(|error| backend_error("TicketQueue", error))?; + outcome.requested_ticket = + model_ticket_reference(backend, &outcome.requested_ticket, "TicketQueue")?; + outcome.queued_tickets = outcome + .queued_tickets + .into_iter() + .map(|ticket| model_ticket_reference(backend, &ticket, "TicketQueue")) + .collect::, _>>()?; + Ok(json_output( + format!( + "Queued {} ticket(s) for Orchestrator", + outcome.queued_tickets.len() + ), + json!({ + "ticket": outcome.requested_ticket, + "queued_tickets": outcome.queued_tickets, + "state": "queued", + "queued_by": queued_by, + "ok": true + }), + )) +} + #[async_trait] impl Tool for TicketCaseUpdateTool { async fn execute( @@ -1298,35 +1358,42 @@ impl Tool for TicketCaseUpdateTool { _ctx: agen::tool::ToolExecutionContext, ) -> Result { let params: TicketCaseUpdateParams = parse_input("TicketCaseUpdate", input_json)?; - let record = self - .backend - .update_case( - ¶ms.ticket, - crate::TicketCaseUpdate { - expected_content_digest: params.expected_content_digest, - expected_state: params.expected_state, - expected_closure_operation_key: params.expected_closure_operation_key, - operation_key: params.operation_key, - action: params.action, - explanation: params.explanation, - references: params.references, - author: None, - }, - ) - .map_err(|error| backend_error("TicketCaseUpdate", error))?; - let reference = model_ticket_reference(&self.backend, ¶ms.ticket, "TicketCaseUpdate")?; - Ok(json_output( - format!( - "Updated Ticket {} case to {}", - reference, record.meta.workflow_state - ), - json!({ - "ticket": reference, "state": record.meta.workflow_state, "closure_operation_key": record.closure_operation_key, "current_closure": record.current_closure.as_ref().map(|j| json!({"operation_key": j.operation_key, "reason": j.reason, "author": j.author, "explanation": truncate_text(&j.explanation, DEFAULT_BODY_MAX_BYTES)})), "ok": true - }), - )) + execute_ticket_case_update(&self.backend, params, _ctx) } } +fn execute_ticket_case_update( + backend: &TicketToolBackend, + params: TicketCaseUpdateParams, + _context: agen::tool::ToolExecutionContext, +) -> Result { + let record = backend + .update_case( + ¶ms.ticket, + crate::TicketCaseUpdate { + expected_content_digest: params.expected_content_digest, + expected_state: params.expected_state, + expected_closure_operation_key: params.expected_closure_operation_key, + operation_key: params.operation_key, + action: params.action, + explanation: params.explanation, + references: params.references, + author: None, + }, + ) + .map_err(|error| backend_error("TicketCaseUpdate", error))?; + let reference = model_ticket_reference(backend, ¶ms.ticket, "TicketCaseUpdate")?; + Ok(json_output( + format!( + "Updated Ticket {} case to {}", + reference, record.meta.workflow_state + ), + json!({ + "ticket": reference, "state": record.meta.workflow_state, "closure_operation_key": record.closure_operation_key, "current_closure": record.current_closure.as_ref().map(|j| json!({"operation_key": j.operation_key, "reason": j.reason, "author": j.author, "explanation": truncate_text(&j.explanation, DEFAULT_BODY_MAX_BYTES)})), "ok": true + }), + )) +} + #[async_trait] impl Tool for TicketRelationRecordTool { async fn execute( @@ -1335,26 +1402,33 @@ impl Tool for TicketRelationRecordTool { _ctx: agen::tool::ToolExecutionContext, ) -> Result { let params: TicketRelationRecordParams = parse_input("TicketRelationRecord", input_json)?; - let relation = NewTicketRelation { - kind: params.kind.into_kind(), - target: params.target.clone(), - note: params.note, - author: None, - }; - let output = self - .backend - .add_ticket_relation(TicketIdOrSlug::Id(params.ticket.clone()), relation) - .map_err(|error| backend_error("TicketRelationRecord", error))?; - Ok(json_output( - format!( - "Recorded ticket relation {} {} {}", - output.ticket_id, output.kind, output.target - ), - ticket_relation_json(&output), - )) + execute_ticket_relation_record(&self.backend, params, _ctx) } } +fn execute_ticket_relation_record( + backend: &TicketToolBackend, + params: TicketRelationRecordParams, + _context: agen::tool::ToolExecutionContext, +) -> Result { + let relation = NewTicketRelation { + kind: params.kind.into_kind(), + target: params.target.clone(), + note: params.note, + author: None, + }; + let output = backend + .add_ticket_relation(TicketIdOrSlug::Id(params.ticket.clone()), relation) + .map_err(|error| backend_error("TicketRelationRecord", error))?; + Ok(json_output( + format!( + "Recorded ticket relation {} {} {}", + output.ticket_id, output.kind, output.target + ), + ticket_relation_json(&output), + )) +} + #[async_trait] impl Tool for TicketRelationRemoveTool { async fn execute( @@ -1363,24 +1437,31 @@ impl Tool for TicketRelationRemoveTool { _ctx: agen::tool::ToolExecutionContext, ) -> Result { let params: TicketRelationRemoveParams = parse_input("TicketRelationRemove", input_json)?; - let output = self - .backend - .remove_ticket_relation( - TicketIdOrSlug::Id(params.ticket), - params.kind.into_kind(), - TicketIdOrSlug::Id(params.target), - ) - .map_err(|error| backend_error("TicketRelationRemove", error))?; - Ok(json_output( - format!( - "Removed ticket relation {} {} {}", - output.ticket_id, output.kind, output.target - ), - ticket_relation_json(&output), - )) + execute_ticket_relation_remove(&self.backend, params, _ctx) } } +fn execute_ticket_relation_remove( + backend: &TicketToolBackend, + params: TicketRelationRemoveParams, + _context: agen::tool::ToolExecutionContext, +) -> Result { + let output = backend + .remove_ticket_relation( + TicketIdOrSlug::Id(params.ticket), + params.kind.into_kind(), + TicketIdOrSlug::Id(params.target), + ) + .map_err(|error| backend_error("TicketRelationRemove", error))?; + Ok(json_output( + format!( + "Removed ticket relation {} {} {}", + output.ticket_id, output.kind, output.target + ), + ticket_relation_json(&output), + )) +} + #[async_trait] impl Tool for TicketRelationQueryTool { async fn execute( @@ -1389,36 +1470,43 @@ impl Tool for TicketRelationQueryTool { _ctx: agen::tool::ToolExecutionContext, ) -> Result { let params: TicketRelationQueryParams = parse_input("TicketRelationQuery", input_json)?; - 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); - let relations = self - .backend - .query_ticket_relations(ticket, kind) - .map_err(|error| backend_error("TicketRelationQuery", error))?; - let count = relations.len(); - let truncated = count > limit; - let returned_relations = relations - .into_iter() - .take(limit) - .map(|relation| ticket_relation_json(&relation)) - .collect::>(); - Ok(json_output( - format!( - "Found {} ticket relation(s){}", - count, - if truncated { " (truncated)" } else { "" } - ), - TicketRelationQueryOutput { - count, - returned: returned_relations.len(), - truncated, - relations: returned_relations, - }, - )) + execute_ticket_relation_query(&self.backend, params, _ctx) } } +fn execute_ticket_relation_query( + backend: &TicketToolBackend, + params: TicketRelationQueryParams, + _context: agen::tool::ToolExecutionContext, +) -> 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); + let relations = backend + .query_ticket_relations(ticket, kind) + .map_err(|error| backend_error("TicketRelationQuery", error))?; + let count = relations.len(); + let truncated = count > limit; + let returned_relations = relations + .into_iter() + .take(limit) + .map(|relation| ticket_relation_json(&relation)) + .collect::>(); + Ok(json_output( + format!( + "Found {} ticket relation(s){}", + count, + if truncated { " (truncated)" } else { "" } + ), + TicketRelationQueryOutput { + count, + returned: returned_relations.len(), + truncated, + relations: returned_relations, + }, + )) +} + #[async_trait] impl Tool for TicketOrchestrationPlanRecordTool { async fn execute( @@ -1428,33 +1516,40 @@ impl Tool for TicketOrchestrationPlanRecordTool { ) -> Result { let params: TicketOrchestrationPlanRecordParams = parse_input("TicketOrchestrationPlanRecord", input_json)?; - let accepted_plan = params.accepted_plan.map(|plan| AcceptedOrchestrationPlan { - summary: plan.summary, - branch: plan.branch, - worktree: plan.worktree, - role_plan: plan.role_plan, - }); - let record = NewOrchestrationPlanRecord { - kind: params.kind.into_kind(), - related_ticket: params.related_ticket, - note: params.note, - accepted_plan, - author: None, - }; - let output = self - .backend - .add_orchestration_plan_record(TicketIdOrSlug::Query(params.ticket.clone()), record) - .map_err(|error| backend_error("TicketOrchestrationPlanRecord", error))?; - Ok(json_output( - format!( - "Recorded orchestration plan {} for ticket {}", - output.kind, params.ticket - ), - output, - )) + execute_ticket_orchestration_plan_record(&self.backend, params, _ctx) } } +fn execute_ticket_orchestration_plan_record( + backend: &TicketToolBackend, + params: TicketOrchestrationPlanRecordParams, + _context: agen::tool::ToolExecutionContext, +) -> Result { + let accepted_plan = params.accepted_plan.map(|plan| AcceptedOrchestrationPlan { + summary: plan.summary, + branch: plan.branch, + worktree: plan.worktree, + role_plan: plan.role_plan, + }); + let record = NewOrchestrationPlanRecord { + kind: params.kind.into_kind(), + related_ticket: params.related_ticket, + note: params.note, + accepted_plan, + author: None, + }; + let output = backend + .add_orchestration_plan_record(TicketIdOrSlug::Query(params.ticket.clone()), record) + .map_err(|error| backend_error("TicketOrchestrationPlanRecord", error))?; + Ok(json_output( + format!( + "Recorded orchestration plan {} for ticket {}", + output.kind, params.ticket + ), + output, + )) +} + #[async_trait] impl Tool for TicketOrchestrationPlanQueryTool { async fn execute( @@ -1464,38 +1559,45 @@ impl Tool for TicketOrchestrationPlanQueryTool { ) -> Result { let params: TicketOrchestrationPlanQueryParams = parse_input("TicketOrchestrationPlanQuery", input_json)?; - let limit = bounded(params.limit, DEFAULT_LIST_LIMIT, MAX_LIST_LIMIT); - let ticket = params.ticket.clone().map(TicketIdOrSlug::Query); - let kind = params - .relation_kind - .map(OrchestrationPlanKindParam::into_kind); - let records = self - .backend - .query_orchestration_plan_records(ticket, kind) - .map_err(|error| backend_error("TicketOrchestrationPlanQuery", error))?; - let count = records.len(); - let truncated = count > limit; - let returned_records = records - .into_iter() - .take(limit) - .map(|record| serde_json::to_value(record).unwrap_or_else(|_| json!({}))) - .collect::>(); - Ok(json_output( - format!( - "Found {} orchestration plan record(s){}", - count, - if truncated { " (truncated)" } else { "" } - ), - TicketOrchestrationPlanQueryOutput { - count, - returned: returned_records.len(), - truncated, - records: returned_records, - }, - )) + execute_ticket_orchestration_plan_query(&self.backend, params, _ctx) } } +fn execute_ticket_orchestration_plan_query( + backend: &TicketToolBackend, + params: TicketOrchestrationPlanQueryParams, + _context: agen::tool::ToolExecutionContext, +) -> Result { + let limit = bounded(params.limit, DEFAULT_LIST_LIMIT, MAX_LIST_LIMIT); + let ticket = params.ticket.clone().map(TicketIdOrSlug::Query); + let kind = params + .relation_kind + .map(OrchestrationPlanKindParam::into_kind); + let records = backend + .query_orchestration_plan_records(ticket, kind) + .map_err(|error| backend_error("TicketOrchestrationPlanQuery", error))?; + let count = records.len(); + let truncated = count > limit; + let returned_records = records + .into_iter() + .take(limit) + .map(|record| serde_json::to_value(record).unwrap_or_else(|_| json!({}))) + .collect::>(); + Ok(json_output( + format!( + "Found {} orchestration plan record(s){}", + count, + if truncated { " (truncated)" } else { "" } + ), + TicketOrchestrationPlanQueryOutput { + count, + returned: returned_records.len(), + truncated, + records: returned_records, + }, + )) +} + #[async_trait] impl Tool for TicketDoctorTool { async fn execute( @@ -1504,24 +1606,31 @@ impl Tool for TicketDoctorTool { _ctx: agen::tool::ToolExecutionContext, ) -> Result { let params: TicketDoctorParams = parse_input("TicketDoctor", input_json)?; - let limit = bounded(params.limit, DEFAULT_DIAGNOSTIC_LIMIT, MAX_DIAGNOSTIC_LIMIT); - let report = self - .backend - .doctor() - .map_err(|error| backend_error("TicketDoctor", error))?; - let output = doctor_output(report, limit); - Ok(json_output( - format!( - "Ticket doctor: {} error(s), {} diagnostic(s){}", - output.error_count, - output.diagnostic_count, - if output.truncated { " (truncated)" } else { "" } - ), - output, - )) + execute_ticket_doctor(&self.backend, params, _ctx) } } +fn execute_ticket_doctor( + backend: &TicketToolBackend, + params: TicketDoctorParams, + _context: agen::tool::ToolExecutionContext, +) -> 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( + format!( + "Ticket doctor: {} error(s), {} diagnostic(s){}", + output.error_count, + output.diagnostic_count, + if output.truncated { " (truncated)" } else { "" } + ), + output, + )) +} + #[async_trait] impl Tool for TicketDependencyCheckTool { async fn execute( @@ -1530,22 +1639,141 @@ impl Tool for TicketDependencyCheckTool { _ctx: agen::tool::ToolExecutionContext, ) -> Result { let params: TicketDependencyCheckParams = parse_input("TicketDependencyCheck", input_json)?; - let check = self - .backend - .dependency_check(TicketIdOrSlug::Query(params.ticket.clone())) - .map_err(|error| backend_error("TicketDependencyCheck", error))?; - Ok(json_output( - format!( - "Ticket {} dependency check: {}", - params.ticket, - if check.queue_guard.can_queue_for_orchestrator { - "queueable" - } else { - "not queueable" - } - ), - check, - )) + execute_ticket_dependency_check(&self.backend, params, _ctx) + } +} + +fn execute_ticket_dependency_check( + backend: &TicketToolBackend, + params: TicketDependencyCheckParams, + _context: agen::tool::ToolExecutionContext, +) -> Result { + let check = backend + .dependency_check(TicketIdOrSlug::Query(params.ticket.clone())) + .map_err(|error| backend_error("TicketDependencyCheck", error))?; + Ok(json_output( + format!( + "Ticket {} dependency check: {}", + params.ticket, + if check.queue_guard.can_queue_for_orchestrator { + "queueable" + } else { + "not queueable" + } + ), + check, + )) +} + +/// Execute an internal Ticket command after an adapter validates and converts its contract. +/// +/// These DTOs are shared with ordinary Tool input conversion, not a native public contract. +/// This boundary never invokes Tools or validates their JSON schemas. Backend authorization, +/// business validation and durable operation identities remain shared. +pub async fn execute_ticket_command( + 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}"))) + } + match operation { + "TicketCreate" => { + execute_ticket_create(&backend, convert::(command)?, context) + } + "TicketEditItem" => { + execute_ticket_edit_item(&backend, convert::(command)?, context) + } + "TicketEditPost" => { + execute_ticket_edit_post(&backend, convert::(command)?, context) + } + "QueryTicket" => { + execute_query_ticket(&backend, convert::(command)?, context) + } + "ShowTicket" => { + execute_show_ticket(&backend, convert::(command)?, context) + } + "TicketMarkReady" => execute_ticket_mark_ready( + &backend, + convert::(command)?, + context, + ), + "TicketIntakeReady" => execute_ticket_intake_ready( + &backend, + convert::(command)?, + context, + ), + "TicketQueue" => { + execute_ticket_queue(&backend, convert::(command)?, context) + } + "TicketCaseUpdate" => execute_ticket_case_update( + &backend, + convert::(command)?, + context, + ), + "TicketRelationRecord" => execute_ticket_relation_record( + &backend, + convert::(command)?, + context, + ), + "TicketRelationRemove" => execute_ticket_relation_remove( + &backend, + convert::(command)?, + context, + ), + "TicketRelationQuery" => execute_ticket_relation_query( + &backend, + convert::(command)?, + context, + ), + "TicketOrchestrationPlanRecord" => execute_ticket_orchestration_plan_record( + &backend, + convert::(command)?, + context, + ), + "TicketOrchestrationPlanQuery" => execute_ticket_orchestration_plan_query( + &backend, + convert::(command)?, + context, + ), + "TicketDoctor" => { + execute_ticket_doctor(&backend, convert::(command)?, context) + } + "TicketDependencyCheck" => execute_ticket_dependency_check( + &backend, + convert::(command)?, + context, + ), + "TicketComment" => execute_ticket_thread_event( + &backend, + operation, + TicketEventKind::Comment, + convert(command)?, + ), + "TicketPlan" => execute_ticket_thread_event( + &backend, + operation, + TicketEventKind::Plan, + convert(command)?, + ), + "TicketDecision" => execute_ticket_thread_event( + &backend, + operation, + TicketEventKind::Decision, + convert(command)?, + ), + "TicketImplementationReport" => execute_ticket_thread_event( + &backend, + operation, + TicketEventKind::ImplementationReport, + convert(command)?, + ), + _ => Err(ToolError::InvalidArgument(format!( + "unknown Ticket command: {operation}" + ))), } } @@ -2026,6 +2254,193 @@ mod tests { .expect("tool exists") } + #[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( + backend.clone().into(), + "TicketCreate", + json!({"title":"Shared execution", "body":"Original"}), + Default::default(), + ) + .await + .unwrap(); + let id = serde_json::from_str::(created.content.as_deref().unwrap()).unwrap()["id"] + .as_str() + .unwrap() + .to_owned(); + let edit = tool_by_name(backend.clone(), "TicketEditItem"); + edit.execute( + &json!({"ticket":id, "old_string":"Original", "new_string":"Tool edit"}).to_string(), + Default::default(), + ) + .await + .unwrap(); + execute_ticket_command( + backend.clone().into(), + "TicketEditItem", + json!({"ticket":id, "old_string":"Tool edit", "new_string":"Command edit"}), + Default::default(), + ) + .await + .unwrap(); + execute_ticket_command( + backend.clone().into(), + "TicketComment", + json!({"ticket":id, "body":"Discussion"}), + Default::default(), + ) + .await + .unwrap(); + let before = backend.show(id.clone().into()).unwrap(); + assert_eq!(before.document.body.as_str(), "Command edit"); + let post = before + .events + .iter() + .find(|event| !event.is_initial) + .unwrap(); + let input = json!({"ticket":id, "event_ref":post.event_ref, "expected_body":"Discussion", "body":"Updated"}); + execute_ticket_command( + backend.clone().into(), + "TicketEditPost", + input.clone(), + Default::default(), + ) + .await + .unwrap(); + let native_error = execute_ticket_command( + backend.clone().into(), + "TicketEditPost", + input.clone(), + Default::default(), + ) + .await + .unwrap_err(); + let tool_error = tool_by_name(backend.clone(), "TicketEditPost") + .execute(&input.to_string(), Default::default()) + .await + .unwrap_err(); + assert_eq!(native_error.to_string(), tool_error.to_string()); + let after = backend.show(id.clone().into()).unwrap(); + assert_eq!( + after + .events + .iter() + .find(|event| !event.is_initial) + .unwrap() + .body + .as_str(), + "Updated" + ); + let input = json!({"ticket":id}); + let native = execute_ticket_command( + backend.clone().into(), + "TicketDependencyCheck", + input.clone(), + 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); + } + + #[tokio::test] + async fn internal_relation_and_plan_commands_share_tools_business_validation_and_queries() { + let temp = TempDir::new().unwrap(); + let backend = backend(&temp); + 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( + backend.clone().into(), + "TicketRelationRecord", + relation.clone(), + Default::default(), + ) + .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( + backend.clone().into(), + "TicketRelationRemove", + relation, + Default::default(), + ) + .await + .unwrap(); + let plan = json!({"ticket":source, "kind":"accepted_plan", "accepted_plan":{"summary":"Shared execution plan", "branch":"work"}}); + execute_ticket_command( + backend.clone().into(), + "TicketOrchestrationPlanRecord", + plan, + Default::default(), + ) + .await + .unwrap(); + let native = execute_ticket_command( + backend.clone().into(), + "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); + let invalid = json!({"ticket":source, "kind":"accepted_plan"}); + let native = execute_ticket_command( + backend.clone().into(), + "TicketOrchestrationPlanRecord", + invalid.clone(), + Default::default(), + ) + .await + .unwrap_err(); + let ordinary = tool_by_name(backend.clone(), "TicketOrchestrationPlanRecord") + .execute(&invalid.to_string(), Default::default()) + .await + .unwrap_err(); + assert_eq!(native.to_string(), ordinary.to_string()); + assert!(matches!( + execute_ticket_command( + backend.clone().into(), + "TicketRelationRecord", + json!({"ticket":source,"target":target,"kind":"invalid"}), + Default::default() + ) + .await, + Err(ToolError::InvalidArgument(_)) + )); + assert!( + execute_ticket_command(backend.into(), "Unknown", json!({}), Default::default()) + .await + .unwrap_err() + .to_string() + .contains("unknown Ticket command") + ); + } + #[tokio::test] async fn edit_post_tool_and_show_preserve_common_identity_and_metadata() { let temp = TempDir::new().unwrap(); diff --git a/crates/worker/src/controller.rs b/crates/worker/src/controller.rs index 997f7e3f7..cce9352fe 100644 --- a/crates/worker/src/controller.rs +++ b/crates/worker/src/controller.rs @@ -1975,15 +1975,42 @@ where let workspace_client = worker.workspace_client_handle(); let engine = worker.engine_mut(); - if feature_config.merge_request.any() && wip_mode { - let namespace_route = wip_mount_registry - .allocate_namespace("merge-request", "merge-requests") - .map_err(|error| { - std::io::Error::new( - std::io::ErrorKind::InvalidInput, - format!("allocate Merge Request WIP namespace: {error}"), - ) - })?; + // Allocate cross-domain input reference coordinates before mounting either provider. + // Allocation is not publication or authorization; current handlers still check both. + let mut allocate = |enabled, owner, name| { + if enabled && wip_mode { + wip_mount_registry + .allocate_namespace(owner, name) + .map(Some) + .map_err(|error| { + std::io::Error::new( + std::io::ErrorKind::InvalidInput, + format!("allocate {owner} WIP namespace: {error}"), + ) + }) + } else { + Ok(None) + } + }; + let merge_request_namespace = allocate( + feature_config.merge_request.any() || feature_config.ticket.enabled, + "merge-request", + "merge-requests", + )?; + let ticket_namespace = allocate( + feature_config.ticket.enabled || feature_config.merge_request.open, + "ticket", + "tickets", + )?; + let objective_namespace = allocate( + feature_config.objective.enabled || feature_config.ticket.enabled, + "objective", + "objectives", + )?; + + if feature_config.merge_request.any() + && let Some(namespace_route) = &merge_request_namespace + { crate::feature::builtin::merge_request::mount_workspace_http_merge_request_wip( &mut wip_mount_registry, workspace_client.clone(), @@ -1999,21 +2026,15 @@ where })?; } - if feature_config.ticket.enabled && wip_mode { + if feature_config.ticket.enabled + && let Some(namespace_route) = &ticket_namespace + { let ticket_access = crate::feature::builtin::ticket::TicketFeatureAccess { authoring: feature_config.ticket.authoring, thread: feature_config.ticket.thread, intake: feature_config.ticket.intake, workflow: feature_config.ticket.workflow, }; - let namespace_route = wip_mount_registry - .allocate_namespace("ticket", "tickets") - .map_err(|error| { - std::io::Error::new( - std::io::ErrorKind::InvalidInput, - format!("allocate Ticket WIP namespace: {error}"), - ) - })?; crate::feature::builtin::ticket::mount_workspace_http_ticket_wip( &mut wip_mount_registry, workspace_client.clone(), @@ -2039,15 +2060,7 @@ where ) { engine.register_tool(definition); } - if wip_mode { - let namespace_route = wip_mount_registry - .allocate_namespace("objective", "objectives") - .map_err(|error| { - std::io::Error::new( - std::io::ErrorKind::InvalidInput, - format!("allocate Objective WIP namespace: {error}"), - ) - })?; + if let Some(namespace_route) = &objective_namespace { crate::feature::builtin::objective::mount_workspace_http_objective_wip( &mut wip_mount_registry, workspace_client.clone(), @@ -4316,6 +4329,8 @@ mod tests { } } + include!("controller/native_record_tests.rs"); + struct LargeOutputTool; #[async_trait::async_trait] diff --git a/crates/worker/src/controller/native_record_tests.rs b/crates/worker/src/controller/native_record_tests.rs new file mode 100644 index 000000000..bb71ca353 --- /dev/null +++ b/crates/worker/src/controller/native_record_tests.rs @@ -0,0 +1,179 @@ +// Controller composition regression: real registration and model-facing Tools, with only +// Workspace transport substituted. MR-only publication must not depend on Ticket activation. +#[tokio::test] +async fn mr_only_controller_reserves_ticket_coordinates_without_publishing_ticket_or_granting_access() + { + use crate::worker::{ + WorkspaceClient, WorkspaceClientError, WorkspaceRequest, WorkspaceResponse, + }; + use std::sync::atomic::{AtomicBool, Ordering}; + #[derive(Debug)] + struct Host { + allowed: AtomicBool, + requests: Mutex>, + } + impl WorkspaceClient for Host { + fn workspace_id(&self) -> Option<&str> { + Some("workspace") + } + fn kind(&self) -> &str { + "mr-only-controller" + } + fn is_available(&self) -> bool { + true + } + fn execute( + &self, + request: WorkspaceRequest, + ) -> Result { + if request.path.ends_with("/authorization/check") { + let check: server_api::authorization::OperationAuthorizationRequest = + serde_json::from_str(request.body.as_deref().unwrap()).unwrap(); + assert_eq!(check.operation, "merge_request.open"); + assert_eq!( + check.resource, + server_api::authorization::PermissionResource::Ticket { + workspace_id: "workspace".into(), + ticket_id: "T-42".into() + } + ); + return Ok(WorkspaceResponse { + status: if self.allowed.load(Ordering::SeqCst) { + 200 + } else { + 403 + }, + body: "{}".into(), + }); + } + assert_eq!(request.path, "/api/w/workspace/tickets/T-42/merge-request"); + self.requests.lock().unwrap().push(request); + Ok(WorkspaceResponse { + status: 200, + body: serde_json::json!({"merge_request_id":"MR-42"}).to_string(), + }) + } + } + let root = TempDir::new().unwrap(); + let mut manifest=manifest::WorkerManifest::from_toml("[worker]\nname = 'mr-only'\n[model]\nscheme = 'anthropic'\nmodel_id = 'test'\n[engine]\n[scope]\n").unwrap(); + manifest.worker.mode = manifest::WorkerMode::Wip; + manifest.feature.ticket.enabled = false; + manifest.feature.objective.enabled = false; + manifest.feature.drive.enabled = false; + manifest.feature.workdir_catalog.enabled = false; + manifest.feature.merge_request = manifest::MergeRequestFeatureConfig { + open: true, + ..Default::default() + }; + let host = Arc::new(Host { + allowed: AtomicBool::new(true), + requests: Mutex::new(vec![]), + }); + let engine = Engine::<_, agen::state::Mutable, crate::SessionHistoryMetadata>::new_annotated( + ScriptedClient::new(vec![]), + ); + let store = session_store::CombinedStore::new( + session_store::FsStore::new(root.path().join("sessions")).unwrap(), + session_store::FsWorkerStore::new(root.path().join("workers")).unwrap(), + ); + let context = crate::worker::WorkerWorkspaceContext::with_client( + Some(crate::worker::WorkspaceId::new("workspace").unwrap()), + host.clone(), + ); + let mut worker = Worker::new( + manifest, + engine, + store, + context, + crate::worker::WorkerFilesystemAuthority::None, + manifest::Scope::empty(), + ) + .await + .unwrap(); + register_worker_tools( + &mut worker, + root.path().join("runtime"), + SpawnedWorkerRegistry::new_for_internal_services(), + None, + None, + ) + .await + .unwrap(); + assert!( + host.requests.lock().unwrap().is_empty(), + "registration does not authorize or dispatch domain work" + ); + let tools = worker.engine().tool_server_handle(); + tools.flush_pending(); + let (_, inspect) = tools.get_tool("Inspect").unwrap(); + assert!( + inspect + .execute( + r#"{"path":"/tickets/T-42"}"#, + ToolExecutionContext::direct() + ) + .await + .is_err() + ); + let output = inspect + .execute( + r#"{"path":"/merge-requests"}"#, + ToolExecutionContext::direct(), + ) + .await + .unwrap(); + let object: serde_json::Value = serde_json::from_str(&output.content.unwrap()).unwrap(); + let definition = inspect + .execute( + &serde_json::json!({"path":object["interfaces"][0]["path"]}).to_string(), + ToolExecutionContext::direct(), + ) + .await + .unwrap(); + assert!(definition.content.unwrap().contains("ticket: entry")); + let (_, invoke) = tools.get_tool("Invoke").unwrap(); + let input = serde_json::json!({"path":"/merge-requests","interface":object["interfaces"][0]["reference"],"operation":"open","arguments":{"ticket":"/tickets/T-42","repository":{"key":"main","source":"work/T-42","target":"develop"}}}); + invoke + .execute(&input.to_string(), ToolExecutionContext::direct()) + .await + .unwrap(); + assert_eq!(host.requests.lock().unwrap().len(), 1); + host.allowed.store(false, Ordering::SeqCst); + assert!( + invoke + .execute(&input.to_string(), ToolExecutionContext::direct()) + .await + .is_err() + ); + assert_eq!( + host.requests.lock().unwrap().len(), + 1, + "revocation prevents a second domain effect" + ); + for target in [ + "T-42", + "/objectives/O-42", + "/tickets-extra/T-42", + "/tickets/T-42/child", + ] { + let mut invalid = input.clone(); + invalid["arguments"]["ticket"] = serde_json::json!(target); + assert!( + invoke + .execute(&invalid.to_string(), ToolExecutionContext::direct()) + .await + .is_err() + ); + } + assert_eq!(host.requests.lock().unwrap().len(), 1); + assert!(tools.get_tool("TicketCreate").is_none()); + assert!( + inspect + .execute( + r#"{"path":"/tickets/T-42"}"#, + ToolExecutionContext::direct() + ) + .await + .is_err() + ); +} diff --git a/crates/worker/src/feature/builtin.rs b/crates/worker/src/feature/builtin.rs index 137b07e72..c03d07983 100644 --- a/crates/worker/src/feature/builtin.rs +++ b/crates/worker/src/feature/builtin.rs @@ -30,6 +30,7 @@ pub mod memory_staging_output; pub(crate) mod memory_surface_lifecycle; pub(crate) mod memory_surface_output; pub mod merge_request; +mod native_contract; pub mod objective; pub mod orchestration; mod resource_projection; diff --git a/crates/worker/src/feature/builtin/merge_request.rs b/crates/worker/src/feature/builtin/merge_request.rs index 884215b76..6cb0d6a63 100644 --- a/crates/worker/src/feature/builtin/merge_request.rs +++ b/crates/worker/src/feature/builtin/merge_request.rs @@ -1,10 +1,17 @@ //! Connects enabled Merge Request Tools to the Workspace API and native WIP. //! -//! The native projection accepts only `/merge-requests` and its immediate canonical IDs, binding -//! each item's subject to its route. It removes `merge_request_id` from descriptors and reconstructs -//! it for original schema and permission evaluation. Only enabled regular Tools are projected; -//! no Backend-wide catalog or custom query is added. [`crate::wip`] owns shared reference, -//! permission, Client-lifetime, and unknown-outcome contracts. +//! The collection opens a repository candidate linked to a Ticket Entry; an item is one Merge +//! Request whose identity is exclusively route-bound. Keeping these Objects separate prevents a +//! read/review/integration from selecting a different subject in arguments. The item Interface owns +//! independent `read`, `readiness`, `review`, and `complete` actions: reading is not a readiness +//! judgment, a review attests a source, and completion records an already-applied integration. None +//! implies another action, Git integration, or Ticket conclusion. +//! +//! Native v2 contracts are defined in the child `native` module, independently of Tool JSON Schema. +//! Their declarations drive descriptor generation, input validation, and command-field conversion. +//! Tool mode parses its own DTOs. Both enter the shared typed command dispatcher, including Backend +//! operation authorization; native never calls `Tool::execute`. Only configured actions are mounted. +//! [`crate::wip`] owns shared reference, permission, Client-lifetime, and unknown-outcome contracts. //! //! # Injected review capability //! @@ -19,7 +26,8 @@ //! 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. //! Item responses are checked against the bound MR; subject mismatches after mutations are not -//! treated as unexecuted operations. Optional argument omission/null stays as in original Tool input. +//! treated as unexecuted operations. Native omission and explicit nullable unions are distinct; +//! native input changes do not change ordinary Tool omission/null behavior. use crate::feature::{ FeatureDescriptor, FeatureInstallContext, FeatureInstallError, FeatureInstructionContribution, @@ -30,7 +38,7 @@ use crate::permission::permission_action_for; use crate::wip::{ WipCallContext, WipDynamicItem, WipDynamicItemResolver, WipDynamicMount, WipMountError, WipMountRegistry, WipNamespaceRoute, WipOperationError, WipOperationHandler, - WipOperationOutput, WipProjection, WipProjectionKind, json_to_wip, wip_to_json, + WipOperationOutput, WipProjection, WipProjectionKind, json_to_wip, }; use crate::worker::{WorkspaceClient, WorkspaceRequest, WorkspaceRequestMethod}; use agen::tool::{Tool, ToolError, ToolExecutionContext, ToolMeta, ToolOutput}; @@ -41,12 +49,11 @@ use serde::Deserialize; use serde_json::{Value, json}; use server_api::authorization::PermissionResource; use sha2::{Digest, Sha256}; -use std::collections::{BTreeMap, BTreeSet, HashMap}; +use std::collections::{BTreeMap, HashMap}; use std::sync::{Arc, Mutex}; use wip_protocol::{ Documentation, INTERFACE_FORMAT_V1, InterfaceDescriptor, Object, OperationDeclaration, - ParameterDeclaration, ProtocolError, ProtocolErrorCode, ReturnDeclaration, TypeExpr, - Value as WipValue, + ProtocolError, ProtocolErrorCode, Value as WipValue, }; pub const FEATURE_ID: &str = "merge_request"; @@ -190,178 +197,16 @@ impl Kind { matches!(self, Self::Open | Self::Review | Self::Complete) } } +mod command; +mod native; + #[async_trait] impl Tool for MergeRequestTool { async fn execute(&self, input: &str, _: ToolExecutionContext) -> Result { - let ws = self.client.workspace_id().ok_or_else(|| { - ToolError::ExecutionFailed("Merge Request tools require Workspace identity".into()) - })?; - if matches!(self.kind, Kind::Show) { - let value: ShowMergeRequestInput = parse(input)?; - nonempty_named("merge_request_id", &value.merge_request_id)?; - if value.limit.is_some_and(|limit| !(1..=200).contains(&limit)) { - return Err(ToolError::InvalidArgument( - "limit must be between 1 and 200".into(), - )); - } - return self.show_merge_request(ws, &value.merge_request_id, value.after, value.limit); - } - let (method, path, body) = match self.kind { - Kind::Readiness => { - let v: MergeRequestInput = parse(input)?; - nonempty_named("merge_request_id", &v.merge_request_id)?; - self.authorize( - "merge_request.readiness", - PermissionResource::MergeRequest { - workspace_id: ws.into(), - merge_request_id: v.merge_request_id.clone(), - }, - )?; - ( - WorkspaceRequestMethod::Get, - format!( - "/api/w/{ws}/merge-requests/{}/readiness", - encode_path_segment(&v.merge_request_id) - ), - None, - ) - } - Kind::Show => unreachable!("ShowMergeRequest is handled above"), - Kind::Open => { - let v: OpenMergeRequestInput = parse(input)?; - nonempty(&v.ticket)?; - self.authorize( - "merge_request.open", - PermissionResource::Ticket { - workspace_id: ws.into(), - ticket_id: v.ticket.clone(), - }, - )?; - ( - WorkspaceRequestMethod::Post, - format!( - "/api/w/{ws}/tickets/{}/merge-request", - encode_path_segment(&v.ticket) - ), - Some( - json!({"repository_key":v.repository_key,"selector_from":v.selector_from,"selector_to":v.selector_to,"summary":v.summary}), - ), - ) - } - Kind::Complete => { - let v: CompleteMergeRequestInput = parse(input)?; - nonempty_named("merge_request_id", &v.merge_request_id)?; - self.authorize( - "merge_request.complete", - PermissionResource::MergeRequest { - workspace_id: ws.into(), - merge_request_id: v.merge_request_id.clone(), - }, - )?; - ( - WorkspaceRequestMethod::Post, - format!( - "/api/w/{ws}/merge-requests/{}/complete", - encode_path_segment(&v.merge_request_id) - ), - Some( - json!({"operation_id":v.operation_id,"approval_event_id":v.approval_event_id,"target_ref_before":v.target_ref_before,"target_ref_after":v.target_ref_after,"strategy":match v.strategy{MergeStrategyInput::FastForward=>"fast_forward",MergeStrategyInput::Merge=>"merge"},"resolution":match v.resolution{MergeResolutionInput::None=>"none",MergeResolutionInput::Clean=>"clean",MergeResolutionInput::ConflictsResolved=>"conflicts_resolved"}}), - ), - ) - } - Kind::Review => { - let v: ReviewMergeRequestInput = parse(input)?; - let ctx = self.client.reviewer_context().ok_or_else(|| { - ToolError::ExecutionFailed( - "Review submit requires injected Reviewer capability".into(), - ) - })?; - self.authorize( - "merge_request.review", - PermissionResource::MergeRequest { - workspace_id: ws.into(), - merge_request_id: ctx.merge_request_id.clone(), - }, - )?; - ( - WorkspaceRequestMethod::Post, - format!( - "/api/w/{ws}/merge-requests/{}/reviews", - encode_path_segment(&ctx.merge_request_id) - ), - Some( - json!({"decision":match v.decision{ReviewDecisionInput::Approve=>"approve",ReviewDecisionInput::RequestChanges=>"request_changes"},"body":v.body,"findings":v.findings.into_iter().map(|f|json!({"severity":f.severity,"code":f.code,"path":f.path,"line":f.line,"body":f.body})).collect::>() }), - ), - ) - } - }; - let req = match body { - Some(v) => WorkspaceRequest::json(method, path, v.to_string()), - None => WorkspaceRequest::get(path), - }; - let res = self - .client - .execute(req) - .map_err(|e| ToolError::ExecutionFailed(e.to_string()))?; - if !res.is_success() { - return Err(api_error("Merge Request API", &res)); - } - Ok(ToolOutput { - summary: self.kind.name().into(), - content: Some(res.body), - attachments: vec![], - }) - } -} - -impl MergeRequestTool { - fn authorize(&self, operation: &str, resource: PermissionResource) -> Result<(), ToolError> { - self.client.authorize_operation(operation, &resource) - .map_err(|error| { - let denied = matches!(&error, crate::worker::WorkspaceClientError::Request(message) - if message.contains("HTTP status 403") || message.contains("HTTP status 401")); - ToolError::ExecutionFailed(format!("Merge Request operation authorization failed before dispatch: {operation}; resource={resource:?}; denied={denied}")) - }) - } - fn show_merge_request( - &self, - workspace_id: &str, - merge_request_id: &str, - after: Option, - limit: Option, - ) -> Result { - self.authorize( - "merge_request.read", - PermissionResource::MergeRequest { - workspace_id: workspace_id.into(), - merge_request_id: merge_request_id.into(), - }, - )?; - let merge_request_id = encode_path_segment(merge_request_id); - let mut path = format!("/api/w/{workspace_id}/merge-requests/{merge_request_id}"); - let mut query = Vec::new(); - if let Some(after) = after { - query.push(format!("after={after}")); - } - if let Some(limit) = limit { - query.push(format!("limit={limit}")); - } - if !query.is_empty() { - path.push('?'); - path.push_str(&query.join("&")); - } - let response = self - .client - .execute(WorkspaceRequest::get(path)) - .map_err(|error| ToolError::ExecutionFailed(error.to_string()))?; - 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![], - }) + command::execute( + self.client.clone(), + command::Command::from_tool(self.kind, input)?, + ) } } @@ -490,8 +335,8 @@ impl FeatureModule for MergeRequestFeature { } } -const MERGE_REQUEST_COLLECTION_INTERFACE: &str = "yoi.merge-request/collection/v1"; -const MERGE_REQUEST_ITEM_INTERFACE: &str = "yoi.merge-request/item/v1"; +const MERGE_REQUEST_COLLECTION_INTERFACE: &str = "yoi.merge-request/collection/v2"; +const MERGE_REQUEST_ITEM_INTERFACE: &str = "yoi.merge-request/item/v2"; #[derive(Clone, Copy, Debug, Eq, PartialEq)] enum NativeMergeRequestSurface { @@ -501,7 +346,6 @@ enum NativeMergeRequestSurface { #[derive(Clone, Copy, Debug)] struct NativeMergeRequestOperation { - operation: &'static str, surface: NativeMergeRequestSurface, identity_field: Option<&'static str>, } @@ -509,27 +353,22 @@ struct NativeMergeRequestOperation { fn native_merge_request_operation(kind: Kind) -> NativeMergeRequestOperation { match kind { Kind::Show => NativeMergeRequestOperation { - operation: "read", surface: NativeMergeRequestSurface::Item, identity_field: Some("merge_request_id"), }, Kind::Open => NativeMergeRequestOperation { - operation: "open", surface: NativeMergeRequestSurface::Collection, identity_field: None, }, Kind::Review => NativeMergeRequestOperation { - operation: "review", surface: NativeMergeRequestSurface::Item, identity_field: None, }, Kind::Readiness => NativeMergeRequestOperation { - operation: "check_readiness", surface: NativeMergeRequestSurface::Item, identity_field: Some("merge_request_id"), }, Kind::Complete => NativeMergeRequestOperation { - operation: "complete", surface: NativeMergeRequestSurface::Item, identity_field: Some("merge_request_id"), }, @@ -539,11 +378,10 @@ fn native_merge_request_operation(kind: Kind) -> NativeMergeRequestOperation { #[derive(Clone)] struct NativeMergeRequestTool { name: String, - schema: Value, - tool: Arc, + contract: native::Contract, + roots: native::ReferenceRoots, kind: Kind, projection: NativeMergeRequestOperation, - description: String, client: Arc, } @@ -555,20 +393,12 @@ fn native_merge_request_tools( .into_iter() .filter(|kind| kind.enabled(config)) .map(|kind| { - let (meta, tool) = definition(client.clone(), kind)(); - jsonschema::validator_for(&meta.input_schema).map_err(|error| { - WipMountError::InvalidProjection { - route: "/merge-requests".into(), - message: format!("Merge Request tool schema cannot be retained: {error}"), - } - })?; Ok(NativeMergeRequestTool { - name: meta.name, - schema: meta.input_schema, - tool, + name: kind.name().into(), + contract: native::contract(kind), + roots: native::ReferenceRoots::default(), kind, projection: native_merge_request_operation(kind), - description: meta.description, client: client.clone(), }) }) @@ -593,7 +423,11 @@ pub fn mount_workspace_http_merge_request_wip( namespace_route: &WipNamespaceRoute, ) -> Result<(), WipMountError> { let collection_route = namespace_route.root().to_string(); - let tools = native_merge_request_tools(client, config)?; + let roots = native::reference_roots(registry); + let mut tools = native_merge_request_tools(client, config)?; + for tool in &mut tools { + tool.roots = roots.clone(); + } let claimed_tools = tools .iter() .map(|tool| tool.name.as_str()) @@ -613,14 +447,14 @@ pub fn mount_workspace_http_merge_request_wip( let collection_descriptor = merge_request_descriptor( &collection_tools, "Open Merge Requests through existing scoped authority", - "Only collection-scoped Merge Request Feature operations enabled for this Worker are published. Opening retains repository and selector inputs; Ticket completion is a separate Ticket operation.", + "Open links a Ticket Entry to a repository {key, source, target}. Collection and route-bound item objects separate creation from subject operations. Inspect v2 contracts; v1 flattened inputs are not accepted. Ticket conclusion is independent.", )?; registry.mount(WipProjection { route: collection_route.clone(), capability: "merge-request:collection".into(), kind: WipProjectionKind::Native, object: Object { - name: "merge-requests".into(), + name: collection_route.trim_start_matches('/').into(), description: Some( "Authoritative Merge Request collection through scoped Backend authority".into(), ), @@ -647,7 +481,7 @@ pub fn mount_workspace_http_merge_request_wip( let item_descriptor = merge_request_descriptor( &item_tools, "Inspect and operate on the Merge Request bound to this object route", - "The subject Merge Request identity comes exclusively from the target route. Repository selectors, exact review capability/candidate, approval, target refs, strategy, and resolution remain ordinary Backend-validated preconditions.", + "The subject Merge Request identity comes exclusively from the target route. Read uses an optional page record. Review requires the exact injected capability; complete groups approval and target observations in integration. Backend validates business preconditions. No operation retries unknown outcomes.", )?; registry.mount_dynamic(WipDynamicMount { collection_route: collection_route.clone(), @@ -671,7 +505,7 @@ fn merge_request_operation_map( ) -> HashMap { tools .into_iter() - .map(|tool| (tool.projection.operation.to_string(), tool)) + .map(|tool| (tool.contract.name.to_string(), tool)) .collect() } @@ -694,9 +528,8 @@ impl WipOperationHandler for MergeRequestCollectionWipHandler { .tools .get(operation) .ok_or_else(merge_request_operation_not_found)?; - let input = native_merge_request_input(arguments, None, tool.projection.identity_field)?; let output = - execute_native_merge_request_tool(tool, &self.permissions, input, None, context) + execute_native_merge_request_tool(tool, &self.permissions, arguments, None, context) .await?; let mut response = merge_request_tool_output_json(output)?; match tool.kind { @@ -784,15 +617,10 @@ impl WipOperationHandler for MergeRequestItemWipHandler { .tools .get(operation) .ok_or_else(merge_request_operation_not_found)?; - let input = native_merge_request_input( - arguments, - Some(&self.merge_request_id), - tool.projection.identity_field, - )?; let output = execute_native_merge_request_tool( tool, &self.permissions, - input, + arguments, Some(&self.merge_request_id), context, ) @@ -818,9 +646,9 @@ impl WipOperationHandler for MergeRequestItemWipHandler { async fn execute_native_merge_request_tool( tool: &NativeMergeRequestTool, permissions: &Option, - input: Value, + arguments: &BTreeMap, bound_merge_request: Option<&str>, - context: WipCallContext, + _context: WipCallContext, ) -> Result { if tool.kind == Kind::Review { let reviewer = tool.client.reviewer_context().ok_or_else(|| { @@ -836,52 +664,30 @@ async fn execute_native_merge_request_tool( )); } } - let validator = jsonschema::validator_for(&tool.schema).map_err(|error| { - merge_request_protocol_failure( - ProtocolErrorCode::Internal, - format!("stored Merge Request tool schema is invalid: {error}"), - ) - })?; - if let Err(error) = validator.validate(&input) { - return Err(merge_request_protocol_failure( - ProtocolErrorCode::InvalidArguments, - format!("Merge Request operation input violates its typed Tool schema: {error}"), - )); - } + 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. authorize_native_merge_request(permissions, &tool.name, &input)?; - let input = serde_json::to_string(&input).map_err(|error| { - merge_request_protocol_failure(ProtocolErrorCode::InvalidArguments, error.to_string()) - })?; - tool.tool - .execute(&input, context.execution) - .await + let command = native::command(tool.kind, input)?; + command::execute(tool.client.clone(), command) .map_err(|error| map_merge_request_tool_error(error, tool.kind.mutating())) } fn native_merge_request_input( + tool: &NativeMergeRequestTool, arguments: &BTreeMap, bound_merge_request: Option<&str>, - identity_field: Option<&str>, ) -> Result { - if bound_merge_request.is_some() && arguments.contains_key("merge_request_id") { - return Err(merge_request_protocol_failure( - ProtocolErrorCode::InvalidArguments, - "the subject Merge Request is bound exclusively by the object route", - )); + let mut input = native::convert(&tool.contract, arguments, &tool.roots)?; + if let (Some(merge_request_id), Some(field)) = + (bound_merge_request, tool.projection.identity_field) + { + input + .as_object_mut() + .expect("native contract produces a record") + .insert(field.into(), Value::String(merge_request_id.into())); } - let mut input = serde_json::Map::new(); - for (name, value) in arguments { - input.insert( - name.clone(), - wip_to_json(value).map_err(|message| { - merge_request_protocol_failure(ProtocolErrorCode::InvalidArguments, message) - })?, - ); - } - if let (Some(merge_request_id), Some(field)) = (bound_merge_request, identity_field) { - input.insert(field.into(), Value::String(merge_request_id.into())); - } - Ok(Value::Object(input)) + Ok(input) } fn authorize_native_merge_request( @@ -1063,77 +869,7 @@ fn merge_request_descriptor( fn merge_request_operation_declaration( tool: &NativeMergeRequestTool, ) -> Result { - let properties = tool - .schema - .get("properties") - .and_then(Value::as_object) - .ok_or_else(|| WipMountError::InvalidProjection { - route: "/merge-requests".into(), - message: format!( - "Merge Request tool `{}` schema has no properties", - tool.name - ), - })?; - let required = tool - .schema - .get("required") - .and_then(Value::as_array) - .into_iter() - .flatten() - .filter_map(Value::as_str) - .collect::>(); - let parameters = properties - .iter() - .filter(|(name, _)| tool.projection.identity_field != Some(name.as_str())) - .map(|(name, schema)| ParameterDeclaration { - name: name.clone(), - required: required.contains(name.as_str()), - documentation: schema - .get("description") - .and_then(Value::as_str) - .map(|summary| Documentation { - summary: summary.into(), - details: None, - }), - r#type: merge_request_schema_type_expr(schema), - }) - .collect(); - Ok(OperationDeclaration { - name: tool.projection.operation.into(), - documentation: Some(Documentation { - summary: tool.description.clone(), - details: Some(format!( - "Delegates to the existing `{}` Merge Request operation and retains its exact JSON Schema and Backend authority checks.", - tool.name - )), - }), - parameters, - returns: ReturnDeclaration { - documentation: Some(Documentation { - summary: "Authoritative bounded Merge Request operation result".into(), - details: None, - }), - r#type: TypeExpr::Json, - }, - }) -} - -fn merge_request_schema_type_expr(schema: &Value) -> TypeExpr { - match schema.get("type").and_then(Value::as_str) { - Some("string") => TypeExpr::String, - Some("boolean") => TypeExpr::Boolean, - Some("integer") => TypeExpr::Integer, - Some("number") => TypeExpr::Number, - Some("array") => TypeExpr::List { - items: Box::new( - schema - .get("items") - .map(merge_request_schema_type_expr) - .unwrap_or(TypeExpr::Json), - ), - }, - _ => TypeExpr::Json, - } + Ok(native::declaration(&tool.contract)) } fn merge_request_protocol_failure( @@ -1182,15 +918,16 @@ mod tests { use std::{collections::VecDeque, sync::Mutex}; #[derive(Debug)] - struct RecordingWorkspaceClient { - auth_denied: std::sync::atomic::AtomicBool, - authorizations: Mutex>, + pub(super) struct RecordingWorkspaceClient { + pub(super) auth_denied: std::sync::atomic::AtomicBool, + pub(super) authorizations: + Mutex>, responses: Mutex>, - requests: Mutex>, + pub(super) requests: Mutex>, } impl RecordingWorkspaceClient { - fn new(responses: Vec) -> Self { + pub(super) fn new(responses: Vec) -> Self { Self { auth_denied: std::sync::atomic::AtomicBool::new(false), authorizations: Mutex::new(Vec::new()), @@ -1241,7 +978,7 @@ mod tests { } } - fn response(body: serde_json::Value) -> WorkspaceResponse { + pub(super) fn response(body: serde_json::Value) -> WorkspaceResponse { WorkspaceResponse { status: 200, body: body.to_string(), @@ -1437,7 +1174,7 @@ mod tests { native_merge_request_tools(client.clone(), config) .unwrap() .into_iter() - .map(|tool| (tool.projection.surface, tool.projection.operation)) + .map(|tool| (tool.projection.surface, tool.contract.name)) .collect::>() }; assert_eq!( @@ -1458,7 +1195,7 @@ mod tests { operations(orchestrator), [ (NativeMergeRequestSurface::Item, "read"), - (NativeMergeRequestSurface::Item, "check_readiness"), + (NativeMergeRequestSurface::Item, "readiness"), (NativeMergeRequestSurface::Item, "complete"), ] ); @@ -1485,14 +1222,7 @@ mod tests { .iter() .find(|operation| operation.name == "complete") .unwrap(); - for required in [ - "operation_id", - "approval_event_id", - "target_ref_before", - "target_ref_after", - "strategy", - "resolution", - ] { + for required in ["operation_id", "integration"] { assert!( complete .parameters @@ -1541,44 +1271,36 @@ mod tests { #[test] fn native_input_preserves_null_omission_permissions_and_route_identity() { - let arguments = BTreeMap::from([ - ("body".into(), WipValue::String("approved".into())), - ("findings".into(), json_to_wip(&json!([])).unwrap()), - ("optional".into(), WipValue::Unit), - ]); - let input = native_merge_request_input(&arguments, Some("MR-1"), Some("merge_request_id")) - .unwrap_or_else(|_| panic!("route-bound input should project")); - assert_eq!( - input, - json!({ - "merge_request_id": "MR-1", - "body": "approved", - "findings": [], - "optional": null - }) - ); - let omitted = native_merge_request_input( - &BTreeMap::from([("body".into(), WipValue::String("approved".into()))]), - Some("MR-1"), - Some("merge_request_id"), + let tool = native_merge_request_tools( + Arc::new(TestWorkspaceHttpClient::new("workspace", "http://unused")), + MergeRequestFeatureConfig { + show: true, + ..Default::default() + }, ) - .unwrap_or_else(|_| panic!("omitted optional input should project")); - assert_eq!( - omitted, - json!({"merge_request_id": "MR-1", "body": "approved"}) - ); + .unwrap() + .remove(0); + let arguments = BTreeMap::from([( + "page".into(), + json_to_wip(&json!({"after": {"$case": "null"}})).unwrap(), + )]); + let input = native_merge_request_input(&tool, &arguments, Some("MR-1")) + .unwrap_or_else(|_| panic!("valid native input")); + assert_eq!(input, json!({"merge_request_id": "MR-1", "after": null})); + let omitted = native_merge_request_input(&tool, &BTreeMap::new(), Some("MR-1")) + .unwrap_or_else(|_| panic!("valid native input")); + assert_eq!(omitted, json!({"merge_request_id": "MR-1"})); assert!(matches!( native_merge_request_input( + &tool, &BTreeMap::from([("merge_request_id".into(), WipValue::String("MR-2".into()))]), - Some("MR-1"), - Some("merge_request_id") + Some("MR-1") ), Err(WipOperationError::Protocol(ProtocolError { code: ProtocolErrorCode::InvalidArguments, .. })) )); - let permissions = Some(ToolPermissionConfig { default_action: ToolPermissionAction::Allow, rules: vec![manifest::ToolPermissionRule { @@ -1630,11 +1352,15 @@ mod tests { assert!(!serialized.contains("secret-capability")); assert!(!serialized.contains("capability_token")); - let input = json!({"decision": "approve", "body": "", "findings": []}); + let WipValue::Record(input) = + json_to_wip(&json!({"decision": "approve", "body": "", "findings": []})).unwrap() + else { + unreachable!() + }; let rejected = execute_native_merge_request_tool( &tool, &None, - input.clone(), + &input, Some("MR-2"), WipCallContext { execution: ToolExecutionContext::direct(), @@ -1654,7 +1380,7 @@ mod tests { execute_native_merge_request_tool( &tool, &None, - input, + &input, Some("MR-1"), WipCallContext { execution: ToolExecutionContext::direct(), @@ -1936,7 +1662,7 @@ mod tests { let result = execute_native_merge_request_tool( &native, &None, - json!({"merge_request_id":"MR-1"}), + &BTreeMap::new(), Some("MR-1"), WipCallContext { execution: ToolExecutionContext::direct(), diff --git a/crates/worker/src/feature/builtin/merge_request/command.rs b/crates/worker/src/feature/builtin/merge_request/command.rs new file mode 100644 index 000000000..4e6e7ea3a --- /dev/null +++ b/crates/worker/src/feature/builtin/merge_request/command.rs @@ -0,0 +1,304 @@ +//! Shared typed commands, below the independent Tool and native input contracts. +//! +//! Native validation produces normalized command fields, not a Tool invocation. Both adapters +//! enter the same authorization and Workspace request boundary; neither adapter retries dispatch. + +use super::*; + +#[derive(Debug, Deserialize)] +#[serde(tag = "command", rename_all = "snake_case")] +pub(super) enum Command { + Read { + merge_request_id: String, + after: Option, + limit: Option, + }, + Readiness { + merge_request_id: String, + }, + Open { + ticket: String, + repository_key: String, + selector_from: String, + selector_to: String, + #[serde(default)] + summary: String, + }, + Complete { + merge_request_id: String, + operation_id: String, + approval_event_id: String, + target_ref_before: String, + target_ref_after: String, + strategy: MergeStrategyInput, + resolution: MergeResolutionInput, + }, + Review { + decision: ReviewDecisionInput, + #[serde(default)] + body: String, + #[serde(default)] + findings: Vec, + }, +} + +impl Command { + /// Tool-mode parsing remains governed by its own DTOs and schemars declarations. + pub(super) fn from_tool(kind: Kind, input: &str) -> Result { + Ok(match kind { + Kind::Show => { + let v: ShowMergeRequestInput = parse(input)?; + Self::Read { + merge_request_id: v.merge_request_id, + after: v.after, + limit: v.limit, + } + } + Kind::Readiness => { + let v: MergeRequestInput = parse(input)?; + Self::Readiness { + merge_request_id: v.merge_request_id, + } + } + Kind::Open => { + let v: OpenMergeRequestInput = parse(input)?; + Self::Open { + ticket: v.ticket, + repository_key: v.repository_key, + selector_from: v.selector_from, + selector_to: v.selector_to, + summary: v.summary, + } + } + Kind::Complete => { + let v: CompleteMergeRequestInput = parse(input)?; + Self::Complete { + merge_request_id: v.merge_request_id, + operation_id: v.operation_id, + approval_event_id: v.approval_event_id, + target_ref_before: v.target_ref_before, + target_ref_after: v.target_ref_after, + strategy: v.strategy, + resolution: v.resolution, + } + } + Kind::Review => { + let v: ReviewMergeRequestInput = parse(input)?; + Self::Review { + decision: v.decision, + body: v.body, + findings: v.findings, + } + } + }) + } + + pub(super) fn kind(&self) -> Kind { + match self { + Self::Read { .. } => Kind::Show, + Self::Readiness { .. } => Kind::Readiness, + Self::Open { .. } => Kind::Open, + Self::Complete { .. } => Kind::Complete, + Self::Review { .. } => Kind::Review, + } + } +} + +/// The single domain dispatch boundary. Backend authorization precedes every data request. +pub(super) fn execute( + client: Arc, + command: Command, +) -> Result { + let kind = command.kind(); + let executor = Executor { client, kind }; + let ws = executor.client.workspace_id().ok_or_else(|| { + ToolError::ExecutionFailed("Merge Request operations require Workspace identity".into()) + })?; + let (method, path, body) = match command { + Command::Read { + merge_request_id, + after, + limit, + } => { + nonempty_named("merge_request_id", &merge_request_id)?; + if limit.is_some_and(|limit| !(1..=200).contains(&limit)) { + return Err(ToolError::InvalidArgument( + "limit must be between 1 and 200".into(), + )); + } + return executor.show_merge_request(ws, &merge_request_id, after, limit); + } + Command::Readiness { merge_request_id } => { + nonempty_named("merge_request_id", &merge_request_id)?; + executor.authorize( + "merge_request.readiness", + PermissionResource::MergeRequest { + workspace_id: ws.into(), + merge_request_id: merge_request_id.clone(), + }, + )?; + ( + WorkspaceRequestMethod::Get, + format!( + "/api/w/{ws}/merge-requests/{}/readiness", + encode_path_segment(&merge_request_id) + ), + None, + ) + } + Command::Open { + ticket, + repository_key, + selector_from, + selector_to, + summary, + } => { + nonempty(&ticket)?; + executor.authorize( + "merge_request.open", + PermissionResource::Ticket { + workspace_id: ws.into(), + ticket_id: ticket.clone(), + }, + )?; + ( + WorkspaceRequestMethod::Post, + format!( + "/api/w/{ws}/tickets/{}/merge-request", + encode_path_segment(&ticket) + ), + Some( + json!({"repository_key":repository_key,"selector_from":selector_from,"selector_to":selector_to,"summary":summary}), + ), + ) + } + Command::Complete { + merge_request_id, + operation_id, + approval_event_id, + target_ref_before, + target_ref_after, + strategy, + resolution, + } => { + nonempty_named("merge_request_id", &merge_request_id)?; + executor.authorize( + "merge_request.complete", + PermissionResource::MergeRequest { + workspace_id: ws.into(), + merge_request_id: merge_request_id.clone(), + }, + )?; + ( + WorkspaceRequestMethod::Post, + format!( + "/api/w/{ws}/merge-requests/{}/complete", + encode_path_segment(&merge_request_id) + ), + Some( + json!({"operation_id":operation_id,"approval_event_id":approval_event_id,"target_ref_before":target_ref_before,"target_ref_after":target_ref_after,"strategy":match strategy{MergeStrategyInput::FastForward=>"fast_forward",MergeStrategyInput::Merge=>"merge"},"resolution":match resolution{MergeResolutionInput::None=>"none",MergeResolutionInput::Clean=>"clean",MergeResolutionInput::ConflictsResolved=>"conflicts_resolved"}}), + ), + ) + } + Command::Review { + decision, + body, + findings, + } => { + let ctx = executor.client.reviewer_context().ok_or_else(|| { + ToolError::ExecutionFailed( + "Review submit requires injected Reviewer capability".into(), + ) + })?; + executor.authorize( + "merge_request.review", + PermissionResource::MergeRequest { + workspace_id: ws.into(), + merge_request_id: ctx.merge_request_id.clone(), + }, + )?; + ( + WorkspaceRequestMethod::Post, + format!( + "/api/w/{ws}/merge-requests/{}/reviews", + encode_path_segment(&ctx.merge_request_id) + ), + Some( + json!({"decision":match decision{ReviewDecisionInput::Approve=>"approve",ReviewDecisionInput::RequestChanges=>"request_changes"},"body":body,"findings":findings.into_iter().map(|f|json!({"severity":f.severity,"code":f.code,"path":f.path,"line":f.line,"body":f.body})).collect::>() }), + ), + ) + } + }; + let req = match body { + Some(v) => WorkspaceRequest::json(method, path, v.to_string()), + None => WorkspaceRequest::get(path), + }; + let res = executor + .client + .execute(req) + .map_err(|e| ToolError::ExecutionFailed(e.to_string()))?; + if !res.is_success() { + return Err(api_error("Merge Request API", &res)); + } + Ok(ToolOutput { + summary: kind.name().into(), + content: Some(res.body), + attachments: vec![], + }) +} + +struct Executor { + client: Arc, + kind: Kind, +} + +impl Executor { + fn authorize(&self, operation: &str, resource: PermissionResource) -> Result<(), ToolError> { + self.client.authorize_operation(operation, &resource) + .map_err(|error| { + let denied = matches!(&error, crate::worker::WorkspaceClientError::Request(message) + if message.contains("HTTP status 403") || message.contains("HTTP status 401")); + ToolError::ExecutionFailed(format!("Merge Request operation authorization failed before dispatch: {operation}; resource={resource:?}; denied={denied}")) + }) + } + fn show_merge_request( + &self, + workspace_id: &str, + merge_request_id: &str, + after: Option, + limit: Option, + ) -> Result { + self.authorize( + "merge_request.read", + PermissionResource::MergeRequest { + workspace_id: workspace_id.into(), + merge_request_id: merge_request_id.into(), + }, + )?; + let merge_request_id = encode_path_segment(merge_request_id); + let mut path = format!("/api/w/{workspace_id}/merge-requests/{merge_request_id}"); + let mut query = Vec::new(); + if let Some(after) = after { + query.push(format!("after={after}")); + } + if let Some(limit) = limit { + query.push(format!("limit={limit}")); + } + if !query.is_empty() { + path.push('?'); + path.push_str(&query.join("&")); + } + let response = self + .client + .execute(WorkspaceRequest::get(path)) + .map_err(|error| ToolError::ExecutionFailed(error.to_string()))?; + 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![], + }) + } +} diff --git a/crates/worker/src/feature/builtin/merge_request/native.rs b/crates/worker/src/feature/builtin/merge_request/native.rs new file mode 100644 index 000000000..db908fd79 --- /dev/null +++ b/crates/worker/src/feature/builtin/merge_request/native.rs @@ -0,0 +1,236 @@ +//! Native v2 input contracts and their domain-command adapter. +//! +//! v1 exposed flattened Tool-shaped fields and `check_readiness`. v2 deliberately has no aliases: +//! consumers must Inspect the current Object and then its Interface again, use Ticket Entries, +//! group repository/page/integration records, and invoke `readiness`. Tool mode is unchanged. +//! +//! Examples below show Invoke's JSON binding, not Tool JSON. Substitute the actual allocated +//! Ticket collection path discovered from the current registry (it need not be `/tickets`). +//! +//! ```text +//! collection.open({ticket: "/tickets/T-797", +//! repository: {key: "main", source: "work/T-797", target: "develop"}}) +//! item.read({page: {after: {$case: "value", value: 7}, +//! limit: {$case: "value", value: 25}}}) +//! item.readiness({}) +//! item.review({decision: "request_changes", findings: [ +//! {severity: "major", path: {$case: "value", value: "src/main.rs"}, +//! line: {$case: "value", value: 12}, code: {$case: "null"}, body: "Explain the defect"}]}) +//! item.complete({operation_id: "integration-1", integration: { +//! approval_event_id: "approved-1", target: {before: "target-1", after: "result-1"}, +//! strategy: "fast_forward", resolution: "clean"}}) +//! ``` +//! +//! Optional fields may be omitted; only Nullable shapes accept explicit null, represented as +//! `{$case: "null"}` or `{$case: "value", value: T}` because WIP has no nullable primitive. +//! Plain JSON null is not accepted. Omitted summary/body/findings use domain defaults. Backend +//! remains authoritative for candidate freshness, exact approval, target observations, and business +//! eligibility. Page bounds and finding line integer ranges are checked before authorization and +//! dispatch, separately from native type validation. Complete records integration; it never applies +//! Git changes. Unknown dispatched outcomes are not retried. + +use super::*; +pub(super) use crate::feature::builtin::native_contract::Contract; +use crate::feature::builtin::native_contract::{Destination, Input, ReferenceKind, Shape}; + +/// Owned, per-mount roots keep handlers independent of registry lifetime. Empty means that +/// collection was not allocated; reference conversion then fails closed, with no default route. +#[derive(Clone, Default)] +pub(super) struct ReferenceRoots { + ticket: String, + merge_request: String, + objective: String, +} + +pub(super) fn reference_roots(registry: &WipMountRegistry) -> ReferenceRoots { + ReferenceRoots { + ticket: registry.namespace_root("ticket").unwrap_or_default().into(), + objective: registry + .namespace_root("objective") + .unwrap_or_default() + .into(), + merge_request: registry + .namespace_root("merge-request") + .unwrap_or_default() + .into(), + } +} + +fn field(name: &'static str, required: bool, ty: Shape) -> Input { + Input { + name, + required, + ty, + destination: Destination::Same, + } +} + +fn renamed(name: &'static str, required: bool, ty: Shape, destination: &'static str) -> Input { + Input { + name, + required, + ty, + destination: Destination::Rename(destination), + } +} + +fn flattened(name: &'static str, required: bool, fields: Vec) -> Input { + Input { + name, + required, + ty: Shape::Record(fields), + destination: Destination::Flatten, + } +} + +fn nullable(ty: Shape) -> Shape { + Shape::Nullable(Box::new(ty)) +} + +pub(super) fn contract(kind: Kind) -> Contract { + let (name, summary, parameters) = match kind { + Kind::Open => ( + "open", + "Open or reuse a Ticket-linked repository candidate. ticket is an Entry in the current Ticket collection; repository.source/target are selectors, not asserted commit evidence. Backend validates linkage and authority.", + vec![ + field("ticket", true, Shape::Reference(ReferenceKind::Ticket)), + flattened( + "repository", + true, + vec![ + renamed("key", true, Shape::String, "repository_key"), + renamed("source", true, Shape::String, "selector_from"), + renamed("target", true, Shape::String, "selector_to"), + ], + ), + field("summary", false, Shape::String), + ], + ), + Kind::Show => ( + "read", + "Read this route-bound MR and a bounded thread page. page.after is exclusive (0..9007199254740991); page.limit is 1..200. Omit page for defaults. Optional page fields use null/value unions when explicitly present; omitted fields preserve Backend defaults.", + vec![flattened( + "page", + false, + vec![ + field("after", false, nullable(Shape::Integer)), + field("limit", false, nullable(Shape::Integer)), + ], + )], + ), + Kind::Readiness => ( + "readiness", + "Check current review and exact-source integration eligibility for this MR; no integration or Ticket conclusion is performed.", + vec![], + ), + Kind::Review => ( + "review", + "Record an injected Reviewer's decision on this exact MR. decision is required. Omit body/findings for empty defaults; finding severity/body are required, optional code/path/line use null/value unions, and a present line is 0..4294967295. Backend validates candidate freshness; model inputs never carry a capability.", + vec![ + field( + "decision", + true, + Shape::Enum(vec!["approve", "request_changes"]), + ), + field("body", false, Shape::String), + field( + "findings", + false, + Shape::List(Box::new(Shape::Record(vec![ + field( + "severity", + true, + Shape::Enum(vec!["blocker", "major", "minor", "note"]), + ), + field("code", false, nullable(Shape::String)), + field("path", false, nullable(Shape::String)), + field("line", false, nullable(Shape::Integer)), + field("body", true, Shape::String), + ]))), + ), + ], + ), + Kind::Complete => ( + "complete", + "Record an already-applied exact-source integration using an observed approval event and target before/after refs. All integration fields are required. operation_id is the stable replay key; Backend rechecks exact approval/target observations and integration authority. No Git operation, automatic retry, or Ticket closure occurs.", + vec![ + field("operation_id", true, Shape::String), + flattened( + "integration", + true, + vec![ + field("approval_event_id", true, Shape::String), + flattened( + "target", + true, + vec![ + renamed("before", true, Shape::String, "target_ref_before"), + renamed("after", true, Shape::String, "target_ref_after"), + ], + ), + field("strategy", true, Shape::Enum(vec!["fast_forward", "merge"])), + field( + "resolution", + true, + Shape::Enum(vec!["none", "clean", "conflicts_resolved"]), + ), + ], + ), + ], + ), + }; + Contract { + name, + summary, + parameters, + } +} + +pub(super) fn declaration(contract: &Contract) -> OperationDeclaration { + contract.declaration() +} + +pub(super) fn convert( + contract: &Contract, + arguments: &BTreeMap, + roots: &ReferenceRoots, +) -> Result { + contract.convert( + arguments, + &crate::feature::builtin::native_contract::ReferenceRoots { + ticket: &roots.ticket, + merge_request: &roots.merge_request, + objective: &roots.objective, + }, + ) +} + +pub(super) fn command(kind: Kind, mut input: Value) -> Result { + if kind == Kind::Show + && input + .get("after") + .and_then(Value::as_u64) + .is_some_and(|after| after > 9_007_199_254_740_991) + { + return Err(merge_request_protocol_failure( + ProtocolErrorCode::InvalidArguments, + "page.after exceeds the lossless integer range", + )); + } + input + .as_object_mut() + .expect("native contract produces a record") + .insert("command".into(), Value::String(contract(kind).name.into())); + serde_json::from_value(input).map_err(|error| { + merge_request_protocol_failure( + ProtocolErrorCode::InvalidArguments, + format!("native command: {error}"), + ) + }) +} + +#[cfg(test)] +mod tests; + +#[cfg(test)] +mod domain_tests; 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 new file mode 100644 index 000000000..87586056e --- /dev/null +++ b/crates/worker/src/feature/builtin/merge_request/native/domain_tests.rs @@ -0,0 +1,72 @@ +//! Normal Tool and native adapters deliberately have different inputs but identical domain dispatch. + +use super::super::tests::{RecordingWorkspaceClient, response}; +use super::tests::{arguments, roots, runtime}; +use super::*; + +#[tokio::test] +async fn tool_and_native_open_contracts_produce_the_same_authorized_domain_request() { + let client = Arc::new(RecordingWorkspaceClient::new(vec![ + response(json!({"merge_request_id":"MR-1"})), + response(json!({"merge_request_id":"MR-1"})), + ])); + let tool = MergeRequestTool { + client: client.clone(), + kind: Kind::Open, + }; + tool.execute(r#"{"ticket":"T-797","repository_key":"main","selector_from":"work","selector_to":"develop","summary":"candidate"}"#, ToolExecutionContext::direct()).await.unwrap(); + let runtime = runtime(client.clone()); + runtime.invoke("/changes".into(), crate::wip::root_reference(MERGE_REQUEST_COLLECTION_INTERFACE), "open".into(), + json!({"ticket":"/cases/T-797","repository":{"key":"main","source":"work","target":"develop"},"summary":"candidate"}), ToolExecutionContext::direct()).await.unwrap(); + let requests = client.requests.lock().unwrap(); + assert_eq!(requests.len(), 2); + assert_eq!(requests[0].path, requests[1].path); + assert_eq!(requests[0].body, requests[1].body); + let authorizations = client.authorizations.lock().unwrap(); + assert_eq!(authorizations.len(), 2); + assert_eq!( + serde_json::to_value(&authorizations[0]).unwrap(), + serde_json::to_value(&authorizations[1]).unwrap() + ); + // Neither adapter accepts the other adapter's public form. + assert!(tool.execute(r#"{"ticket":"/cases/T-797","repository":{"key":"main","source":"work","target":"develop"}}"#, ToolExecutionContext::direct()).await.is_err()); + assert!(convert(&contract(Kind::Open), &arguments(json!({"ticket":"T-797","repository_key":"main","selector_from":"work","selector_to":"develop"})), &roots()).is_err()); +} + +#[tokio::test] +async fn native_backend_conflict_is_a_domain_rejection_not_a_type_rejection_or_retry() { + let client = Arc::new(RecordingWorkspaceClient::new(vec![ + crate::worker::WorkspaceResponse { + status: 409, + body: "approval snapshot is stale".into(), + }, + ])); + let tool = native_merge_request_tools( + client.clone(), + MergeRequestFeatureConfig { + complete: true, + ..Default::default() + }, + ) + .unwrap() + .remove(0); + let input = arguments( + json!({"operation_id":"op-1","integration":{"approval_event_id":"approval-1","target":{"before":"before","after":"after"},"strategy":"merge","resolution":"clean"}}), + ); + let rejected = execute_native_merge_request_tool( + &tool, + &None, + &input, + Some("MR-1"), + WipCallContext { + execution: ToolExecutionContext::direct(), + security_context: "test".into(), + }, + ) + .await; + assert!( + matches!(rejected, Err(WipOperationError::Protocol(ProtocolError { code:ProtocolErrorCode::InvalidArguments, message })) if message.contains("HTTP 409") && message.contains("approval snapshot is stale")) + ); + assert_eq!(client.requests.lock().unwrap().len(), 1); + assert_eq!(client.authorizations.lock().unwrap().len(), 1); +} diff --git a/crates/worker/src/feature/builtin/merge_request/native/tests.rs b/crates/worker/src/feature/builtin/merge_request/native/tests.rs new file mode 100644 index 000000000..1ccfdcef5 --- /dev/null +++ b/crates/worker/src/feature/builtin/merge_request/native/tests.rs @@ -0,0 +1,407 @@ +//! Component/connection tests use the production WIP Host, Client, Inspect and Invoke binding, +//! native contracts and domain dispatcher. Only Workspace HTTP is replaced by the existing MR +//! recording fixture; these tests assert dispatch/authorization, not Backend transaction durability. + +use super::super::tests::{RecordingWorkspaceClient, response}; +use super::*; +use crate::wip::WipRuntime; +use wip_protocol::TypeExpr; + +pub(super) fn arguments(value: Value) -> BTreeMap { + let WipValue::Record(arguments) = json_to_wip(&value).unwrap() else { + panic!("expected arguments") + }; + arguments +} + +pub(super) fn roots() -> ReferenceRoots { + ReferenceRoots { + ticket: "/cases".into(), + merge_request: "/changes".into(), + objective: String::new(), + } +} + +fn all_config() -> MergeRequestFeatureConfig { + MergeRequestFeatureConfig { + show: true, + open: true, + review: true, + readiness_check: true, + complete: true, + } +} + +pub(super) fn runtime(client: Arc) -> WipRuntime { + let mut registry = WipMountRegistry::new(); + registry.allocate_namespace("ticket", "cases").unwrap(); + let route = registry + .allocate_namespace("merge-request", "changes") + .unwrap(); + mount_workspace_http_merge_request_wip(&mut registry, client, all_config(), None, &route) + .unwrap(); + WipRuntime::from_mounts(registry, "mr-native-test".into()).unwrap() +} + +async fn inspect_interface(runtime: &WipRuntime, path: &str) -> String { + let object = runtime.inspect(path.into(), false).await.unwrap(); + let object: Value = serde_json::from_str(object.content.as_deref().unwrap()).unwrap(); + let interface = runtime + .inspect( + object["interfaces"][0]["path"].as_str().unwrap().into(), + false, + ) + .await + .unwrap(); + let interface: Value = serde_json::from_str(interface.content.as_deref().unwrap()).unwrap(); + interface["interface_signature"].as_str().unwrap().into() +} + +#[test] +fn native_contract_exposes_records_enums_nullable_and_entry_without_tool_schema() { + let open = contract(Kind::Open).declaration(); + assert!(matches!(open.parameters[0].r#type, TypeExpr::Entry)); + let TypeExpr::Record { ref fields } = open.parameters[1].r#type else { + panic!("repository record") + }; + assert_eq!( + fields + .iter() + .map(|f| (f.name.as_str(), f.required)) + .collect::>(), + [("key", true), ("source", true), ("target", true)] + ); + let review = contract(Kind::Review).declaration(); + assert!(review.parameters[0].required); + assert!(matches!(review.parameters[0].r#type, TypeExpr::Enum { .. })); + let TypeExpr::List { items } = &review.parameters[2].r#type else { + panic!("finding list") + }; + let TypeExpr::Record { fields } = items.as_ref() else { + panic!("typed finding") + }; + assert!( + fields + .iter() + .find(|f| f.name == "severity") + .unwrap() + .required + ); + assert!(matches!( + fields.iter().find(|f| f.name == "line").unwrap().r#type, + TypeExpr::Union { .. } + )); + assert!(!fields.iter().find(|f| f.name == "line").unwrap().required); + // Tool mode intentionally accepts any severity string for Backend validation. Native has its + // own enum contract; equality with the Tool schema is not the acceptance criterion. + assert_eq!( + Kind::Review.schema()["$defs"]["ReviewFindingInput"]["properties"]["severity"]["type"], + "string" + ); + assert!( + convert( + &contract(Kind::Review), + &arguments( + json!({"decision":"approve","findings":[{"severity":"arbitrary","body":"x"}]}) + ), + &roots() + ) + .is_err() + ); +} + +#[test] +fn native_mapping_groups_repository_and_exact_integration_observations() { + let opened = convert( + &contract(Kind::Open), + &arguments(json!({ + "ticket":"/cases/T-797", "repository":{"key":"main","source":"work","target":"develop"} + })), + &roots(), + ) + .unwrap_or_else(|_| panic!("valid native fixture was rejected")); + assert_eq!( + opened, + json!({"ticket":"T-797","repository_key":"main","selector_from":"work","selector_to":"develop"}) + ); + assert!( + matches!(command(Kind::Open, opened).unwrap_or_else(|_| panic!("valid native command was rejected")), command::Command::Open { ticket, summary, .. } if ticket == "T-797" && summary.is_empty()) + ); + let complete = convert(&contract(Kind::Complete), &arguments(json!({ + "operation_id":"op-1", "integration":{"approval_event_id":"approval-1", "target":{"before":"before","after":"after"}, "strategy":"merge","resolution":"conflicts_resolved"} + })), &roots()).unwrap_or_else(|_| panic!("valid native fixture was rejected")); + assert_eq!( + complete, + json!({"operation_id":"op-1","approval_event_id":"approval-1","target_ref_before":"before","target_ref_after":"after","strategy":"merge","resolution":"conflicts_resolved"}) + ); +} + +#[test] +fn native_omission_and_explicit_null_have_distinct_declared_forms() { + assert_eq!( + convert(&contract(Kind::Show), &BTreeMap::new(), &roots()) + .unwrap_or_else(|_| panic!("valid native fixture was rejected")), + json!({}) + ); + assert_eq!( + convert( + &contract(Kind::Show), + &arguments( + json!({"page":{"after":{"$case":"null"},"limit":{"$case":"value","value":25}}}) + ), + &roots() + ) + .unwrap_or_else(|_| panic!("valid native fixture was rejected")), + json!({"after":null,"limit":25}) + ); + let review = convert(&contract(Kind::Review), &arguments(json!({"decision":"approve", "findings":[{"severity":"note","body":"x","code":{"$case":"null"},"line":{"$case":"value","value":7}}]})), &roots()).unwrap_or_else(|_| panic!("valid native fixture was rejected")); + assert_eq!( + review["findings"][0], + json!({"severity":"note","body":"x","code":null,"line":7}) + ); + for invalid in [ + json!({"page":null}), + json!({"page":{"limit":null}}), + json!({"page":{"limit":25}}), + ] { + assert!(convert(&contract(Kind::Show), &arguments(invalid), &roots()).is_err()); + } + for invalid in [ + json!({"decision":null}), + json!({"decision":"approve","body":null}), + json!({"decision":"approve","findings":null}), + ] { + assert!(convert(&contract(Kind::Review), &arguments(invalid), &roots()).is_err()); + } +} + +#[test] +fn native_references_use_the_allocated_namespace_and_reject_identity_spoofing() { + for ticket in [ + "T-797", + "/tickets/T-797", + "/cases2/T-797", + "/changes/MR-1", + "/cases/T-797/extra", + "/cases/T%2D797", + ] { + assert!(convert(&contract(Kind::Open), &arguments(json!({"ticket":ticket,"repository":{"key":"main","source":"work","target":"develop"}})), &roots()).is_err(), "accepted {ticket}"); + } + assert!( + convert( + &contract(Kind::Show), + &arguments(json!({"merge_request_id":"MR-2"})), + &roots() + ) + .is_err() + ); + assert!( + convert( + &contract(Kind::Review), + &arguments(json!({"decision":"approve","capability_token":"spoof"})), + &roots() + ) + .is_err() + ); +} + +#[tokio::test] +async fn inspect_then_invoke_native_open_read_readiness_and_complete_reaches_shared_domain() { + let client = Arc::new(RecordingWorkspaceClient::new(vec![ + response(json!({"merge_request_id":"MR-1"})), + response(json!({"merge_request_id":"MR-1","thread":[]})), + response(json!({"merge_request_id":"MR-1","ready":true})), + response(json!({"merge_request_id":"MR-1","event_id":"integrated-1"})), + ])); + let runtime = runtime(client.clone()); + let collection = inspect_interface(&runtime, "/changes").await; + assert!(collection.contains("ticket: entry")); + assert!(collection.contains("repository:")); + runtime.invoke("/changes".into(), crate::wip::root_reference(MERGE_REQUEST_COLLECTION_INTERFACE), "open".into(), + json!({"ticket":"/cases/T-797","repository":{"key":"main","source":"work","target":"develop"}}), ToolExecutionContext::direct()).await.unwrap(); + let item = inspect_interface(&runtime, "/changes/MR-1").await; + assert!(item.contains("operation readiness(")); + assert!(item.contains("integration:")); + assert!(item.contains("findings?:")); + assert!(!item.contains("merge_request_id:")); + for (operation, input) in [ + ( + "read", + json!({"page":{"after":{"$case":"value","value":7},"limit":{"$case":"value","value":25}}}), + ), + ("readiness", json!({})), + ( + "complete", + json!({"operation_id":"op-1","integration":{"approval_event_id":"approval-1","target":{"before":"before","after":"after"},"strategy":"fast_forward","resolution":"clean"}}), + ), + ] { + // Read observations update the object's content validator. Explicitly refresh between + // independent operations; Invoke must not retry a dispatched validator rejection. + runtime.inspect("/changes/MR-1".into(), true).await.unwrap(); + runtime + .invoke( + "/changes/MR-1".into(), + crate::wip::root_reference(MERGE_REQUEST_ITEM_INTERFACE), + operation.into(), + input, + ToolExecutionContext::direct(), + ) + .await + .unwrap(); + } + let requests = client.requests.lock().unwrap(); + assert_eq!(requests.len(), 4); + assert_eq!(requests[0].path, "/api/w/ws/tickets/T-797/merge-request"); + assert_eq!( + requests[1].path, + "/api/w/ws/merge-requests/MR-1?after=7&limit=25" + ); + assert_eq!(requests[2].path, "/api/w/ws/merge-requests/MR-1/readiness"); + assert_eq!( + serde_json::from_str::(requests[3].body.as_deref().unwrap()).unwrap(), + json!({"operation_id":"op-1","approval_event_id":"approval-1","target_ref_before":"before","target_ref_after":"after","strategy":"fast_forward","resolution":"clean"}) + ); + assert_eq!( + client + .authorizations + .lock() + .unwrap() + .iter() + .map(|a| a.operation.as_str()) + .collect::>(), + [ + "merge_request.open", + "merge_request.read", + "merge_request.readiness", + "merge_request.complete" + ] + ); +} + +#[tokio::test] +async fn native_contract_and_command_rejections_precede_authorization_and_dispatch() { + let client = Arc::new(RecordingWorkspaceClient::new(vec![])); + let runtime = runtime(client.clone()); + for (operation, input) in [ + ( + "read", + json!({"page":{"limit":{"$case":"value","value":201}}}), + ), + ( + "read", + json!({"page":{"after":{"$case":"value","value":-1}}}), + ), + ("read", json!({"after":7})), + ("read", json!({"merge_request_id":"MR-2"})), + ("check_readiness", json!({})), + ( + "complete", + json!({"operation_id":"op","integration":{"approval_event_id":"approval","target":{"before":"before"},"strategy":"merge","resolution":"clean"}}), + ), + ( + "complete", + json!({"operation_id":"op","integration":{"approval_event_id":"approval","target":{"before":"before","after":"after"},"strategy":"squash","resolution":"clean"}}), + ), + ] { + assert!( + runtime + .invoke( + "/changes/MR-1".into(), + crate::wip::root_reference(MERGE_REQUEST_ITEM_INTERFACE), + operation.into(), + input, + ToolExecutionContext::direct() + ) + .await + .is_err() + ); + } + assert!(client.requests.lock().unwrap().is_empty()); + assert!(client.authorizations.lock().unwrap().is_empty()); +} + +#[tokio::test] +async fn native_review_typed_findings_reaches_only_injected_route_and_never_retries_unknown() { + use crate::worker::{ReviewerChildWorkspaceClient, ReviewerContext}; + let inner = Arc::new(RecordingWorkspaceClient::new(vec![ + response(json!({"event_id":"review-1"})), + crate::worker::WorkspaceResponse { + status: 503, + body: "unavailable".into(), + }, + ])); + let client: Arc = Arc::new(ReviewerChildWorkspaceClient::new( + inner.clone(), + ReviewerContext { + ticket_id: "T-797".into(), + merge_request_id: "MR-1".into(), + }, + "secret-capability".into(), + )); + let runtime = runtime(client); + let signature = inspect_interface(&runtime, "/changes/MR-1").await; + assert!(signature.contains("request_changes")); + assert!(signature.contains("major")); + assert!(!signature.contains("secret-capability")); + let input = json!({"decision":"request_changes","findings":[{"severity":"major","body":"Fix it","path":{"$case":"value","value":"src/main.rs"},"line":{"$case":"value","value":12},"code":{"$case":"null"}}]}); + assert!( + runtime + .invoke( + "/changes/MR-2".into(), + crate::wip::root_reference(MERGE_REQUEST_ITEM_INTERFACE), + "review".into(), + input.clone(), + ToolExecutionContext::direct() + ) + .await + .is_err() + ); + assert!(inner.requests.lock().unwrap().is_empty()); + for invalid in [ + json!({"decision":"approve","findings":[{"severity":"major","body":"x","line":{"$case":"value","value":-1}}]}), + json!({"decision":"approve","findings":[{"severity":"major","body":"x","line":{"$case":"value","value":4294967296_u64}}]}), + json!({"decision":"defer"}), + ] { + assert!( + runtime + .invoke( + "/changes/MR-1".into(), + crate::wip::root_reference(MERGE_REQUEST_ITEM_INTERFACE), + "review".into(), + invalid, + ToolExecutionContext::direct() + ) + .await + .is_err() + ); + } + assert!(inner.authorizations.lock().unwrap().is_empty()); + runtime + .invoke( + "/changes/MR-1".into(), + crate::wip::root_reference(MERGE_REQUEST_ITEM_INTERFACE), + "review".into(), + input.clone(), + ToolExecutionContext::direct(), + ) + .await + .unwrap(); + let failed = runtime + .invoke( + "/changes/MR-1".into(), + crate::wip::root_reference(MERGE_REQUEST_ITEM_INTERFACE), + "review".into(), + input, + ToolExecutionContext::direct(), + ) + .await; + assert!(failed.is_err()); + let requests = inner.requests.lock().unwrap(); + assert_eq!(requests.len(), 2, "unknown dispatch must not retry"); + let body: Value = serde_json::from_str(requests[0].body.as_deref().unwrap()).unwrap(); + assert_eq!( + body["findings"][0], + json!({"severity":"major","body":"Fix it","path":"src/main.rs","line":12,"code":null}) + ); + assert_eq!(body["capability_token"], "secret-capability"); +} diff --git a/crates/worker/src/feature/builtin/native_contract.rs b/crates/worker/src/feature/builtin/native_contract.rs new file mode 100644 index 000000000..31af633b2 --- /dev/null +++ b/crates/worker/src/feature/builtin/native_contract.rs @@ -0,0 +1,920 @@ +//! Native input contracts own both WIP declarations and domain-command conversion. +//! +//! These shapes are authored for native operations, not derived from Tool schemas. WIP validates +//! the generated declaration before destination mapping; authorization and business constraints +//! remain with the domain handler. Conversion neither resolves objects nor grants access. +//! +//! WIP 0.2 represents unions as records with `$case` and an optional `value`, and has no nullable +//! type. Nullable inputs therefore expose an explicit `null | value(T)` union rather than an +//! opaque JSON escape hatch. Omission and explicit null remain distinct. + +use std::collections::BTreeMap; + +use serde_json::{Map, Value as Json}; +use wip_protocol::{ + CallOperationRequest, Documentation, EnumCase, FieldDeclaration, INTERFACE_FORMAT_V1, + InterfaceDescriptor, InterfaceReference, InterfaceTarget, OperationDeclaration, + ParameterDeclaration, ProtocolError, ProtocolErrorCode, ReturnDeclaration, Target, TypeExpr, + UnionCase, Value, +}; + +use crate::wip::WipOperationError; + +#[derive(Clone, Debug)] +pub struct Contract { + pub name: &'static str, + pub summary: &'static str, + pub parameters: Vec, +} + +#[derive(Clone, Debug)] +pub struct Input { + pub name: &'static str, + pub required: bool, + pub ty: Shape, + pub destination: Destination, +} + +#[derive(Clone, Copy, Debug, Default)] +pub enum Destination { + #[default] + Same, + Rename(&'static str), + /// Merge a converted object into its enclosing command, rejecting collisions. + Flatten, +} + +#[derive(Clone, Debug)] +pub enum Shape { + String, + Boolean, + Integer, + Enum(Vec<&'static str>), + Nullable(Box), + List(Box), + Record(Vec), + /// Record payloads merge into `{tag: case}`; other payloads use the `value` field. + Union { + tag: &'static str, + cases: Vec, + }, + /// Payload-free cases become strings; payload cases become `{case: payload}`. + ExternalUnion { + cases: Vec, + }, + Reference(ReferenceKind), +} + +#[derive(Clone, Debug)] +pub struct Case { + pub name: &'static str, + pub payload: Option, +} + +#[derive(Clone, Copy, Debug)] +pub enum ReferenceKind { + Ticket, + MergeRequest, + Objective, +} + +/// Exact collection routes allocated by the caller's current namespace registry. +/// There are deliberately no default paths or process-global namespace assumptions. +#[derive(Clone, Copy, Debug)] +pub struct ReferenceRoots<'a> { + pub ticket: &'a str, + pub merge_request: &'a str, + pub objective: &'a str, +} + +impl Contract { + pub fn declaration(&self) -> OperationDeclaration { + OperationDeclaration { + name: self.name.into(), + documentation: Some(Documentation { + summary: self.summary.into(), + details: None, + }), + parameters: self + .parameters + .iter() + .map(|input| ParameterDeclaration { + name: input.name.into(), + required: input.required, + documentation: input.ty.documentation(), + r#type: input.ty.declaration(), + }) + .collect(), + // Typed result Entries are owned separately from this input contract. + returns: ReturnDeclaration { + documentation: None, + r#type: TypeExpr::Json, + }, + } + } + + pub fn convert( + &self, + arguments: &BTreeMap, + roots: &ReferenceRoots<'_>, + ) -> Result { + let descriptor = InterfaceDescriptor { + format: INTERFACE_FORMAT_V1.into(), + documentation: None, + types: Vec::new(), + operations: vec![self.declaration()], + }; + descriptor.validate().map_err(invalid)?; + // validate_call is the upstream type validator. These inert targets exist only to + // supply its request envelope; no Host, observation, dispatch, or authority is involved. + descriptor + .validate_call(&CallOperationRequest { + target: Target { + path: "/".into(), + validator: None, + }, + interface: InterfaceTarget { + scope_ref: None, + reference: InterfaceReference { + scope: "/".into(), + name: "native.contract/v1".into(), + }, + validator: None, + }, + operation: self.name.into(), + arguments: arguments.clone(), + }) + .map_err(invalid)?; + convert_record(&self.parameters, arguments, roots).map(Json::Object) + } +} + +impl Shape { + fn documentation(&self) -> Option { + let summary = match self { + Self::Reference(ReferenceKind::Ticket) => { + "Ticket Entry: an exact item path under the current Ticket collection" + } + Self::Reference(ReferenceKind::MergeRequest) => { + "Merge Request Entry: an exact item path under the current Merge Request collection" + } + Self::Reference(ReferenceKind::Objective) => { + "Objective Entry: an exact item path under the current Objective collection; leaf O- followed by decimal digits, or an 8–128 character ASCII alphanumeric canonical ID" + } + Self::Nullable(_) => { + "Explicit null uses {$case: null}; a present value uses {$case: value, value: T}" + } + _ => return None, + }; + Some(Documentation { + summary: summary.into(), + details: None, + }) + } + + fn declaration(&self) -> TypeExpr { + match self { + Self::String => TypeExpr::String, + Self::Boolean => TypeExpr::Boolean, + Self::Integer => TypeExpr::Integer, + Self::Enum(cases) => TypeExpr::Enum { + cases: cases + .iter() + .map(|name| EnumCase { + name: (*name).into(), + documentation: None, + }) + .collect(), + }, + Self::Nullable(inner) => TypeExpr::Union { + cases: vec![ + UnionCase { + name: "null".into(), + documentation: None, + payload: None, + }, + UnionCase { + name: "value".into(), + documentation: inner.documentation(), + payload: Some(inner.declaration()), + }, + ], + }, + Self::List(items) => TypeExpr::List { + items: Box::new(items.declaration()), + }, + Self::Record(fields) => TypeExpr::Record { + fields: fields + .iter() + .map(|input| FieldDeclaration { + name: input.name.into(), + required: input.required, + documentation: input.ty.documentation(), + r#type: input.ty.declaration(), + }) + .collect(), + }, + Self::Union { cases, .. } | Self::ExternalUnion { cases } => TypeExpr::Union { + cases: cases + .iter() + .map(|case| UnionCase { + name: case.name.into(), + documentation: None, + payload: case.payload.as_ref().map(Self::declaration), + }) + .collect(), + }, + Self::Reference(_) => TypeExpr::Entry, + } + } + + fn convert( + &self, + value: &Value, + roots: &ReferenceRoots<'_>, + ) -> Result { + match (self, value) { + (Self::String | Self::Enum(_), Value::String(value)) => Ok(Json::String(value.clone())), + (Self::Boolean, Value::Boolean(value)) => Ok(Json::Bool(*value)), + (Self::Integer, Value::Integer(value)) => Ok(Json::from(*value)), + (Self::Reference(kind), Value::String(path)) => { + kind.identity(path, roots).map(|id| Json::String(id.into())) + } + (Self::Record(fields), Value::Record(values)) => { + convert_record(fields, values, roots).map(Json::Object) + } + (Self::List(item), Value::List(values)) => values + .iter() + .map(|value| item.convert(value, roots)) + .collect::, _>>() + .map(Json::Array), + (Self::Nullable(inner), Value::Record(fields)) => match union_case(fields)? { + "null" => Ok(Json::Null), + "value" => inner.convert(union_payload(fields)?, roots), + _ => Err(invalid("unknown nullable case")), + }, + (Self::ExternalUnion { cases }, Value::Record(fields)) => { + let name = union_case(fields)?; + let case = cases + .iter() + .find(|case| case.name == name) + .ok_or_else(|| invalid("unknown union case"))?; + match &case.payload { + None => Ok(Json::String(name.into())), + Some(payload) => { + let converted = payload.convert(union_payload(fields)?, roots)?; + let mut output = Map::new(); + insert(&mut output, name, converted)?; + Ok(Json::Object(output)) + } + } + } + (Self::Union { tag, cases }, Value::Record(fields)) => { + let name = union_case(fields)?; + let case = cases + .iter() + .find(|case| case.name == name) + .ok_or_else(|| invalid("unknown union case"))?; + let mut output = Map::new(); + insert(&mut output, tag, Json::String(name.into()))?; + if let Some(payload) = &case.payload { + let converted = payload.convert(union_payload(fields)?, roots)?; + if let Json::Object(fields) = converted { + for (name, value) in fields { + insert(&mut output, &name, value)?; + } + } else { + insert(&mut output, "value", converted)?; + } + } + Ok(Json::Object(output)) + } + _ => Err(invalid("value does not match the declared native shape")), + } + } +} + +fn union_case(fields: &BTreeMap) -> Result<&str, WipOperationError> { + match fields.get("$case") { + Some(Value::String(case)) => Ok(case), + _ => Err(invalid("union requires a string $case")), + } +} + +fn union_payload(fields: &BTreeMap) -> Result<&Value, WipOperationError> { + fields + .get("value") + .ok_or_else(|| invalid("union case requires a value payload")) +} + +fn convert_record( + fields: &[Input], + values: &BTreeMap, + roots: &ReferenceRoots<'_>, +) -> Result, WipOperationError> { + let mut output = Map::new(); + for field in fields { + let Some(value) = values.get(field.name) else { + continue; + }; + let value = field.ty.convert(value, roots)?; + match field.destination { + Destination::Same => insert(&mut output, field.name, value)?, + Destination::Rename(name) => insert(&mut output, name, value)?, + Destination::Flatten => { + let Json::Object(fields) = value else { + return Err(invalid(format!( + "flattened field `{}` must convert to an object", + field.name + ))); + }; + for (name, value) in fields { + insert(&mut output, &name, value)?; + } + } + } + } + Ok(output) +} + +fn insert( + output: &mut Map, + name: &str, + value: Json, +) -> Result<(), WipOperationError> { + if output.contains_key(name) { + return Err(invalid(format!("destination collision at `{name}`"))); + } + output.insert(name.into(), value); + Ok(()) +} + +impl ReferenceKind { + fn identity<'a>( + self, + path: &'a str, + roots: &ReferenceRoots<'_>, + ) -> Result<&'a str, WipOperationError> { + let root = match self { + Self::Ticket => roots.ticket, + Self::MergeRequest => roots.merge_request, + Self::Objective => roots.objective, + }; + wip_protocol::validate_path(root).map_err(invalid)?; + wip_protocol::validate_path(path).map_err(invalid)?; + let leaf = path + .strip_prefix(root) + .and_then(|suffix| suffix.strip_prefix('/')) + .filter(|leaf| !leaf.contains('/')) + .ok_or_else(|| invalid("reference must name an item in its exact collection"))?; + // Keep the dynamic resolvers' route-leaf grammar, without accepting bare identities, + // encoded separators, descendants, prefix siblings, or cross-namespace references. + let valid = match self { + Self::Ticket | Self::Objective => { + let prefix = if matches!(self, Self::Ticket) { + "T-" + } else { + "O-" + }; + leaf.strip_prefix(prefix).is_some_and(|sequence| { + !sequence.is_empty() && sequence.bytes().all(|byte| byte.is_ascii_digit()) + }) || ((8..=128).contains(&leaf.len()) + && leaf.bytes().all(|byte| byte.is_ascii_alphanumeric())) + } + Self::MergeRequest => { + !leaf.is_empty() + && leaf.len() <= 128 + && leaf != "." + && leaf != ".." + && leaf.bytes().all(|byte| { + byte.is_ascii_alphanumeric() || matches!(byte, b'-' | b'_' | b'.' | b'~') + }) + } + }; + if !valid { + return Err(invalid("reference has an invalid route identity")); + } + Ok(leaf) + } +} + +fn invalid(message: impl std::fmt::Display) -> WipOperationError { + WipOperationError::Protocol(ProtocolError { + code: ProtocolErrorCode::InvalidArguments, + message: format!("native contract: {message}"), + }) +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + const ROOTS: ReferenceRoots<'_> = ReferenceRoots { + ticket: "/workspace/cases", + merge_request: "/workspace/changes", + objective: "/workspace/goals", + }; + + fn field(name: &'static str, required: bool, ty: Shape) -> Input { + Input { + name, + required, + ty, + destination: Destination::Same, + } + } + + fn contract(parameters: Vec) -> Contract { + Contract { + name: "apply", + summary: "Apply a native domain command", + parameters, + } + } + + fn fields(values: [(&str, Value); N]) -> BTreeMap { + values + .into_iter() + .map(|(name, value)| (name.into(), value)) + .collect() + } + + fn text(value: &str) -> Value { + Value::String(value.into()) + } + + fn variant(case: &str, payload: Option) -> Value { + let mut values = fields([("$case", text(case))]); + if let Some(payload) = payload { + values.insert("value".into(), payload); + } + Value::Record(values) + } + + fn rejected(contract: &Contract, arguments: BTreeMap) { + let error = contract.convert(&arguments, &ROOTS).unwrap_err(); + assert!( + matches!( + &error, + WipOperationError::Protocol(ProtocolError { + code: ProtocolErrorCode::InvalidArguments, + message, + }) if message.starts_with("native contract:") + ), + "expected a prefixed InvalidArguments error" + ); + } + + fn failed(error: WipOperationError) -> Json { + match error { + WipOperationError::Protocol(error) => panic!("{}", error.message), + _ => panic!("unexpected non-protocol failure"), + } + } + + #[test] + fn union_record_payload_converts_to_tagged_command_and_maps_destinations() { + let contract = contract(vec![Input { + name: "principal", + required: true, + ty: Shape::Union { + tag: "kind", + cases: vec![ + Case { + name: "user", + payload: Some(Shape::Record(vec![Input { + name: "account", + required: true, + ty: Shape::String, + destination: Destination::Rename("account_id"), + }])), + }, + Case { + name: "system", + payload: None, + }, + ], + }, + destination: Destination::Flatten, + }]); + assert!(matches!( + contract.declaration().parameters[0].r#type, + TypeExpr::Union { .. } + )); + let arguments = fields([( + "principal", + variant( + "user", + Some(Value::Record(fields([("account", text("A-1"))]))), + ), + )]); + assert_eq!( + contract.convert(&arguments, &ROOTS).unwrap_or_else(failed), + json!({"kind": "user", "account_id": "A-1"}) + ); + assert_eq!( + contract + .convert(&fields([("principal", variant("system", None))]), &ROOTS) + .unwrap_or_else(failed), + json!({"kind": "system"}) + ); + for bad in [ + variant("unknown", None), + variant("user", None), + variant("user", Some(text("not a record"))), + variant("system", Some(Value::Unit)), + Value::Record(fields([ + ("$case", text("system")), + ("extra", Value::Boolean(true)), + ])), + ] { + rejected(&contract, fields([("principal", bad)])); + } + } + + #[test] + fn enum_lists_and_nested_records_are_closed_before_mapping() { + let contract = contract(vec![field( + "filter", + true, + Shape::Record(vec![field( + "states", + true, + Shape::List(Box::new(Shape::Enum(vec!["open", "closed"]))), + )]), + )]); + let valid = Value::Record(fields([( + "states", + Value::List(vec![text("open"), text("closed")]), + )])); + assert_eq!( + contract + .convert(&fields([("filter", valid.clone())]), &ROOTS) + .unwrap_or_else(failed), + json!({"filter": {"states": ["open", "closed"]}}) + ); + for bad in [ + Value::Record(fields([("states", Value::List(vec![text("queued")]))])), + Value::Record(fields([("states", Value::Unit)])), + Value::Record(fields([])), + Value::Record(fields([ + ("states", Value::List(vec![])), + ("undeclared", text("hidden")), + ])), + ] { + rejected(&contract, fields([("filter", bad)])); + } + rejected( + &contract, + fields([("filter", valid), ("extra", Value::Unit)]), + ); + } + + #[test] + fn optional_nullable_preserves_omission_null_and_present_value() { + let contract = contract(vec![field( + "note", + false, + Shape::Nullable(Box::new(Shape::String)), + )]); + let declaration = contract.declaration(); + assert!(!declaration.parameters[0].required); + assert!(matches!( + &declaration.parameters[0].r#type, + TypeExpr::Union { cases } if cases[0].name == "null" + && cases[0].payload.is_none() + && cases[1].name == "value" + && cases[1].payload == Some(TypeExpr::String) + )); + assert_eq!( + contract.convert(&fields([]), &ROOTS).unwrap_or_else(failed), + json!({}) + ); + for (value, expected) in [ + (variant("null", None), json!({"note": null})), + ( + variant("value", Some(text("hello"))), + json!({"note": "hello"}), + ), + ] { + assert_eq!( + contract + .convert(&fields([("note", value)]), &ROOTS) + .unwrap_or_else(failed), + expected + ); + } + for bad in [ + Value::Unit, + text("hello"), + variant("null", Some(text("unexpected"))), + variant("value", None), + variant("value", Some(Value::Unit)), + ] { + rejected(&contract, fields([("note", bad)])); + } + let nonnullable = contract_with_string(); + rejected(&nonnullable, fields([("value", Value::Unit)])); + rejected(&nonnullable, fields([])); + } + + fn contract_with_string() -> Contract { + contract(vec![field("value", true, Shape::String)]) + } + + #[test] + fn scalar_union_payload_converts_without_losing_case() { + let contract = contract(vec![field( + "reason", + true, + Shape::Union { + tag: "reason", + cases: vec![Case { + name: "other", + payload: Some(Shape::String), + }], + }, + )]); + assert_eq!( + contract + .convert( + &fields([("reason", variant("other", Some(text("explanation"))))]), + &ROOTS, + ) + .unwrap_or_else(failed), + json!({"reason": {"reason": "other", "value": "explanation"}}) + ); + rejected( + &contract, + fields([("reason", variant("other", Some(Value::Unit)))]), + ); + } + + #[test] + fn external_union_converts_to_typed_ticket_closure_reasons() { + use ticket::TicketClosureReason; + + let contract = contract(vec![field( + "reason", + true, + Shape::ExternalUnion { + cases: vec![ + Case { + name: "completed", + payload: None, + }, + Case { + name: "not_planned", + payload: None, + }, + Case { + name: "duplicate", + payload: None, + }, + Case { + name: "unknown", + payload: None, + }, + Case { + name: "other", + payload: Some(Shape::String), + }, + ], + }, + )]); + assert!(matches!( + contract.declaration().parameters[0].r#type, + TypeExpr::Union { .. } + )); + for (value, expected_json, expected_reason) in [ + ( + variant("completed", None), + json!("completed"), + TicketClosureReason::Completed, + ), + ( + variant("not_planned", None), + json!("not_planned"), + TicketClosureReason::NotPlanned, + ), + ( + variant("duplicate", None), + json!("duplicate"), + TicketClosureReason::Duplicate, + ), + ( + variant("unknown", None), + json!("unknown"), + TicketClosureReason::Unknown, + ), + ( + variant("other", Some(text("explanation"))), + json!({"other": "explanation"}), + TicketClosureReason::Other("explanation".into()), + ), + ] { + let converted = contract + .convert(&fields([("reason", value)]), &ROOTS) + .unwrap_or_else(failed); + assert_eq!(converted["reason"], expected_json); + let reason: TicketClosureReason = + serde_json::from_value(converted["reason"].clone()).unwrap(); + assert_eq!(reason, expected_reason); + } + for bad in [ + variant("unknown_case", None), + variant("completed", Some(Value::Unit)), + variant("other", None), + variant("other", Some(Value::Boolean(true))), + Value::Record(fields([ + ("$case", text("completed")), + ("extra", text("hidden")), + ])), + ] { + rejected(&contract, fields([("reason", bad)])); + } + } + + #[test] + fn objective_entries_use_exact_configured_collection_and_route_leaf_grammar() { + let contract = contract(vec![field( + "linked", + true, + Shape::Reference(ReferenceKind::Objective), + )]); + let parameter = &contract.declaration().parameters[0]; + assert_eq!(parameter.r#type, TypeExpr::Entry); + assert!( + parameter + .documentation + .as_ref() + .unwrap() + .summary + .contains("8–128") + ); + for identity in ["O-42", "00001M60CK1PM"] { + assert_eq!( + contract + .convert( + &fields([("linked", text(&format!("{}/{identity}", ROOTS.objective)))]), + &ROOTS + ) + .unwrap_or_else(failed), + json!({"linked": identity}) + ); + } + for bad in [ + "/workspace/cases/O-42", + "/workspace/changes/O-42", + "/other/goals/O-42", + "/workspace/goals-extra/O-42", + "/workspace/goals/O-42/child", + "/workspace/goals/O-42/", + "/workspace/goals//O-42", + "/workspace/goals/%4f-42", + "/workspace/goals/O-no", + "/workspace/goals/T-42", + "/workspace/goals/short", + "/workspace/goals/..", + "/workspace/goals", + "O-42", + ] { + rejected(&contract, fields([("linked", text(bad))])); + } + let moved = ReferenceRoots { + objective: "/moved/goals", + ..ROOTS + }; + assert!( + contract + .convert(&fields([("linked", text("/workspace/goals/O-42"))]), &moved) + .is_err() + ); + assert_eq!( + contract + .convert(&fields([("linked", text("/moved/goals/O-42"))]), &moved) + .unwrap_or_else(failed), + json!({"linked": "O-42"}) + ); + } + + #[test] + fn entry_references_require_exact_caller_namespace_collection_and_valid_leaf() { + let contract = contract(vec![ + field("ticket", true, Shape::Reference(ReferenceKind::Ticket)), + field( + "change", + true, + Shape::Reference(ReferenceKind::MergeRequest), + ), + ]); + let arguments = fields([ + ("ticket", text("/workspace/cases/T-42")), + ("change", text("/workspace/changes/MR-9")), + ]); + assert!(matches!( + contract.declaration().parameters[0].r#type, + TypeExpr::Entry + )); + assert_eq!( + contract.convert(&arguments, &ROOTS).unwrap_or_else(failed), + json!({"ticket": "T-42", "change": "MR-9"}) + ); + for (field, invalid_path) in [ + ("ticket", "/workspace/changes/T-42"), + ("ticket", "/other/cases/T-42"), + ("ticket", "/workspace/cases-extra/T-42"), + ("ticket", "/workspace/cases/T-42/child"), + ("ticket", "/workspace/cases/MR-9"), + ("ticket", "/workspace/cases/T-no"), + ("ticket", "/workspace/cases"), + ("ticket", "T-42"), + ("ticket", "/workspace/cases//T-42"), + ("ticket", "/workspace/cases/T-42/"), + ("ticket", "/workspace/cases/%54-42"), + ("change", "/workspace/cases/MR-9"), + ("change", "/workspace/changes/.."), + ("change", "/workspace/changes/MR%2f9"), + ("change", "/workspace/changes/MR-9?query"), + ] { + let mut invalid = arguments.clone(); + invalid.insert(field.into(), text(invalid_path)); + rejected(&contract, invalid); + } + let moved_roots = ReferenceRoots { + ticket: "/moved/cases", + merge_request: ROOTS.merge_request, + objective: ROOTS.objective, + }; + assert!(contract.convert(&arguments, &moved_roots).is_err()); + let mut moved = arguments; + moved.insert("ticket".into(), text("/moved/cases/T-42")); + assert_eq!( + contract + .convert(&moved, &moved_roots) + .unwrap_or_else(failed)["ticket"], + "T-42" + ); + } + + #[test] + fn scalar_validation_rejects_wrong_types_and_unsafe_integers() { + for (shape, good, bad) in [ + (Shape::String, text("x"), Value::Integer(1)), + (Shape::Boolean, Value::Boolean(true), text("true")), + (Shape::Integer, Value::Integer(1), Value::Number(1.5)), + ( + Shape::Integer, + Value::Integer(0), + Value::Integer(wip_protocol::MAX_SAFE_INTEGER + 1), + ), + ] { + let contract = contract(vec![field("value", true, shape)]); + assert!(contract.convert(&fields([("value", good)]), &ROOTS).is_ok()); + rejected(&contract, fields([("value", bad)])); + } + } + + #[test] + fn destination_collisions_fail_instead_of_overwriting_command_fields() { + let contract = contract(vec![ + field("id", true, Shape::String), + Input { + name: "details", + required: true, + ty: Shape::Record(vec![field("id", true, Shape::String)]), + destination: Destination::Flatten, + }, + ]); + rejected( + &contract, + fields([ + ("id", text("outer")), + ("details", Value::Record(fields([("id", text("inner"))]))), + ]), + ); + let tagged = contract_with_tag_collision(); + rejected( + &tagged, + fields([( + "choice", + variant("case", Some(Value::Record(fields([("kind", text("bad"))])))), + )]), + ); + } + + fn contract_with_tag_collision() -> Contract { + contract(vec![field( + "choice", + true, + Shape::Union { + tag: "kind", + cases: vec![Case { + name: "case", + payload: Some(Shape::Record(vec![field("kind", true, Shape::String)])), + }], + }, + )]) + } +} diff --git a/crates/worker/src/feature/builtin/ticket.rs b/crates/worker/src/feature/builtin/ticket.rs index 806d0a8ff..b13a6f207 100644 --- a/crates/worker/src/feature/builtin/ticket.rs +++ b/crates/worker/src/feature/builtin/ticket.rs @@ -8,10 +8,11 @@ //! //! The native projection exposes `/tickets` and one immediate child segment. Items accept canonical //! `T-*` or canonical internal IDs, never resolving Objective keys or deeper suffixes as Tickets. -//! Item descriptors omit subject `ticket`/`id`, reconstructing original Tool arguments only from the -//! resolved route. Related/dependency Tickets remain separate inputs resolved by the Backend. -//! Operations come from regular Tools actually enabled by `TicketFeatureAccess`, retaining original -//! schemas and permission identities. WIP adds neither a broader business catalog nor another workflow. +//! Item subjects come exclusively from the route; related targets remain explicit typed inputs. +//! The `native` module owns the native action contracts, descriptor and input conversion. Enabled domain +//! actions retain the Profile publication ceiling and operation-specific permission identities; +//! neither native presence nor Feature activation grants Backend authority. The Tool and native +//! input boundaries converge on shared domain execution without inspecting each other's schemas. //! //! # Independent judgments and delegation //! @@ -35,11 +36,13 @@ //! //! Fetching canonical references after mutations uses failure classes acknowledging that writes may //! already have succeeded. HTTP failures at that stage are not misclassified as "unexecuted". -//! Optional arguments distinguish omission from explicit null for original JSON Schema/permission -//! evaluation; defaults use the same implementation as regular Tools. +//! Optional native arguments distinguish omission from the explicitly declared nullable union; +//! defaults and business validation belong to the shared domain executor. Refresh v1 observations +//! to obtain v2 contracts; never reinterpret or automatically replay an old mutation. mod assignment_public; mod assignment_tracking; +mod native; mod notification_diagnostics; mod tags; @@ -62,8 +65,7 @@ use ticket::{ }; use wip_protocol::{ Documentation, INTERFACE_FORMAT_V1, InterfaceDescriptor, Object, OperationDeclaration, - ParameterDeclaration, ProtocolError, ProtocolErrorCode, ReturnDeclaration, TypeExpr, - Value as WipValue, + ProtocolError, ProtocolErrorCode, Value as WipValue, }; use crate::feature::{ @@ -75,7 +77,7 @@ use crate::permission::permission_action_for; use crate::wip::{ WipCallContext, WipDynamicItem, WipDynamicItemResolver, WipDynamicMount, WipMountError, WipMountRegistry, WipNamespaceRoute, WipOperationError, WipOperationHandler, - WipOperationOutput, WipProjection, WipProjectionKind, json_to_wip, wip_to_json, + WipOperationOutput, WipProjection, WipProjectionKind, json_to_wip, }; use crate::worker::{WorkspaceClient, WorkspaceRequest, WorkspaceRequestMethod}; use agen::tool::{Tool, ToolError, ToolExecutionContext, ToolMeta, ToolOutput}; @@ -248,13 +250,31 @@ impl Tool for WorkspaceTicketReadTool { input: &str, _context: ToolExecutionContext, ) -> Result { + let command = match self.kind { + WorkspaceTicketReadKind::Query => WorkspaceTicketReadCommand::Query( + serde_json::from_str(input) + .map_err(|e| ToolError::InvalidArgument(e.to_string()))?, + ), + WorkspaceTicketReadKind::Show => WorkspaceTicketReadCommand::Show( + serde_json::from_str(input) + .map_err(|e| ToolError::InvalidArgument(e.to_string()))?, + ), + }; + self.execute_domain(command) + } +} + +enum WorkspaceTicketReadCommand { + Query(WorkspaceQueryTicketInput), + Show(WorkspaceShowTicketInput), +} +impl WorkspaceTicketReadTool { + fn execute_domain(&self, command: WorkspaceTicketReadCommand) -> Result { let workspace_id = self.client.workspace_id().ok_or_else(|| { ToolError::InvalidArgument("Workspace Ticket reads require workspace identity".into()) })?; - let (path, body, target) = match self.kind { - WorkspaceTicketReadKind::Query => { - let input: WorkspaceQueryTicketInput = serde_json::from_str(&input) - .map_err(|error| ToolError::InvalidArgument(error.to_string()))?; + let (path, body, target) = match command { + WorkspaceTicketReadCommand::Query(input) => { for (name, ids) in [ ("tag_ids", &input.tag_ids), ("tag_ids_any", &input.tag_ids_any), @@ -269,9 +289,7 @@ impl Tool for WorkspaceTicketReadTool { None, ) } - WorkspaceTicketReadKind::Show => { - let input: WorkspaceShowTicketInput = serde_json::from_str(&input) - .map_err(|error| ToolError::InvalidArgument(error.to_string()))?; + WorkspaceTicketReadCommand::Show(input) => { if input.id.trim().is_empty() { return Err(ToolError::InvalidArgument( "ShowTicket.id must not be empty".into(), @@ -412,11 +430,19 @@ struct WorkspaceTicketDecisionTool { #[async_trait] impl Tool for WorkspaceTicketDecisionTool { async fn execute(&self, input: &str, _: ToolExecutionContext) -> Result { + let value = + serde_json::from_str(input).map_err(|e| ToolError::InvalidArgument(e.to_string()))?; + self.execute_domain(value) + } +} +impl WorkspaceTicketDecisionTool { + fn execute_domain( + &self, + value: WorkspaceTicketCaseUpdateInput, + ) -> Result { self.client.workspace_id().ok_or_else(|| { ToolError::InvalidArgument("Ticket decisions require Workspace identity".into()) })?; - let value: WorkspaceTicketCaseUpdateInput = serde_json::from_str(input) - .map_err(|error| ToolError::InvalidArgument(error.to_string()))?; for (name, value) in [ ("ticket", value.ticket.as_str()), ("operation_key", value.operation_key.as_str()), @@ -1681,8 +1707,8 @@ impl TicketBackend for WorkspaceHttpTicketBackend { } } -const TICKET_COLLECTION_INTERFACE: &str = "yoi.ticket/collection/v1"; -const TICKET_ITEM_INTERFACE: &str = "yoi.ticket/item/v1"; +const TICKET_COLLECTION_INTERFACE: &str = "yoi.ticket/collection/v2"; +const TICKET_ITEM_INTERFACE: &str = "yoi.ticket/item/v2"; #[derive(Clone, Copy, Debug, Eq, PartialEq)] enum NativeTicketSurface { @@ -1876,10 +1902,8 @@ fn native_ticket_operation(tool_name: &str) -> Option { #[derive(Clone)] struct NativeTicketTool { name: String, - schema: Value, - tool: Arc, + client: Arc, projection: NativeTicketOperation, - description: String, } pub(crate) fn enabled_ticket_definitions( @@ -1953,31 +1977,20 @@ fn native_ticket_tools( client: Arc, access: TicketFeatureAccess, ) -> Result, WipMountError> { - enabled_ticket_definitions(client, access) - .into_iter() - .map(|definition| { - let (meta, tool) = definition(); - let projection = native_ticket_operation(&meta.name).ok_or_else(|| { - WipMountError::InvalidProjection { + access + .tool_names() + .iter() + .map(|name| { + let projection = + native_ticket_operation(name).ok_or_else(|| WipMountError::InvalidProjection { route: "/tickets".into(), - message: format!( - "enabled Ticket tool `{}` has no native projection", - meta.name - ), - } - })?; - jsonschema::validator_for(&meta.input_schema).map_err(|error| { - WipMountError::InvalidProjection { - route: "/tickets".into(), - message: format!("Ticket tool schema cannot be retained: {error}"), - } - })?; + message: format!("enabled Ticket action '{name}' has no native contract"), + })?; + let _ = native::contract(projection.operation); Ok(NativeTicketTool { - name: meta.name, - schema: meta.input_schema, - tool, + name: (*name).into(), + client: client.clone(), projection, - description: meta.description, }) }) .collect() @@ -2003,6 +2016,14 @@ pub fn mount_workspace_http_ticket_wip( ) -> Result<(), WipMountError> { let collection_route = namespace_route.root().to_string(); let tools = native_ticket_tools(client, access)?; + let merge_request_route = registry + .namespace_root("merge-request") + .unwrap_or("") + .to_string(); + let objective_route = registry + .namespace_root("objective") + .unwrap_or("") + .to_string(); let claimed_tools = tools .iter() .map(|tool| tool.name.as_str()) @@ -2029,7 +2050,11 @@ pub fn mount_workspace_http_ticket_wip( capability: "ticket:collection".into(), kind: WipProjectionKind::Native, object: Object { - name: "tickets".into(), + name: collection_route + .rsplit('/') + .next() + .unwrap_or_default() + .into(), description: Some( "Authoritative Ticket collection through scoped Backend authority".into(), ), @@ -2044,6 +2069,8 @@ pub fn mount_workspace_http_ticket_wip( tools: operation_map(collection_tools), permissions: permissions.clone(), collection_route: collection_route.clone(), + merge_request_route: merge_request_route.clone(), + objective_route: objective_route.clone(), observations: Arc::clone(&observations), }), })?; @@ -2063,6 +2090,8 @@ pub fn mount_workspace_http_ticket_wip( tools: operation_map(item_tools), permissions, collection_route: collection_route.clone(), + merge_request_route: merge_request_route.clone(), + objective_route: objective_route.clone(), observations, }), })?; @@ -2081,6 +2110,8 @@ struct TicketCollectionWipHandler { tools: HashMap, permissions: Option, collection_route: String, + merge_request_route: String, + objective_route: String, observations: TicketObservations, } @@ -2093,7 +2124,15 @@ impl WipOperationHandler for TicketCollectionWipHandler { context: WipCallContext, ) -> Result { let tool = self.tools.get(operation).ok_or_else(operation_not_found)?; - let input = native_ticket_input(arguments, None, tool.projection.identity_field)?; + let input = native::input( + operation, + arguments, + None, + tool.projection.identity_field, + &self.collection_route, + &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 { @@ -2121,6 +2160,8 @@ struct TicketItemResolver { tools: HashMap, permissions: Option, collection_route: String, + merge_request_route: String, + objective_route: String, observations: TicketObservations, } @@ -2151,6 +2192,9 @@ impl WipDynamicItemResolver for TicketItemResolver { tools: self.tools.clone(), permissions: self.permissions.clone(), ticket_reference: item_reference.into(), + collection_route: self.collection_route.clone(), + merge_request_route: self.merge_request_route.clone(), + objective_route: self.objective_route.clone(), observations: Arc::clone(&self.observations), }), }) @@ -2161,6 +2205,9 @@ struct TicketItemWipHandler { tools: HashMap, permissions: Option, ticket_reference: String, + collection_route: String, + merge_request_route: String, + objective_route: String, observations: TicketObservations, } @@ -2173,10 +2220,14 @@ impl WipOperationHandler for TicketItemWipHandler { context: WipCallContext, ) -> Result { let tool = self.tools.get(operation).ok_or_else(operation_not_found)?; - let input = native_ticket_input( + let input = native::input( + operation, arguments, Some(&self.ticket_reference), tool.projection.identity_field, + &self.collection_route, + &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)?; @@ -2204,53 +2255,80 @@ async fn execute_native_ticket_tool( input: Value, context: WipCallContext, ) -> Result { - let validator = jsonschema::validator_for(&tool.schema).map_err(|error| { - protocol_failure( - ProtocolErrorCode::Internal, - format!("stored Ticket tool schema is invalid: {error}"), - ) - })?; - if let Err(error) = validator.validate(&input) { - return Err(protocol_failure( - ProtocolErrorCode::InvalidArguments, - format!("Ticket operation input violates its typed Tool schema: {error}"), - )); - } authorize_native_ticket(permissions, &tool.name, &input)?; - let input = serde_json::to_string(&input).map_err(|error| { - protocol_failure(ProtocolErrorCode::InvalidArguments, error.to_string()) - })?; - tool.tool - .execute(&input, context.execution) + execute_ticket_domain(tool.client.clone(), &tool.name, input, context.execution) .await .map_err(map_ticket_tool_error) } -fn native_ticket_input( - arguments: &BTreeMap, - bound_ticket: Option<&str>, - identity_field: Option<&str>, -) -> Result { - if bound_ticket.is_some() && (arguments.contains_key("ticket") || arguments.contains_key("id")) +async fn execute_ticket_domain( + client: Arc, + name: &str, + command: Value, + context: ToolExecutionContext, +) -> Result { + if name == "QueryTicket" || name == "ShowTicket" { + let kind = if name == "QueryTicket" { + WorkspaceTicketReadKind::Query + } else { + WorkspaceTicketReadKind::Show + }; + let command = if name == "QueryTicket" { + WorkspaceTicketReadCommand::Query( + serde_json::from_value(command) + .map_err(|e| ToolError::InvalidArgument(e.to_string()))?, + ) + } else { + WorkspaceTicketReadCommand::Show( + serde_json::from_value(command) + .map_err(|e| ToolError::InvalidArgument(e.to_string()))?, + ) + }; + return WorkspaceTicketReadTool { client, kind }.execute_domain(command); + } + if name == "TicketCaseUpdate" { + let command = serde_json::from_value(command) + .map_err(|e| ToolError::InvalidArgument(e.to_string()))?; + return WorkspaceTicketDecisionTool { + client, + kind: WorkspaceTicketDecisionKind::CaseUpdate, + } + .execute_domain(command); + } + if let Some(result) = + assignment_public::execute_native(client.clone(), name, command.clone(), context.clone()) + .await { - return Err(protocol_failure( - ProtocolErrorCode::InvalidArguments, - "the subject Ticket is bound exclusively by the object route", - )); + return result; } - let mut input = serde_json::Map::new(); - for (name, value) in arguments { - input.insert( - name.clone(), - wip_to_json(value).map_err(|message| { - protocol_failure(ProtocolErrorCode::InvalidArguments, message) - })?, - ); + if let Some(result) = + assignment_tracking::execute_native(client.clone(), name, command.clone(), context.clone()) + .await + { + return result; } - if let (Some(ticket), Some(field)) = (bound_ticket, identity_field) { - input.insert(field.into(), Value::String(ticket.into())); + if let Some(result) = + tags::execute_native(client.clone(), name, command.clone(), context.clone()).await + { + return result; } - Ok(Value::Object(input)) + if let Some(result) = notification_diagnostics::execute_native( + client.clone(), + name, + command.clone(), + context.clone(), + ) + .await + { + return result; + } + ticket::tool::execute_ticket_command( + TicketToolBackend::new(WorkspaceHttpTicketBackend::new(client)), + name, + command, + context, + ) + .await } fn authorize_native_ticket( @@ -2328,9 +2406,10 @@ fn ticket_api_error_message(status: u16, body: &str) -> String { fn map_ticket_tool_error(error: ToolError) -> WipOperationError { match error { - ToolError::InvalidArgument(message) => { - protocol_failure(ProtocolErrorCode::InvalidArguments, message) - } + ToolError::InvalidArgument(message) => protocol_failure( + ProtocolErrorCode::InvalidArguments, + format!("Ticket domain rejection: {message}"), + ), ToolError::Cancelled(output) => WipOperationError::Cancelled(output), ToolError::Interrupted(output) => WipOperationError::Interrupted(output), ToolError::StructuredConflict { code, message } => protocol_failure( @@ -2352,9 +2431,10 @@ fn map_ticket_tool_error(error: ToolError) -> WipOperationError { match status { Some(401 | 403) => protocol_failure(ProtocolErrorCode::PermissionDenied, message), Some(404) => protocol_failure(ProtocolErrorCode::NotFound, message), - Some(400 | 409 | 422) => { - protocol_failure(ProtocolErrorCode::InvalidArguments, message) - } + Some(400 | 409 | 422) => protocol_failure( + ProtocolErrorCode::InvalidArguments, + format!("Ticket domain rejection: {message}"), + ), _ => WipOperationError::OutcomeUnknown(message), } } @@ -2534,74 +2614,7 @@ fn ticket_descriptor( fn ticket_operation_declaration( tool: &NativeTicketTool, ) -> Result { - let properties = tool - .schema - .get("properties") - .and_then(Value::as_object) - .ok_or_else(|| WipMountError::InvalidProjection { - route: "/tickets".into(), - message: format!("Ticket tool `{}` schema has no properties", tool.name), - })?; - let required = tool - .schema - .get("required") - .and_then(Value::as_array) - .into_iter() - .flatten() - .filter_map(Value::as_str) - .collect::>(); - let parameters = properties - .iter() - .filter(|(name, _)| tool.projection.identity_field != Some(name.as_str())) - .map(|(name, schema)| ParameterDeclaration { - name: name.clone(), - required: required.contains(name.as_str()), - documentation: schema - .get("description") - .and_then(Value::as_str) - .map(|summary| Documentation { - summary: summary.into(), - details: None, - }), - r#type: schema_type_expr(schema), - }) - .collect(); - Ok(OperationDeclaration { - name: tool.projection.operation.into(), - documentation: Some(Documentation { - summary: tool.description.clone(), - details: Some(format!( - "Delegates to the existing `{}` typed Ticket operation and retains its exact JSON Schema validation.", - tool.name - )), - }), - parameters, - returns: ReturnDeclaration { - documentation: Some(Documentation { - summary: "Authoritative bounded Ticket operation result".into(), - details: None, - }), - r#type: TypeExpr::Json, - }, - }) -} - -fn schema_type_expr(schema: &Value) -> TypeExpr { - match schema.get("type").and_then(Value::as_str) { - Some("string") => TypeExpr::String, - Some("boolean") => TypeExpr::Boolean, - Some("integer") => TypeExpr::Integer, - Some("number") => TypeExpr::Number, - Some("array") => TypeExpr::List { - items: Box::new( - schema - .get("items") - .map(schema_type_expr) - .unwrap_or(TypeExpr::Json), - ), - }, - _ => TypeExpr::Json, - } + Ok(native::contract(tool.projection.operation).declaration()) } fn protocol_failure(code: ProtocolErrorCode, message: impl Into) -> WipOperationError { @@ -2974,14 +2987,14 @@ mod tests { .into_iter() .find(|tool| tool.projection.operation == "read") .unwrap(); - let output = read - .tool - .execute( - r#"{"id":"T-739","event_cursor":"10"}"#, - ToolExecutionContext::direct(), - ) - .await - .unwrap(); + let output = execute_ticket_domain( + read.client.clone(), + &read.name, + json!({"id":"T-739","event_cursor":"10"}), + ToolExecutionContext::direct(), + ) + .await + .unwrap(); assert_eq!( ticket_tool_output_json(output) .unwrap_or_else(|_| panic!("Ticket read returned invalid JSON"))["targets"], @@ -3286,14 +3299,7 @@ mod tests { .iter() .any(|p| p.name == "ticket" || p.name == "actor" || p.name == "author") ); - for field in [ - "operation_key", - "expected_content_digest", - "expected_state", - "expected_closure_operation_key", - "action", - "explanation", - ] { + for field in ["operation_key", "observed", "action", "explanation"] { assert!( case_update .parameters @@ -3302,13 +3308,14 @@ mod tests { ); } let summary = &case_update.documentation.as_ref().unwrap().summary; - assert!(summary.contains("Closed does not imply success")); - assert!(summary.contains("not real judgment")); - assert!(summary.contains("Return without conclusion need not call this tool")); + assert!(summary.contains("Only completed asserts fulfillment")); + assert!(summary.contains("never automatically replay")); let resolver = TicketItemResolver { tools: operation_map(tools), permissions: None, collection_route: "/tickets".into(), + merge_request_route: "/merge-requests".into(), + objective_route: "/objectives".into(), observations: Arc::new(Mutex::new(TicketObservationState::default())), }; record_ticket_observation( @@ -3320,8 +3327,13 @@ mod tests { let bound = resolver.resolve("T-739").unwrap(); let before = bound.object.validator; let other = resolver.resolve("T-740").unwrap().object.validator; - let mut input = decision_input(); - input.as_object_mut().unwrap().remove("ticket"); + let expected = decision_input(); + let input = json!({ + "operation_key":expected["operation_key"], + "observed":{"content_digest":expected["expected_content_digest"],"state":expected["expected_state"],"closure_key":{"$case":"null"}}, + "action":{"$case":"close","value":{"reason":{"$case":"not_planned"}}}, + "explanation":expected["explanation"] + }); let arguments = input .as_object() .unwrap() @@ -3347,7 +3359,8 @@ mod tests { assert_eq!(requests[0].path, "/workspaces/workspace/tickets/T-739/case"); assert_eq!(requests[1].path, "/api/w/workspace/tickets/T-739/record"); let body: Value = serde_json::from_str(requests[0].body.as_deref().unwrap()).unwrap(); - let mut expected = input; + let mut expected = expected; + expected.as_object_mut().unwrap().remove("ticket"); expected["references"] = json!([]); assert_eq!(body, expected); } @@ -3466,6 +3479,8 @@ mod tests { tools: HashMap::new(), permissions: None, collection_route: "/tickets".into(), + merge_request_route: "/merge-requests".into(), + objective_route: "/objectives".into(), observations, }; assert!(resolver.resolve("T-42").is_some()); @@ -3476,43 +3491,52 @@ mod tests { #[test] fn native_ticket_permission_input_preserves_presence_and_route_identity() { - let arguments = BTreeMap::from([ - ("body".into(), WipValue::String("comment".into())), - ("reason".into(), WipValue::Unit), - ]); - let input = native_ticket_input(&arguments, Some("T-42"), Some("ticket")) - .unwrap_or_else(|_| panic!("route-bound Ticket input should project")); - assert_eq!( - input, - json!({"ticket": "T-42", "body": "comment", "reason": null}) - ); - let omitted = native_ticket_input( - &BTreeMap::from([("body".into(), WipValue::String("comment".into()))]), - Some("T-42"), - Some("ticket"), + let arguments = json_to_wip(&json!({"content":{"title":"New"}})).unwrap(); + let WipValue::Record(arguments) = arguments else { + unreachable!() + }; + let input = native::input( + "create", + &arguments, + None, + None, + "/tickets", + "/merge-requests", + "/objectives", ) - .unwrap_or_else(|_| panic!("omitted optional Ticket input should project")); - assert_eq!(omitted, json!({"ticket": "T-42", "body": "comment"})); - let replaced = native_ticket_input( - &BTreeMap::from([("ticket".into(), WipValue::String("T-99".into()))]), + .unwrap_or_else(|_| panic!("native content")); + assert!(input.get("body").is_none()); + let arguments = BTreeMap::from([ + ("ticket".into(), WipValue::String("T-99".into())), + ("body".into(), WipValue::String("text".into())), + ]); + assert!( + native::input( + "comment", + &arguments, + Some("T-42"), + Some("ticket"), + "/tickets", + "/merge-requests", + "/objectives" + ) + .is_err() + ); + let arguments = BTreeMap::from([("body".into(), WipValue::String("text".into()))]); + let input = native::input( + "comment", + &arguments, Some("T-42"), Some("ticket"), - ); - assert!(matches!( - replaced, - Err(WipOperationError::Protocol(ProtocolError { - code: ProtocolErrorCode::InvalidArguments, - .. - })) - )); - + "/tickets", + "/merge-requests", + "/objectives", + ) + .unwrap_or_else(|_| panic!("native comment")); + assert_eq!(input["ticket"], "T-42"); let permissions = Some(ToolPermissionConfig { - default_action: ToolPermissionAction::Allow, - rules: vec![manifest::ToolPermissionRule { - tool: "TicketComment".into(), - pattern: serde_json::to_string(&input).unwrap(), - action: ToolPermissionAction::Deny, - }], + default_action: ToolPermissionAction::Deny, + rules: Vec::new(), }); assert!(matches!( authorize_native_ticket(&permissions, "TicketComment", &input), @@ -3530,6 +3554,8 @@ mod tests { tools: HashMap::new(), permissions: None, collection_route: "/tickets".into(), + merge_request_route: "/merge-requests".into(), + objective_route: "/objectives".into(), observations: Arc::clone(&observations), }; let unobserved_internal = resolver.resolve("00001TICKET").unwrap().object.validator; @@ -3576,6 +3602,8 @@ mod tests { tools: HashMap::new(), permissions: None, collection_route: "/tickets".into(), + merge_request_route: "/merge-requests".into(), + objective_route: "/objectives".into(), observations: Arc::clone(&observations), }; let response = json!({"ticket": "T-42", "body": "actual content", "state": "open"}); @@ -3613,6 +3641,8 @@ mod tests { tools: HashMap::new(), permissions: None, collection_route: "/tickets".into(), + merge_request_route: "/merge-requests".into(), + objective_route: "/objectives".into(), observations: Arc::clone(&observations), }; let target_v1 = resolver.resolve("T-43").unwrap().object.validator; @@ -4409,14 +4439,16 @@ mod tests { tools: operation_map(tools), permissions: None, collection_route: "/tickets".into(), + merge_request_route: "/merge-requests".into(), + objective_route: "/objectives".into(), observations: Arc::new(Mutex::new(TicketObservationState::default())), }; handler .call( "create", &BTreeMap::from([( - "title".into(), - WipValue::String("Create".into()), + "content".into(), + json_to_wip(&json!({"title":"Create"})).unwrap(), )]), WipCallContext { execution: ToolExecutionContext::direct(), @@ -4824,14 +4856,16 @@ mod tests { ), permissions: None, collection_route: "/tickets".into(), + merge_request_route: "/merge-requests".into(), + objective_route: "/objectives".into(), observations: Arc::new(Mutex::new(TicketObservationState::default())), }; handler .call( "create", &BTreeMap::from([( - "title".into(), - WipValue::String("Authorized create".into()), + "content".into(), + json_to_wip(&json!({"title":"Authorized create"})).unwrap(), )]), WipCallContext { execution: ToolExecutionContext::direct(), @@ -5155,6 +5189,9 @@ mod tests { ), permissions: None, ticket_reference: "T-739".into(), + collection_route: "/tickets".into(), + merge_request_route: "/merge-requests".into(), + objective_route: "/objectives".into(), observations: Arc::new(Mutex::new(TicketObservationState::default())), }; let error = handler diff --git a/crates/worker/src/feature/builtin/ticket/assignment_public.rs b/crates/worker/src/feature/builtin/ticket/assignment_public.rs index 727c67e14..23939d959 100644 --- a/crates/worker/src/feature/builtin/ticket/assignment_public.rs +++ b/crates/worker/src/feature/builtin/ticket/assignment_public.rs @@ -197,6 +197,31 @@ struct PublicAssignmentTool { client: Arc, kind: PublicAssignmentKind, } +// Internal command DTOs belong to the shared execution boundary, not a native schema. +enum Command { + Assign(AssignInput), + Clear(ClearInput), + Assignments(AssignmentsInput), + Diagnostics(DiagnosticsInput), + Answer(AnswerInput), + ImplementationResult(ImplementationResultInput), + ImplementationResultQuery(ImplementationResultQuery), +} + +impl Command { + fn kind(&self) -> PublicAssignmentKind { + match self { + Self::Assign(..) => PublicAssignmentKind::Assign, + Self::Clear(..) => PublicAssignmentKind::Clear, + Self::Assignments(..) => PublicAssignmentKind::Assignments, + Self::Diagnostics(..) => PublicAssignmentKind::Diagnostics, + Self::Answer(..) => PublicAssignmentKind::Answer, + Self::ImplementationResult(..) => PublicAssignmentKind::ImplementationResult, + Self::ImplementationResultQuery(..) => PublicAssignmentKind::ImplementationResultQuery, + } + } +} + #[async_trait] impl Tool for PublicAssignmentTool { async fn execute( @@ -204,253 +229,302 @@ impl Tool for PublicAssignmentTool { input: &str, ctx: ToolExecutionContext, ) -> Result { - self.client - .workspace_id() - .filter(|id| !id.trim().is_empty()) - .ok_or_else(|| { - ToolError::InvalidArgument( - "Assignment tools require Backend Workspace identity; no local fallback".into(), - ) - })?; - let parse = |error: serde_json::Error| ToolError::InvalidArgument(error.to_string()); - let operation = match self.kind { - PublicAssignmentKind::Assign => { - let input: AssignInput = serde_json::from_str(input).map_err(parse)?; - identity("ticket", &input.ticket)?; - identity("operation_id", &input.operation_id)?; - optional_identity("expected_assignment_id", &input.expected_assignment_id)?; - match &input.principal { - server_api::TicketAssignmentPrincipal::User { account_id } => { - identity("account_id", account_id)? - } - server_api::TicketAssignmentPrincipal::Worker { - runtime_id, - worker_id, - } => { - identity("runtime_id", runtime_id)?; - identity("worker_id", worker_id)?; - } - server_api::TicketAssignmentPrincipal::WorkspaceAgent { agent_key } => { - identity("agent_key", agent_key)? - } - } - WorkspaceServerOperation::TicketAssignmentSet { - ticket_id: input.ticket, - role: server_api::TicketAssignmentRole::Orchestrator, - request: server_api::SetTicketRoleAssignmentRequest { - orchestrator_assignment_id: None, - operation_id: input.operation_id, - principal: input.principal, - expected_assignment_id: input.expected_assignment_id, - workdir_bindings: vec![], - }, - } - } - PublicAssignmentKind::Clear => { - let input: ClearInput = serde_json::from_str(input).map_err(parse)?; - identity("ticket", &input.ticket)?; - identity("assignment_id", &input.assignment_id)?; - identity("operation_id", &input.operation_id)?; - WorkspaceServerOperation::TicketAssignmentClear { - ticket_id: input.ticket, - role: server_api::TicketAssignmentRole::Orchestrator, - query: server_api::ClearTicketRoleAssignmentQuery { - operation_id: Some(input.operation_id), - assignment_id: Some(input.assignment_id), - }, - } - } - PublicAssignmentKind::Assignments => { - let mut input: AssignmentsInput = serde_json::from_str(input).map_err(parse)?; - identity("ticket", &input.ticket)?; - optional_identity("after_assignment_id", &input.query.after_assignment_id)?; - input.query.limit = Some(page(input.query.limit)?); - WorkspaceServerOperation::TicketAssignmentsPage { - ticket_id: input.ticket, - query: input.query, - } - } - PublicAssignmentKind::Diagnostics => { - let mut input: DiagnosticsInput = serde_json::from_str(input).map_err(parse)?; - identity("ticket", &input.ticket)?; - identity("assignment_id", &input.query.assignment_id)?; - optional_identity("after_input_event_id", &input.query.after_input_event_id)?; - optional_identity( - "after_start_operation_id", - &input.query.after_start_operation_id, - )?; - input.query.limit = Some(page(input.query.limit)?); - WorkspaceServerOperation::TicketAssignmentDiagnostics { - ticket_id: input.ticket, - query: input.query, - } - } - PublicAssignmentKind::ImplementationResult => { - let input: ImplementationResultInput = - serde_json::from_str(input).map_err(parse)?; - identity("ticket", &input.ticket)?; - // Retry identity belongs to the Host, never model input. Missing trusted - // context must fail closed rather than minting a replacement identity. - identity("tool call_id", &ctx.call_id)?; - let operation_key = format!( - "ticket-implementation-result:{}:{}", - input.ticket, ctx.call_id - ); - identity("operation_key", &operation_key)?; - if input.body.trim().is_empty() - || input.body.len() > 16384 - || input.body.contains('\0') - { - return Err(ToolError::InvalidArgument( - "Result body must be nonblank, NUL-free and at most 16384 bytes".into(), - )); - } - if let Some(mr) = &input.merge_request { - identity("merge_request_id", &mr.merge_request_id)?; - identity("subject_ref", &mr.subject_ref)?; - } - WorkspaceServerOperation::TicketImplementationResult { - ticket_id: input.ticket, - request: server_api::TicketImplementationResultRequest { - operation_key, - kind: input.kind, - body: input.body, - merge_request: input.merge_request, - }, - } - } - PublicAssignmentKind::ImplementationResultQuery => { - let input: ImplementationResultQuery = - serde_json::from_str(input).map_err(parse)?; - identity("ticket", &input.ticket)?; - identity("result_id", &input.result_id)?; - WorkspaceServerOperation::TicketImplementationResultGet { - ticket_id: input.ticket, - result_id: input.result_id, - } - } - PublicAssignmentKind::Answer => { - let input: AnswerInput = serde_json::from_str(input).map_err(parse)?; - identity("ticket", &input.ticket)?; - for (name, value) in [ - ("assignment_id", &input.request.0.assignment_id), - ( - "expected_last_event_id", - &input.request.0.expected_last_event_id, - ), - ("operation_key", &input.request.0.operation_key), - ] { - identity(name, value)?; - } - let body = &input.request.0.body; - if body.trim().is_empty() || body.len() > 16384 || body.contains('\0') { - return Err(ToolError::InvalidArgument( - "Answer body must be nonempty, at most 16384 bytes and without NUL".into(), - )); - } - WorkspaceServerOperation::TicketAssignmentAnswer { - ticket_id: input.ticket, - request: input.request, - } - } - }; - let ticket_id = match &operation { - WorkspaceServerOperation::TicketAssignmentSet { ticket_id, .. } - | WorkspaceServerOperation::TicketAssignmentClear { ticket_id, .. } - | WorkspaceServerOperation::TicketAssignmentsPage { ticket_id, .. } - | WorkspaceServerOperation::TicketAssignmentDiagnostics { ticket_id, .. } - | WorkspaceServerOperation::TicketAssignmentAnswer { ticket_id, .. } - | WorkspaceServerOperation::TicketImplementationResult { ticket_id, .. } - | WorkspaceServerOperation::TicketImplementationResultGet { ticket_id, .. } => { - ticket_id - } - _ => unreachable!("public assignment operation"), - }; - assignment_tracking::authorize_ticket_target( - self.client.as_ref(), - match self.kind { - PublicAssignmentKind::Assign => "ticket.assignment.assign", - PublicAssignmentKind::Clear => "ticket.assignment.clear", - PublicAssignmentKind::Assignments => "ticket.assignment.query", - PublicAssignmentKind::Diagnostics => "ticket.assignment.diagnostics", - PublicAssignmentKind::Answer => "ticket.assignment.answer", - PublicAssignmentKind::ImplementationResult => "ticket.implementation_result.write", - PublicAssignmentKind::ImplementationResultQuery => { - "ticket.implementation_result.read" - } - }, - ticket_id, - )?; - let response = self - .client - .execute_server_operation(operation) - .map_err(|error| ToolError::ExecutionFailed(error.to_string()))?; - if !response.is_success() { - return Err(ToolError::ExecutionFailed(format!( - "Assignment request failed with HTTP status {}: {}", - response.status, - response.body.chars().take(1024).collect::() - ))); - } - if response.body.len() > 1024 * 1024 { - return Err(ToolError::ExecutionFailed( - "Assignment response exceeds 1 MiB; request a smaller page".into(), - )); - } - // Decode the actual public DTO and serialize only its declared fields; no raw - // backend payload/credentials or alternative result authority reaches the model. - let decode = |error: serde_json::Error| { - ToolError::ExecutionFailed(format!( - "Invalid Backend assignment response (outcome unknown after dispatch): {error}" - )) - }; - let content = match self.kind { - PublicAssignmentKind::Assign | PublicAssignmentKind::Clear => serde_json::to_value( - serde_json::from_str::( - &response.body, - ) - .map_err(decode)?, + let command = match self.kind { + PublicAssignmentKind::Assign => Command::Assign( + serde_json::from_str(input) + .map_err(|error| ToolError::InvalidArgument(error.to_string()))?, ), - PublicAssignmentKind::Assignments => { - let result: server_api::TicketAssignmentsPage = - serde_json::from_str(&response.body).map_err(decode)?; - if result.0.items.len() > 100 { - return Err(ToolError::ExecutionFailed( - "Assignment page exceeds 100 items".into(), - )); - } - serde_json::to_value(result) - } - PublicAssignmentKind::Diagnostics => { - let result: server_api::TicketAssignmentDiagnosticsPage = - serde_json::from_str(&response.body).map_err(decode)?; - if result.inputs.len() > 100 || result.starts.len() > 100 { - return Err(ToolError::ExecutionFailed( - "Diagnostic page exceeds 100 items".into(), - )); - } - serde_json::to_value(result) - } - PublicAssignmentKind::ImplementationResult - | PublicAssignmentKind::ImplementationResultQuery => serde_json::to_value( - serde_json::from_str::( - &response.body, - ) - .map_err(decode)?, + PublicAssignmentKind::Clear => Command::Clear( + serde_json::from_str(input) + .map_err(|error| ToolError::InvalidArgument(error.to_string()))?, ), - PublicAssignmentKind::Answer => serde_json::to_value( - serde_json::from_str::(&response.body) - .map_err(decode)?, + PublicAssignmentKind::Assignments => Command::Assignments( + serde_json::from_str(input) + .map_err(|error| ToolError::InvalidArgument(error.to_string()))?, ), - } - .map_err(|error| ToolError::Internal(error.to_string()))?; - Ok(ToolOutput { - summary: self.kind.name().into(), - content: Some(content.to_string()), - attachments: vec![], - }) + PublicAssignmentKind::Diagnostics => Command::Diagnostics( + serde_json::from_str(input) + .map_err(|error| ToolError::InvalidArgument(error.to_string()))?, + ), + PublicAssignmentKind::Answer => Command::Answer( + serde_json::from_str(input) + .map_err(|error| ToolError::InvalidArgument(error.to_string()))?, + ), + PublicAssignmentKind::ImplementationResult => Command::ImplementationResult( + serde_json::from_str(input) + .map_err(|error| ToolError::InvalidArgument(error.to_string()))?, + ), + PublicAssignmentKind::ImplementationResultQuery => Command::ImplementationResultQuery( + serde_json::from_str(input) + .map_err(|error| ToolError::InvalidArgument(error.to_string()))?, + ), + }; + execute_domain(self.client.clone(), command, ctx).await } } + +/// Convert an adapter-owned internal command; never invokes Tool execution or its schema. +pub(super) async fn execute_native( + 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), + "TicketOrchestratorClear" => serde_json::from_value(command).map(Command::Clear), + "TicketAssignmentsQuery" => serde_json::from_value(command).map(Command::Assignments), + "TicketAssignmentDiagnostics" => serde_json::from_value(command).map(Command::Diagnostics), + "TicketAssignmentAnswer" => serde_json::from_value(command).map(Command::Answer), + "TicketImplementationResult" => { + serde_json::from_value(command).map(Command::ImplementationResult) + } + "TicketImplementationResultQuery" => { + serde_json::from_value(command).map(Command::ImplementationResultQuery) + } + _ => return None, + }; + Some(match command { + Ok(command) => execute_domain(client, command, ctx).await, + Err(error) => Err(parse(error)), + }) +} + +async fn execute_domain( + client: Arc, + command: Command, + ctx: ToolExecutionContext, +) -> Result { + let kind = command.kind(); + client + .workspace_id() + .filter(|id| !id.trim().is_empty()) + .ok_or_else(|| { + ToolError::InvalidArgument( + "Assignment tools require Backend Workspace identity; no local fallback".into(), + ) + })?; + let operation = match command { + Command::Assign(input) => { + identity("ticket", &input.ticket)?; + identity("operation_id", &input.operation_id)?; + optional_identity("expected_assignment_id", &input.expected_assignment_id)?; + match &input.principal { + server_api::TicketAssignmentPrincipal::User { account_id } => { + identity("account_id", account_id)? + } + server_api::TicketAssignmentPrincipal::Worker { + runtime_id, + worker_id, + } => { + identity("runtime_id", runtime_id)?; + identity("worker_id", worker_id)?; + } + server_api::TicketAssignmentPrincipal::WorkspaceAgent { agent_key } => { + identity("agent_key", agent_key)? + } + } + WorkspaceServerOperation::TicketAssignmentSet { + ticket_id: input.ticket, + role: server_api::TicketAssignmentRole::Orchestrator, + request: server_api::SetTicketRoleAssignmentRequest { + orchestrator_assignment_id: None, + operation_id: input.operation_id, + principal: input.principal, + expected_assignment_id: input.expected_assignment_id, + workdir_bindings: vec![], + }, + } + } + Command::Clear(input) => { + identity("ticket", &input.ticket)?; + identity("assignment_id", &input.assignment_id)?; + identity("operation_id", &input.operation_id)?; + WorkspaceServerOperation::TicketAssignmentClear { + ticket_id: input.ticket, + role: server_api::TicketAssignmentRole::Orchestrator, + query: server_api::ClearTicketRoleAssignmentQuery { + operation_id: Some(input.operation_id), + assignment_id: Some(input.assignment_id), + }, + } + } + Command::Assignments(mut input) => { + identity("ticket", &input.ticket)?; + optional_identity("after_assignment_id", &input.query.after_assignment_id)?; + input.query.limit = Some(page(input.query.limit)?); + WorkspaceServerOperation::TicketAssignmentsPage { + ticket_id: input.ticket, + query: input.query, + } + } + Command::Diagnostics(mut input) => { + identity("ticket", &input.ticket)?; + identity("assignment_id", &input.query.assignment_id)?; + optional_identity("after_input_event_id", &input.query.after_input_event_id)?; + optional_identity( + "after_start_operation_id", + &input.query.after_start_operation_id, + )?; + input.query.limit = Some(page(input.query.limit)?); + WorkspaceServerOperation::TicketAssignmentDiagnostics { + ticket_id: input.ticket, + query: input.query, + } + } + Command::ImplementationResult(input) => { + identity("ticket", &input.ticket)?; + // Retry identity belongs to the Host, never model input. Missing trusted + // context must fail closed rather than minting a replacement identity. + identity("tool call_id", &ctx.call_id)?; + let operation_key = format!( + "ticket-implementation-result:{}:{}", + input.ticket, ctx.call_id + ); + identity("operation_key", &operation_key)?; + if input.body.trim().is_empty() || input.body.len() > 16384 || input.body.contains('\0') + { + return Err(ToolError::InvalidArgument( + "Result body must be nonblank, NUL-free and at most 16384 bytes".into(), + )); + } + if let Some(mr) = &input.merge_request { + identity("merge_request_id", &mr.merge_request_id)?; + identity("subject_ref", &mr.subject_ref)?; + } + WorkspaceServerOperation::TicketImplementationResult { + ticket_id: input.ticket, + request: server_api::TicketImplementationResultRequest { + operation_key, + kind: input.kind, + body: input.body, + merge_request: input.merge_request, + }, + } + } + Command::ImplementationResultQuery(input) => { + identity("ticket", &input.ticket)?; + identity("result_id", &input.result_id)?; + WorkspaceServerOperation::TicketImplementationResultGet { + ticket_id: input.ticket, + result_id: input.result_id, + } + } + Command::Answer(input) => { + identity("ticket", &input.ticket)?; + for (name, value) in [ + ("assignment_id", &input.request.0.assignment_id), + ( + "expected_last_event_id", + &input.request.0.expected_last_event_id, + ), + ("operation_key", &input.request.0.operation_key), + ] { + identity(name, value)?; + } + let body = &input.request.0.body; + if body.trim().is_empty() || body.len() > 16384 || body.contains('\0') { + return Err(ToolError::InvalidArgument( + "Answer body must be nonempty, at most 16384 bytes and without NUL".into(), + )); + } + WorkspaceServerOperation::TicketAssignmentAnswer { + ticket_id: input.ticket, + request: input.request, + } + } + }; + let ticket_id = match &operation { + WorkspaceServerOperation::TicketAssignmentSet { ticket_id, .. } + | WorkspaceServerOperation::TicketAssignmentClear { ticket_id, .. } + | WorkspaceServerOperation::TicketAssignmentsPage { ticket_id, .. } + | WorkspaceServerOperation::TicketAssignmentDiagnostics { ticket_id, .. } + | WorkspaceServerOperation::TicketAssignmentAnswer { ticket_id, .. } + | WorkspaceServerOperation::TicketImplementationResult { ticket_id, .. } + | WorkspaceServerOperation::TicketImplementationResultGet { ticket_id, .. } => ticket_id, + _ => unreachable!("public assignment operation"), + }; + assignment_tracking::authorize_ticket_target( + client.as_ref(), + match kind { + PublicAssignmentKind::Assign => "ticket.assignment.assign", + PublicAssignmentKind::Clear => "ticket.assignment.clear", + PublicAssignmentKind::Assignments => "ticket.assignment.query", + PublicAssignmentKind::Diagnostics => "ticket.assignment.diagnostics", + PublicAssignmentKind::Answer => "ticket.assignment.answer", + PublicAssignmentKind::ImplementationResult => "ticket.implementation_result.write", + PublicAssignmentKind::ImplementationResultQuery => "ticket.implementation_result.read", + }, + ticket_id, + )?; + let response = client + .execute_server_operation(operation) + .map_err(|error| ToolError::ExecutionFailed(error.to_string()))?; + if !response.is_success() { + return Err(ToolError::ExecutionFailed(format!( + "Assignment request failed with HTTP status {}: {}", + response.status, + response.body.chars().take(1024).collect::() + ))); + } + if response.body.len() > 1024 * 1024 { + return Err(ToolError::ExecutionFailed( + "Assignment response exceeds 1 MiB; request a smaller page".into(), + )); + } + // Decode the actual public DTO and serialize only its declared fields; no raw + // backend payload/credentials or alternative result authority reaches the model. + let decode = |error: serde_json::Error| { + ToolError::ExecutionFailed(format!( + "Invalid Backend assignment response (outcome unknown after dispatch): {error}" + )) + }; + let content = match kind { + PublicAssignmentKind::Assign | PublicAssignmentKind::Clear => serde_json::to_value( + serde_json::from_str::( + &response.body, + ) + .map_err(decode)?, + ), + PublicAssignmentKind::Assignments => { + let result: server_api::TicketAssignmentsPage = + serde_json::from_str(&response.body).map_err(decode)?; + if result.0.items.len() > 100 { + return Err(ToolError::ExecutionFailed( + "Assignment page exceeds 100 items".into(), + )); + } + serde_json::to_value(result) + } + PublicAssignmentKind::Diagnostics => { + let result: server_api::TicketAssignmentDiagnosticsPage = + serde_json::from_str(&response.body).map_err(decode)?; + if result.inputs.len() > 100 || result.starts.len() > 100 { + return Err(ToolError::ExecutionFailed( + "Diagnostic page exceeds 100 items".into(), + )); + } + serde_json::to_value(result) + } + PublicAssignmentKind::ImplementationResult + | PublicAssignmentKind::ImplementationResultQuery => serde_json::to_value( + serde_json::from_str::(&response.body) + .map_err(decode)?, + ), + PublicAssignmentKind::Answer => serde_json::to_value( + serde_json::from_str::(&response.body) + .map_err(decode)?, + ), + } + .map_err(|error| ToolError::Internal(error.to_string()))?; + Ok(ToolOutput { + summary: kind.name().into(), + content: Some(content.to_string()), + attachments: vec![], + }) +} + pub(super) fn definition( client: Arc, kind: PublicAssignmentKind, @@ -467,3 +541,121 @@ pub(super) fn definition( ) }) } + +#[cfg(test)] +mod native_command_tests { + use super::*; + use assignment_tracking::authorization_tests::{AuthorizedClient, ticket}; + + #[tokio::test] + async fn internal_commands_share_exact_authorization_requests_and_host_retry_identity() { + for (kind, input, response) in super::tests::cases() { + let operation = match kind { + PublicAssignmentKind::Assign => "ticket.assignment.assign", + PublicAssignmentKind::Clear => "ticket.assignment.clear", + PublicAssignmentKind::Assignments => "ticket.assignment.query", + PublicAssignmentKind::Diagnostics => "ticket.assignment.diagnostics", + PublicAssignmentKind::Answer => "ticket.assignment.answer", + PublicAssignmentKind::ImplementationResult => "ticket.implementation_result.write", + PublicAssignmentKind::ImplementationResultQuery => { + "ticket.implementation_result.read" + } + }; + let client = AuthorizedClient::new(response); + let ctx = ToolExecutionContext::new("host-call", "batch", 0); + assert!( + execute_native(client.clone(), kind.name(), input.clone(), ctx.clone()) + .await + .unwrap() + .is_err() + ); + assert!(client.requests.lock().unwrap().is_empty()); + client.grant(operation, ticket("T-743")); + let (_, tool) = definition(client.clone(), kind)(); + let ordinary = tool.execute(&input.to_string(), ctx.clone()).await.unwrap(); + let native = execute_native(client.clone(), kind.name(), input.clone(), ctx.clone()) + .await + .unwrap() + .unwrap(); + assert_eq!(native.content, ordinary.content); + { + let requests = client.requests.lock().unwrap(); + assert_eq!( + requests[0], + requests[1], + "{} must preserve the exact request including CAS and replay identities", + kind.name() + ); + if matches!(kind, PublicAssignmentKind::ImplementationResult) { + let body: Value = + serde_json::from_str(requests[1].body.as_deref().unwrap()).unwrap(); + assert_eq!( + body["operation_key"], + "ticket-implementation-result:T-743:host-call" + ); + } + } + let mut other = input.clone(); + other["ticket"] = json!("T-other"); + assert!( + execute_native(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) + .await + .unwrap() + .is_err() + ); + assert_eq!(client.requests.lock().unwrap().len(), 2); + assert!( + execute_native( + client.clone(), + "Unknown", + Value::Null, + ToolExecutionContext::direct() + ) + .await + .is_none() + ); + } + } + + #[tokio::test] + async fn internal_assign_requires_observed_null_and_result_rejects_missing_host_identity() { + let client = AuthorizedClient::new(json!({})); + let mut assign = super::tests::cases().remove(0).1; + assign + .as_object_mut() + .unwrap() + .remove("expected_assignment_id"); + assert!(matches!( + execute_native( + client.clone(), + "TicketOrchestratorAssign", + assign, + ToolExecutionContext::direct() + ) + .await + .unwrap(), + Err(ToolError::InvalidArgument(_)) + )); + let result = json!({"ticket":"T-743", "kind":"result", "body":"Done"}); + assert!(matches!( + execute_native( + client.clone(), + "TicketImplementationResult", + result, + ToolExecutionContext::new("", "batch", 0) + ) + .await + .unwrap(), + Err(ToolError::InvalidArgument(_)) + )); + assert!(client.requests.lock().unwrap().is_empty()); + assert!(client.authorizations.lock().unwrap().is_empty()); + } +} 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 8ab3f159e..05d2f424e 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 @@ -1,4 +1,4 @@ -use super::tests::cases; +use super::tests::{cases, native_args}; use super::*; use assignment_tracking::authorization_tests::{AuthorizedClient, ticket}; @@ -53,15 +53,12 @@ async fn public_operations_require_exact_ticket_grants_on_tool_and_wip() { ), permissions: None, ticket_reference: "T-743".into(), + collection_route: "/tickets".into(), + merge_request_route: "/merge-requests".into(), + objective_route: "/objectives".into(), observations: Arc::new(Mutex::new(TicketObservationState::default())), }; - let args = input - .as_object() - .unwrap() - .iter() - .filter(|(key, _)| key.as_str() != "ticket") - .map(|(key, value)| (key.clone(), json_to_wip(value).unwrap())) - .collect(); + let args = native_args(kind, &input); let projection = native_ticket_operation(kind.name()).unwrap(); handler .call( 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 5927810c5..7381a667c 100644 --- a/crates/worker/src/feature/builtin/ticket/assignment_public/tests.rs +++ b/crates/worker/src/feature/builtin/ticket/assignment_public/tests.rs @@ -81,6 +81,36 @@ pub(super) fn cases() -> Vec<(PublicAssignmentKind, Value, Value)> { ), ] } +// Paired inputs exercise the two public contracts against identical domain requests. +pub(super) fn native_args(kind: PublicAssignmentKind, input: &Value) -> BTreeMap { + let value = match kind { + PublicAssignmentKind::Assign => { + let mut principal = input["principal"].clone(); + let case = principal.as_object_mut().unwrap().remove("kind").unwrap(); + json!({"principal":{"$case":case,"value":principal},"observed":{"assignment_id":if input["expected_assignment_id"].is_null(){json!({"$case":"null"})}else{json!({"$case":"value","value":input["expected_assignment_id"]})}},"operation_id":input["operation_id"]}) + } + PublicAssignmentKind::Clear => { + json!({"assignment_id":input["assignment_id"],"operation_id":input["operation_id"]}) + } + PublicAssignmentKind::Assignments => { + json!({"page":{"cursor":input["after_assignment_id"],"limit":input["limit"]}}) + } + PublicAssignmentKind::Diagnostics => { + json!({"assignment_id":input["assignment_id"],"page":{"after_input_event_id":input["after_input_event_id"],"after_start_operation_id":input["after_start_operation_id"],"limit":input["limit"]}}) + } + PublicAssignmentKind::Answer => { + json!({"observed":{"assignment_id":input["assignment_id"],"expected_last_event_id":input["expected_last_event_id"]},"operation_key":input["operation_key"],"body":input["body"]}) + } + PublicAssignmentKind::ImplementationResult => { + json!({"kind":input["kind"],"body":input["body"]}) + } + PublicAssignmentKind::ImplementationResultQuery => json!({"result_id":input["result_id"]}), + }; + match json_to_wip(&value).unwrap() { + WipValue::Record(fields) => fields, + _ => unreachable!(), + } +} #[tokio::test] async fn public_assignment_schemas_require_observations_and_exclude_model_actor_claims() { for (kind, input, response) in cases() { @@ -737,15 +767,12 @@ async fn implementation_result_wip_and_native_dispatch_preserve_trusted_call_con ), permissions: None, ticket_reference: "T-743".into(), + collection_route: "/tickets".into(), + merge_request_route: "/merge-requests".into(), + objective_route: "/objectives".into(), observations: Arc::new(Mutex::new(TicketObservationState::default())), }; - let args: BTreeMap<_, _> = input - .as_object() - .unwrap() - .iter() - .filter(|(key, _)| key.as_str() != "ticket") - .map(|(key, value)| (key.clone(), json_to_wip(value).unwrap())) - .collect(); + let args = native_args(kind, &input); for call_id in ["call-a", "call-b"] { handler .call( @@ -828,15 +855,13 @@ async fn assignment_wip_binds_ticket_and_preserves_normal_permission_and_cas_inp ), permissions: None, ticket_reference: "T-743".into(), + collection_route: "/tickets".into(), + merge_request_route: "/merge-requests".into(), + objective_route: "/objectives".into(), observations: Arc::new(Mutex::new(TicketObservationState::default())), }; input.as_object_mut().unwrap().remove("ticket"); - let args: BTreeMap<_, _> = input - .as_object() - .unwrap() - .iter() - .map(|(key, value)| (key.clone(), json_to_wip(value).unwrap())) - .collect(); + let args = native_args(kind, &input); let operation = native_ticket_operation(kind.name()).unwrap().operation; handler .call( @@ -882,15 +907,12 @@ async fn assignment_wip_binds_ticket_and_preserves_normal_permission_and_cas_inp rules: vec![], }), ticket_reference: "T-743".into(), + collection_route: "/tickets".into(), + merge_request_route: "/merge-requests".into(), + objective_route: "/objectives".into(), observations: Arc::new(Mutex::new(TicketObservationState::default())), }; - let args = input - .as_object() - .unwrap() - .iter() - .filter(|(k, _)| k.as_str() != "ticket") - .map(|(k, v)| (k.clone(), json_to_wip(v).unwrap())) - .collect(); + let args = native_args(PublicAssignmentKind::Answer, &input); assert!(matches!( denied .call( diff --git a/crates/worker/src/feature/builtin/ticket/assignment_tracking.rs b/crates/worker/src/feature/builtin/ticket/assignment_tracking.rs index 7f7a9b38f..712d0852d 100644 --- a/crates/worker/src/feature/builtin/ticket/assignment_tracking.rs +++ b/crates/worker/src/feature/builtin/ticket/assignment_tracking.rs @@ -137,123 +137,180 @@ struct TrackingTool { kind: TrackingToolKind, } +// Internal command DTOs belong to the shared execution boundary, not a native schema. +enum Command { + Query(QueryInput), + History(HistoryInput), + Update(UpdateInput), +} + +impl Command { + fn kind(&self) -> TrackingToolKind { + match self { + Self::Query(..) => TrackingToolKind::Query, + Self::History(..) => TrackingToolKind::History, + Self::Update(..) => TrackingToolKind::Update, + } + } +} + #[async_trait] impl Tool for TrackingTool { - async fn execute(&self, input: &str, _: ToolExecutionContext) -> Result { - let workspace = self - .client - .workspace_id() - .filter(|id| !id.trim().is_empty()) - .ok_or_else(|| { - ToolError::InvalidArgument( - "Assignment tracking requires Backend Workspace identity; no local fallback" - .into(), - ) - })?; - let prefix = format!("/workspaces/{}/tickets", encode_segment(workspace)); - let parse_error = |error: serde_json::Error| ToolError::InvalidArgument(error.to_string()); - let (method, path, body, ticket_id) = match self.kind { - TrackingToolKind::Query => { - let input: QueryInput = serde_json::from_str(input).map_err(parse_error)?; - required("ticket", &input.ticket)?; - let mut path = format!( + async fn execute( + &self, + input: &str, + ctx: ToolExecutionContext, + ) -> Result { + let command = match self.kind { + TrackingToolKind::Query => Command::Query( + serde_json::from_str(input) + .map_err(|error| ToolError::InvalidArgument(error.to_string()))?, + ), + TrackingToolKind::History => Command::History( + serde_json::from_str(input) + .map_err(|error| ToolError::InvalidArgument(error.to_string()))?, + ), + TrackingToolKind::Update => Command::Update( + serde_json::from_str(input) + .map_err(|error| ToolError::InvalidArgument(error.to_string()))?, + ), + }; + execute_domain(self.client.clone(), command, ctx).await + } +} + +/// Convert an adapter-owned internal command; never invokes Tool execution or its schema. +pub(super) async fn execute_native( + 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), + "TicketAssignmentTrackingHistory" => serde_json::from_value(command).map(Command::History), + "TicketAssignmentTrackingUpdate" => serde_json::from_value(command).map(Command::Update), + _ => return None, + }; + Some(match command { + Ok(command) => execute_domain(client, command, ctx).await, + Err(error) => Err(parse(error)), + }) +} + +async fn execute_domain( + client: Arc, + command: Command, + _ctx: ToolExecutionContext, +) -> Result { + let kind = command.kind(); + let workspace = client + .workspace_id() + .filter(|id| !id.trim().is_empty()) + .ok_or_else(|| { + ToolError::InvalidArgument( + "Assignment tracking requires Backend Workspace identity; no local fallback".into(), + ) + })?; + let prefix = format!("/workspaces/{}/tickets", encode_segment(workspace)); + let (method, path, body, ticket_id) = match command { + Command::Query(input) => { + required("ticket", &input.ticket)?; + let mut path = format!( + "{prefix}/{}/assignment-tracking", + encode_segment(&input.ticket) + ); + if let Some(assignment_id) = input.assignment_id { + required("assignment_id", &assignment_id)?; + path.push_str(&format!( + "?assignment_id={}", + encode_segment(&assignment_id) + )); + } + (WorkspaceRequestMethod::Get, path, None, input.ticket) + } + Command::History(input) => { + required("ticket", &input.ticket)?; + required("assignment_id", &input.assignment_id)?; + let limit = input.limit.unwrap_or(100); + if !(1..=100).contains(&limit) { + return Err(ToolError::InvalidArgument( + "limit must be between 1 and 100".into(), + )); + } + let mut path = format!( + "{prefix}/{}/assignment-tracking/history?assignment_id={}&limit={limit}", + encode_segment(&input.ticket), + encode_segment(&input.assignment_id) + ); + if let Some(after) = input.after_event_id { + required("after_event_id", &after)?; + path.push_str(&format!("&after_event_id={}", encode_segment(&after))); + } + (WorkspaceRequestMethod::Get, path, None, input.ticket) + } + Command::Update(input) => { + for (name, value) in [ + ("ticket", &input.ticket), + ("assignment_id", &input.assignment_id), + ("expected_last_event_id", &input.expected_last_event_id), + ("operation_key", &input.operation_key), + ] { + required(name, value)?; + } + // Unknown fields (including author, role, runtime and worker) are + // rejected before transport; authenticated context belongs to Backend. + let body = json!({"assignment_id": input.assignment_id, "expected_last_event_id": input.expected_last_event_id, "operation_key": input.operation_key, "tracking": input.tracking}); + ( + WorkspaceRequestMethod::Post, + format!( "{prefix}/{}/assignment-tracking", encode_segment(&input.ticket) - ); - if let Some(assignment_id) = input.assignment_id { - required("assignment_id", &assignment_id)?; - path.push_str(&format!( - "?assignment_id={}", - encode_segment(&assignment_id) - )); - } - (WorkspaceRequestMethod::Get, path, None, input.ticket) - } - TrackingToolKind::History => { - let input: HistoryInput = serde_json::from_str(input).map_err(parse_error)?; - required("ticket", &input.ticket)?; - required("assignment_id", &input.assignment_id)?; - let limit = input.limit.unwrap_or(100); - if !(1..=100).contains(&limit) { - return Err(ToolError::InvalidArgument( - "limit must be between 1 and 100".into(), - )); - } - let mut path = format!( - "{prefix}/{}/assignment-tracking/history?assignment_id={}&limit={limit}", - encode_segment(&input.ticket), - encode_segment(&input.assignment_id) - ); - if let Some(after) = input.after_event_id { - required("after_event_id", &after)?; - path.push_str(&format!("&after_event_id={}", encode_segment(&after))); - } - (WorkspaceRequestMethod::Get, path, None, input.ticket) - } - TrackingToolKind::Update => { - let input: UpdateInput = serde_json::from_str(input).map_err(parse_error)?; - for (name, value) in [ - ("ticket", &input.ticket), - ("assignment_id", &input.assignment_id), - ("expected_last_event_id", &input.expected_last_event_id), - ("operation_key", &input.operation_key), - ] { - required(name, value)?; - } - // Unknown fields (including author, role, runtime and worker) are - // rejected before transport; authenticated context belongs to Backend. - let body = json!({"assignment_id": input.assignment_id, "expected_last_event_id": input.expected_last_event_id, "operation_key": input.operation_key, "tracking": input.tracking}); - ( - WorkspaceRequestMethod::Post, - format!( - "{prefix}/{}/assignment-tracking", - encode_segment(&input.ticket) - ), - Some(body.to_string()), - input.ticket, - ) - } - }; - let request = match body { - Some(body) => WorkspaceRequest::json(method, path, body), - None => WorkspaceRequest::get(path), - }; - authorize_ticket_target( - self.client.as_ref(), - match self.kind { - TrackingToolKind::Query => "ticket.assignment.query", - TrackingToolKind::History => "ticket.assignment.history", - TrackingToolKind::Update => "ticket.assignment.update", - }, - &ticket_id, - )?; - let response = self - .client - .execute(request) - .map_err(|error| ToolError::ExecutionFailed(error.to_string()))?; - if !response.is_success() { - let detail: String = response.body.chars().take(1024).collect(); - return Err(ToolError::ExecutionFailed(format!( - "Assignment tracking request failed with HTTP status {}: {}", - response.status, detail - ))); + ), + Some(body.to_string()), + input.ticket, + ) } - if response.body.len() > 1024 * 1024 { - return Err(ToolError::ExecutionFailed( - "Assignment tracking response exceeds 1 MiB; request a smaller history page".into(), - )); - } - let content: Value = serde_json::from_str(&response.body).map_err(|error| { - ToolError::ExecutionFailed(format!( - "Backend returned invalid assignment tracking JSON: {error}" - )) - })?; - Ok(ToolOutput { - summary: self.kind.name().into(), - content: Some(content.to_string()), - attachments: Vec::new(), - }) + }; + let request = match body { + Some(body) => WorkspaceRequest::json(method, path, body), + None => WorkspaceRequest::get(path), + }; + authorize_ticket_target( + client.as_ref(), + match kind { + TrackingToolKind::Query => "ticket.assignment.query", + TrackingToolKind::History => "ticket.assignment.history", + TrackingToolKind::Update => "ticket.assignment.update", + }, + &ticket_id, + )?; + let response = client + .execute(request) + .map_err(|error| ToolError::ExecutionFailed(error.to_string()))?; + if !response.is_success() { + let detail: String = response.body.chars().take(1024).collect(); + return Err(ToolError::ExecutionFailed(format!( + "Assignment tracking request failed with HTTP status {}: {}", + response.status, detail + ))); } + if response.body.len() > 1024 * 1024 { + return Err(ToolError::ExecutionFailed( + "Assignment tracking response exceeds 1 MiB; request a smaller history page".into(), + )); + } + let content: Value = serde_json::from_str(&response.body).map_err(|error| { + ToolError::ExecutionFailed(format!( + "Backend returned invalid assignment tracking JSON: {error}" + )) + })?; + Ok(ToolOutput { + summary: kind.name().into(), + content: Some(content.to_string()), + attachments: Vec::new(), + }) } pub(super) fn definition( @@ -513,3 +570,88 @@ mod tests { assert!(!reads.contains(&"TicketAssignmentTrackingUpdate")); } } + +#[cfg(test)] +mod native_command_tests { + use super::authorization_tests::{AuthorizedClient, ticket}; + use super::*; + + #[tokio::test] + async fn internal_tracking_commands_share_authorization_and_full_cas_payload_with_tools() { + for (kind, operation, input) in [ + ( + TrackingToolKind::Query, + "ticket.assignment.query", + json!({"ticket":"T-A", "assignment_id":"old/delegation?"}), + ), + ( + TrackingToolKind::History, + "ticket.assignment.history", + json!({"ticket":"T-A", "assignment_id":"a", "after_event_id":"event/7?", "limit":5}), + ), + ( + TrackingToolKind::Update, + "ticket.assignment.update", + json!({"ticket":"T-A", "assignment_id":"a", "expected_last_event_id":"observed", "operation_key":"stable", "tracking":{"status":"done", "waiting_reason":null, "progress":"Compared", "next_action":null, "result":"Returned"}}), + ), + ] { + let client = + AuthorizedClient::new(json!({"assignment_id":"a", "last_event_id":"next"})); + assert!( + execute_native( + client.clone(), + kind.name(), + input.clone(), + ToolExecutionContext::direct() + ) + .await + .unwrap() + .is_err() + ); + assert!(client.requests.lock().unwrap().is_empty()); + client.grant(operation, ticket("T-A")); + let (_, tool) = definition(client.clone(), kind)(); + let ordinary = tool + .execute(&input.to_string(), ToolExecutionContext::direct()) + .await + .unwrap(); + let native = execute_native( + client.clone(), + kind.name(), + input.clone(), + ToolExecutionContext::direct(), + ) + .await + .unwrap() + .unwrap(); + assert_eq!(native.content, ordinary.content); + { + let requests = client.requests.lock().unwrap(); + assert_eq!(requests[0], requests[1]); + } + client.grants.lock().unwrap().clear(); + assert!( + execute_native( + client.clone(), + kind.name(), + input, + ToolExecutionContext::direct() + ) + .await + .unwrap() + .is_err() + ); + assert_eq!(client.requests.lock().unwrap().len(), 2); + assert!( + execute_native( + client, + "Unknown", + Value::Null, + ToolExecutionContext::direct() + ) + .await + .is_none() + ); + } + } +} 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 5b84310d2..7b19dd4ce 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 @@ -150,15 +150,47 @@ async fn tracking_operations_authorize_exact_ticket_before_dispatch_and_recheck_ ), permissions: None, ticket_reference: "T-A".into(), + collection_route: "/tickets".into(), + merge_request_route: "/merge-requests".into(), + objective_route: "/objectives".into(), observations: Arc::new(Mutex::new(TicketObservationState::default())), }; - let args = input - .as_object() - .unwrap() - .iter() - .filter(|(key, _)| key.as_str() != "ticket") - .map(|(key, value)| (key.clone(), json_to_wip(value).unwrap())) - .collect(); + let mut native = input.clone(); + native.as_object_mut().unwrap().remove("ticket"); + if matches!(kind, TrackingToolKind::Update) { + let fields = native.as_object_mut().unwrap(); + let assignment = fields.remove("assignment_id").unwrap(); + let event = fields.remove("expected_last_event_id").unwrap(); + fields.insert( + "observed".into(), + json!({"assignment_id":assignment,"expected_last_event_id":event}), + ); + for field in ["waiting_reason", "progress", "next_action", "result"] { + if let Some(value) = native["tracking"].get_mut(field) { + *value = if value.is_null() { + json!({"$case":"null"}) + } else { + json!({"$case":"value","value":value.clone()}) + }; + } + } + } + if matches!(kind, TrackingToolKind::History) { + let fields = native.as_object_mut().unwrap(); + let cursor = fields.remove("after_event_id"); + let limit = fields.remove("limit"); + let mut page = serde_json::Map::new(); + if let Some(v) = cursor { + page.insert("cursor".into(), v); + } + if let Some(v) = limit { + page.insert("limit".into(), v); + } + fields.insert("page".into(), Value::Object(page)); + } + let WipValue::Record(args) = json_to_wip(&native).unwrap() else { + unreachable!() + }; let projection = native_ticket_operation(kind.name()).unwrap(); assert!(matches!( handler diff --git a/crates/worker/src/feature/builtin/ticket/native.rs b/crates/worker/src/feature/builtin/ticket/native.rs new file mode 100644 index 000000000..b97796fe1 --- /dev/null +++ b/crates/worker/src/feature/builtin/ticket/native.rs @@ -0,0 +1,660 @@ +//! Native Ticket actions are defined by their subjects and preconditions, not Tool schemas. +//! +//! Collection/item separation is retained: discovery and creation have no existing subject; +//! judgments, authoring, delegation and relationships do. Assignment/post identifiers are +//! observations within a Ticket, not independently authorized child Objects. Keeping them in +//! records avoids implying that observing a child grants independent execution authority. +//! Read and mutation actions remain separate because their permissions and retry safety differ. +//! +//! 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. +//! +//! For example, Inspect the collection and its Interface, then invoke `query` with +//! `{"filter":{"states":["open"]},"page":{"limit":10}}`. Inspect a returned item path 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. + +use super::*; +#[cfg(test)] +mod tests; +use crate::feature::builtin::native_contract::{ + Case, Contract, Destination, Input, ReferenceKind, ReferenceRoots, Shape, +}; + +fn p(name: &'static str, required: bool, ty: Shape) -> Input { + Input { + name, + required, + ty, + destination: Destination::Same, + } +} +fn rename(name: &'static str, destination: &'static str, required: bool, ty: Shape) -> Input { + Input { + name, + required, + ty, + destination: Destination::Rename(destination), + } +} +fn group(name: &'static str, required: bool, fields: Vec) -> Input { + Input { + name, + required, + ty: Shape::Record(fields), + destination: Destination::Flatten, + } +} +fn record(fields: Vec) -> Shape { + Shape::Record(fields) +} +fn nullable(ty: Shape) -> Shape { + Shape::Nullable(Box::new(ty)) +} +fn list(ty: Shape) -> Shape { + Shape::List(Box::new(ty)) +} +fn strings() -> Shape { + list(Shape::String) +} +fn enumeration(cases: &[&'static str]) -> Shape { + Shape::Enum(cases.to_vec()) +} +fn case(name: &'static str, payload: Option) -> Case { + Case { name, payload } +} +fn union(tag: &'static str, cases: Vec) -> Shape { + Shape::Union { tag, cases } +} +fn state() -> Shape { + enumeration(&["open", "closed"]) +} +fn relation() -> Shape { + enumeration(&[ + "depends_on", + "blocks", + "related", + "supersedes", + "duplicate_of", + ]) +} +fn ticket_ref() -> Shape { + Shape::Reference(ReferenceKind::Ticket) +} +fn page(cursor: &'static str, limit: &'static str) -> Input { + group( + "page", + false, + vec![ + rename("cursor", cursor, false, Shape::String), + rename("limit", limit, false, Shape::Integer), + ], + ) +} +fn principal() -> Shape { + union( + "kind", + vec![ + case( + "user", + Some(record(vec![p("account_id", true, Shape::String)])), + ), + case( + "worker", + Some(record(vec![ + p("runtime_id", true, Shape::String), + p("worker_id", true, Shape::String), + ])), + ), + case( + "workspace_agent", + Some(record(vec![p("agent_key", true, Shape::String)])), + ), + ], + ) +} +fn reason() -> Shape { + // An externally tagged domain enum is encoded by this single native union definition. + Shape::ExternalUnion { + cases: vec![ + case("completed", None), + case("not_planned", None), + case("duplicate", None), + case("unknown", None), + case("other", Some(Shape::String)), + ], + } +} +fn target() -> Shape { + record(vec![ + p("repository_key", true, Shape::String), + p("ref_selector", false, nullable(Shape::String)), + p("access", true, enumeration(&["read_only", "read_write"])), + ]) +} +fn references() -> Shape { + list(record(vec![ + p("kind", true, Shape::String), + p("target", true, Shape::String), + ])) +} +fn assignment_observation() -> Input { + group( + "observed", + true, + vec![ + p("assignment_id", true, Shape::String), + p("expected_last_event_id", true, Shape::String), + ], + ) +} + +pub(super) fn contract(op: &str) -> Contract { + let (name, summary, parameters) = match op { + "query" => ( + "query", + "Discover bounded Tickets; Entry filters select related subjects, not authority. Backend caps pages and validates filter combinations.", + vec![ + group( + "filter", + false, + vec![ + p("query", false, Shape::String), + p("states", false, list(state())), + p("closure_reason", false, reason()), + p("assigned_principal", false, principal()), + p( + "handling_status", + false, + enumeration(&["waiting", "inprogress", "done"]), + ), + p("event_kinds", false, strings()), + p( + "evidence", + false, + list(enumeration(&["merge_request", "commit", "approved_review"])), + ), + p( + "review_status", + false, + enumeration(&[ + "none", + "unknown", + "pending", + "approved", + "request_changes", + "unresolved_changes", + ]), + ), + p( + "attention", + false, + list(enumeration(&[ + "unresolved_review", + "missing_commit", + "blocked", + "unblocked", + "awaiting_review", + "unresolved_changes", + "stale_after_rescope", + "missing_evidence", + ])), + ), + rename("related", "related_ticket_id", false, ticket_ref()), + p("relation_kind", false, relation()), + p("tag_ids", false, strings()), + p("tag_ids_any", false, strings()), + p("tag_ids_none", false, strings()), + rename( + "objective", + "linked_objective_id", + false, + Shape::Reference(ReferenceKind::Objective), + ), + p("updated_after", false, Shape::String), + p("updated_before", false, Shape::String), + ], + ), + p( + "sort", + false, + enumeration(&[ + "relevance", + "updated_desc", + "created_desc", + "priority", + "title", + ]), + ), + page("cursor", "limit"), + ], + ), + "create" => ( + "create", + "Create a Ticket from content and optional ordered implementation targets. Selectors may remain unspecified while planning.", + vec![ + group( + "content", + true, + vec![ + p("title", true, Shape::String), + p("body", false, Shape::String), + p("state", false, state()), + p("assignee", false, Shape::String), + p("readiness", false, Shape::String), + p("risk_flags", false, strings()), + p("queued_at", false, Shape::String), + ], + ), + p("targets", false, list(target())), + ], + ), + "read" => ( + "read", + "Read this route-bound Ticket and a bounded thread page. Omit page for Backend defaults.", + vec![page("event_cursor", "event_limit")], + ), + "edit" => ( + "edit", + "Author this Ticket's content without appending discussion. Body replacement and substring editing are distinct choices; domain validates unique matches.", + vec![ + p("title", false, Shape::String), + Input { + name: "body", + required: false, + ty: union( + "mode", + vec![ + case( + "replace", + Some(record(vec![rename("text", "body", true, Shape::String)])), + ), + case( + "splice", + Some(record(vec![ + rename("old", "old_string", true, Shape::String), + rename("new", "new_string", true, Shape::String), + p("replace_all", false, Shape::Boolean), + ])), + ), + ], + ), + destination: Destination::Flatten, + }, + p( + "targets", + false, + union( + "action", + vec![ + case("clear", None), + case( + "set", + Some(record(vec![p("targets", true, list(target()))])), + ), + ], + ), + ), + ], + ), + "edit_post" => ( + "edit_post", + "Replace a persisted discussion post using its exact observed body. Truncated preimages and system events cannot authorize an edit.", + vec![ + group( + "observed", + true, + vec![ + p("event_ref", true, Shape::String), + p("expected_body", true, Shape::String), + ], + ), + p("body", true, Shape::String), + ], + ), + "comment" => ( + "comment", + "Append Markdown discussion to this Ticket; this is not a fenced assignment answer or result notification.", + vec![p("body", true, Shape::String)], + ), + "case_update" => ( + "case_update", + "Record a case judgment against exact observed content/state/closure key. Only completed asserts fulfillment. Reopen does not reissue work. Keep the operation key and full input on an uncertain outcome; never automatically replay.", + vec![ + group( + "observed", + true, + vec![ + rename( + "content_digest", + "expected_content_digest", + true, + Shape::String, + ), + rename("state", "expected_state", true, state()), + rename( + "closure_key", + "expected_closure_operation_key", + true, + nullable(Shape::String), + ), + ], + ), + p("operation_key", true, Shape::String), + p( + "action", + true, + union( + "action", + vec![ + case("close", Some(record(vec![p("reason", true, reason())]))), + case("reopen", None), + case( + "correct_closure", + Some(record(vec![p("reason", true, reason())])), + ), + ], + ), + ), + p("explanation", true, Shape::String), + p("references", false, references()), + ], + ), + "record_relation" => ( + "record_relation", + "Record a forward Ticket relationship; inverse views are derived. Target Entry is distinct from this route's subject.", + vec![ + p("kind", true, relation()), + p("target", true, ticket_ref()), + p("note", false, Shape::String), + ], + ), + "remove_relation" => ( + "remove_relation", + "Remove exactly the selected forward relationship, not the target Ticket.", + vec![p("kind", true, relation()), p("target", true, ticket_ref())], + ), + "dependency_check" => ( + "dependency_check", + "Read dependency diagnostics; do not start dependencies or infer execution permission.", + vec![], + ), + "orchestrator_assign" => ( + "orchestrator_assign", + "Delegate or explicitly reissue responsibility; no Worker starts. Observed null means unassigned, not unknown. Exact retries retain operation identity and full input.", + vec![ + p("principal", true, principal()), + group( + "observed", + true, + vec![rename( + "assignment_id", + "expected_assignment_id", + true, + nullable(Shape::String), + )], + ), + p("operation_id", true, Shape::String), + ], + ), + "orchestrator_clear" => ( + "orchestrator_clear", + "Release exactly the observed delegation, not Worker lifecycle or handling completion.", + vec![ + p("assignment_id", true, Shape::String), + p("operation_id", true, Shape::String), + ], + ), + "assignments_query" => ( + "assignments_query", + "List retained immutable delegation identities; history is not current execution authority.", + vec![page("after_assignment_id", "limit")], + ), + "assignment_tracking_query" => ( + "assignment_tracking_query", + "Read current delegation or the selected retained identity; unknown tracking is not waiting.", + vec![p("assignment_id", false, Shape::String)], + ), + "assignment_tracking_history" => ( + "assignment_tracking_history", + "Read immutable handling history for this delegation.", + vec![ + p("assignment_id", true, Shape::String), + page("after_event_id", "limit"), + ], + ), + "assignment_tracking_update" => ( + "assignment_tracking_update", + "Report the complete handling payload for the exact assignment/event observation. Domain validates waiting reasons and authenticated sender. Handling does not alter case state.", + vec![ + assignment_observation(), + p("operation_key", true, Shape::String), + p( + "tracking", + true, + record(vec![ + p( + "status", + true, + enumeration(&["waiting", "inprogress", "done"]), + ), + p("waiting_reason", false, nullable(Shape::String)), + p("progress", false, nullable(Shape::String)), + p("next_action", false, nullable(Shape::String)), + p("result", false, nullable(Shape::String)), + ]), + ), + ], + ), + "assignment_answer" => ( + "assignment_answer", + "Continue the same waiting/inprogress assignment with fenced input, not automatic inprogress. Done needs explicit reissue.", + vec![ + assignment_observation(), + p("operation_key", true, Shape::String), + p("body", true, Shape::String), + ], + ), + "assignment_diagnostics" => ( + "assignment_diagnostics", + "Read independent delivery/input/start evidence; acceptance is not successful processing.", + vec![ + p("assignment_id", true, Shape::String), + group( + "page", + false, + vec![ + p("after_input_event_id", false, Shape::String), + p("after_start_operation_id", false, Shape::String), + p("limit", false, Shape::Integer), + ], + ), + ], + ), + "implementation_result" => ( + "implementation_result", + "Publish a result, handoff or blocker with a Host-owned retry identity. This neither closes the case nor approves or integrates an MR; uncertain results are not automatically replayed.", + vec![ + p("kind", true, enumeration(&["handoff", "result", "blocker"])), + p("body", true, Shape::String), + p( + "merge_request", + false, + record(vec![ + rename( + "entry", + "merge_request_id", + true, + Shape::Reference(ReferenceKind::MergeRequest), + ), + p("subject_ref", true, Shape::String), + ]), + ), + ], + ), + "implementation_result_query" => ( + "implementation_result_query", + "Read one retained result; it is historical evidence, not new execution authority.", + vec![p("result_id", true, Shape::String)], + ), + "tag_list" => ( + "tag_list", + "List active Workspace classification tags, not Ticket states.", + vec![page("after", "limit")], + ), + "tag_create" => ( + "tag_create", + "Create classification with stable retry identity; names never grant authority.", + vec![ + p("name", true, Shape::String), + p("operation_key", true, Shape::String), + ], + ), + "tag_rename" => ( + "tag_rename", + "Rename the observed stable tag without changing membership identity.", + vec![ + group( + "observed", + true, + vec![ + p("tag_id", true, Shape::String), + p("expected_last_operation_key", true, Shape::String), + ], + ), + p("name", true, Shape::String), + p("operation_key", true, Shape::String), + ], + ), + "tag_delete" => ( + "tag_delete", + "Tombstone exactly the observed tag and remove membership atomically; this is not case closure.", + vec![ + group( + "observed", + true, + vec![ + p("tag_id", true, Shape::String), + p("expected_last_operation_key", true, Shape::String), + ], + ), + p("operation_key", true, Shape::String), + ], + ), + "tags_set" => ( + "tags_set", + "Replace the entire tag set against observed membership, not content CAS. Preserve unrelated classifications when reconciling conflicts.", + vec![ + group( + "observed", + true, + vec![rename("tags", "expected_tag_ids", true, strings())], + ), + rename("tags", "tag_ids", true, strings()), + p("operation_key", true, Shape::String), + ], + ), + "notification_diagnostics" => ( + "notification_diagnostics", + "Read sanitized routing/delivery evidence; exact origin filters are independently authorized. History and retry candidates are not execution permission.", + vec![ + group( + "filter", + false, + vec![ + p("assignment_id", false, Shape::String), + rename("ticket", "ticket", false, ticket_ref()), + rename( + "merge_request", + "merge_request_id", + false, + Shape::Reference(ReferenceKind::MergeRequest), + ), + p("job_id", false, Shape::String), + p("source_event_id", false, Shape::String), + p("delivery_id", false, Shape::String), + p("source_id", false, Shape::String), + ], + ), + page("cursor", "limit"), + ], + ), + "record_orchestration_plan" => ( + "record_orchestration_plan", + "Record project-level ordering/conflict/capacity/accepted-plan context without moving case state or starting work. Domain validates kind-specific requirements.", + vec![ + p("kind", true, plan_kind()), + p("related_ticket", false, ticket_ref()), + p("note", false, Shape::String), + p( + "accepted_plan", + false, + record(vec![ + p("summary", true, Shape::String), + p("branch", false, Shape::String), + p("worktree", false, Shape::String), + p("role_plan", false, Shape::String), + ]), + ), + ], + ), + "query_orchestration_plans" => ( + "query_orchestration_plans", + "Read bounded project-level plan records; planning history is not execution authority.", + vec![ + p("kind", false, plan_kind()), + p("limit", false, Shape::Integer), + ], + ), + _ => unreachable!("every enabled native Ticket action has a contract: {op}"), + }; + Contract { + name, + summary, + parameters, + } +} +fn plan_kind() -> Shape { + enumeration(&[ + "before", + "after", + "blocked_by", + "blocks", + "conflicts_with", + "do_not_parallelize", + "waiting_capacity_note", + "accepted_plan", + ]) +} + +pub(super) fn input( + op: &str, + arguments: &BTreeMap, + bound: Option<&str>, + identity_field: Option<&str>, + ticket_root: &str, + mr_root: &str, + objective_root: &str, +) -> Result { + let mut command = contract(op).convert( + arguments, + &ReferenceRoots { + ticket: ticket_root, + merge_request: mr_root, + objective: objective_root, + }, + )?; + let fields = command.as_object_mut().expect("native command is a record"); + // Body alternatives are native editing choices, not domain fields. + if op == "edit" { + fields.remove("mode"); + } + if let (Some(subject), Some(field)) = (bound, identity_field) { + fields.insert(field.into(), json!(subject)); + } + Ok(command) +} diff --git a/crates/worker/src/feature/builtin/ticket/native/tests.rs b/crates/worker/src/feature/builtin/ticket/native/tests.rs new file mode 100644 index 000000000..e657aacb1 --- /dev/null +++ b/crates/worker/src/feature/builtin/ticket/native/tests.rs @@ -0,0 +1,251 @@ +//! Contract-boundary regressions. Public WIP values are converted by production contracts and +//! decoded by shared command DTOs; no Tool Schema is used as the expected native contract. +use super::*; + +fn arguments(value: Value) -> BTreeMap { + match json_to_wip(&value).unwrap() { + WipValue::Record(fields) => fields, + _ => panic!("record"), + } +} +fn converted(op: &str, value: Value, bound: Option<&str>) -> Value { + input( + op, + &arguments(value), + bound, + bound.map(|_| "ticket"), + "/cases", + "/changes", + "/goals", + ) + .unwrap_or_else(|_| panic!("native {op} rejected")) +} + +#[test] +fn closure_judgment_compound_union_preserves_domain_reason_and_observed_null() { + for reason in [ + json!({"$case":"not_planned"}), + json!({"$case":"other","value":"Bounded research returned"}), + ] { + let command = converted( + "case_update", + json!({ + "operation_key":"judgment-1", + "observed":{"content_digest":"digest","state":"open","closure_key":{"$case":"null"}}, + "action":{"$case":"close","value":{"reason":reason.clone()}}, + "explanation":"An explicit judgment" + }), + Some("T-42"), + ); + let decoded: WorkspaceTicketCaseUpdateInput = + serde_json::from_value(command.clone()).unwrap(); + assert_eq!(decoded.ticket, "T-42"); + assert_eq!(decoded.expected_closure_operation_key, None); + let expected = if reason["$case"] == "other" { + ticket::TicketClosureReason::Other("Bounded research returned".into()) + } else { + ticket::TicketClosureReason::NotPlanned + }; + assert_eq!( + decoded.action, + ticket::TicketCaseAction::Close { reason: expected } + ); + assert_eq!(command["expected_closure_operation_key"], Value::Null); + } + let base = json!({"operation_key":"j","observed":{"content_digest":"d","state":"closed","closure_key":{"$case":"value","value":"previous"}},"action":{"$case":"reopen"},"explanation":"Handle again"}); + let command = converted("case_update", base.clone(), Some("T-42")); + let decoded: WorkspaceTicketCaseUpdateInput = serde_json::from_value(command).unwrap(); + assert_eq!(decoded.action, ticket::TicketCaseAction::Reopen); + assert_eq!( + decoded.expected_closure_operation_key.as_deref(), + Some("previous") + ); + for invalid in [ + { + let mut v = base.clone(); + v["observed"].as_object_mut().unwrap().remove("closure_key"); + v + }, + { + let mut v = base.clone(); + v["observed"]["closure_key"] = Value::Null; + v + }, + { + let mut v = base.clone(); + v["action"] = json!({"$case":"reopen","value":null}); + v + }, + { + let mut v = base.clone(); + v["action"] = + json!({"$case":"close","value":{"reason":{"$case":"completed","value":"extra"}}}); + v + }, + { + let mut v = base.clone(); + v["ticket"] = json!("T-99"); + v + }, + { + let mut v = base; + v["observed"]["state"] = json!("done"); + v + }, + ] { + assert!( + input( + "case_update", + &arguments(invalid), + Some("T-42"), + Some("ticket"), + "/cases", + "/changes", + "/goals" + ) + .is_err() + ); + } +} + +#[test] +fn editing_choices_and_implementation_targets_produce_closed_domain_commands() { + let command = converted( + "edit", + json!({"body":{"$case":"splice","value":{"old":"before","new":"after"}},"targets":{"$case":"set","value":{"targets":[{"repository_key":"main","ref_selector":{"$case":"null"},"access":"read_write"}]}}}), + Some("T-42"), + ); + assert_eq!( + command, + json!({"ticket":"T-42","old_string":"before","new_string":"after","targets":{"action":"set","targets":[{"repository_key":"main","ref_selector":null,"access":"read_write"}]}}) + ); + assert_eq!( + converted( + "edit", + json!({"body":{"$case":"replace","value":{"text":"Full body"}},"targets":{"$case":"clear"}}), + Some("T-42") + ), + json!({"ticket":"T-42","body":"Full body","targets":{"action":"clear"}}) + ); + let omitted = converted("create", json!({"content":{"title":"Intent"}}), None); + assert!(omitted.get("body").is_none()); + for value in [ + json!({"content":{"title":"Intent","body":null}}), + json!({"content":{"title":"Intent","actor":"forged"}}), + json!({"content":{"title":"Intent"},"targets":[{"repository_key":"main","access":"write"}]}), + ] { + assert!( + input( + "create", + &arguments(value), + None, + None, + "/cases", + "/changes", + "/goals" + ) + .is_err() + ); + } +} + +#[test] +fn native_entry_inputs_use_allocated_namespace_and_never_subject_argument_ids() { + assert_eq!( + converted( + "record_relation", + json!({"kind":"depends_on","target":"/cases/T-9"}), + Some("T-42") + ), + json!({"ticket":"T-42","kind":"depends_on","target":"T-9"}) + ); + for target in [ + "T-9", + "/tickets/T-9", + "/cases-extra/T-9", + "/changes/T-9", + "/cases/O-9", + "/cases/T-9/child", + "/cases/T-9%2Fchild", + ] { + assert!( + input( + "record_relation", + &arguments(json!({"kind":"depends_on","target":target})), + Some("T-42"), + Some("ticket"), + "/cases", + "/changes", + "/goals" + ) + .is_err(), + "{target}" + ); + } + assert!( + input( + "record_relation", + &arguments(json!({"kind":"depends_on","target":"/cases/T-9"})), + Some("T-42"), + Some("ticket"), + "", + "/changes", + "/goals" + ) + .is_err() + ); +} + +#[test] +fn assignment_principal_and_cas_are_declared_compounds_not_opaque_json() { + let value = json!({"principal":{"$case":"worker","value":{"runtime_id":"r","worker_id":"w"}},"observed":{"assignment_id":{"$case":"null"}},"operation_id":"assign-1"}); + let command = converted("orchestrator_assign", value.clone(), Some("T-42")); + assert_eq!( + command, + json!({"ticket":"T-42","principal":{"kind":"worker","runtime_id":"r","worker_id":"w"},"expected_assignment_id":null,"operation_id":"assign-1"}) + ); + for invalid in [ + { + let mut v = value.clone(); + v["principal"]["value"]["account_id"] = json!("spoof"); + v + }, + { + let mut v = value.clone(); + v["principal"] = json!({"$case":"admin"}); + v + }, + { + let mut v = value; + v["observed"] = json!({}); + v + }, + ] { + assert!( + input( + "orchestrator_assign", + &arguments(invalid), + Some("T-42"), + Some("ticket"), + "/cases", + "/changes", + "/goals" + ) + .is_err() + ); + } + let declaration = contract("orchestrator_assign").declaration(); + assert!( + declaration + .parameters + .iter() + .any(|p| p.name == "principal" + && matches!(p.r#type, wip_protocol::TypeExpr::Union { .. })) + ); + assert!( + declaration + .parameters + .iter() + .all(|p| !matches!(p.r#type, wip_protocol::TypeExpr::Json)) + ); +} diff --git a/crates/worker/src/feature/builtin/ticket/notification_diagnostics.rs b/crates/worker/src/feature/builtin/ticket/notification_diagnostics.rs index b1cad8888..7d32501a2 100644 --- a/crates/worker/src/feature/builtin/ticket/notification_diagnostics.rs +++ b/crates/worker/src/feature/builtin/ticket/notification_diagnostics.rs @@ -13,129 +13,148 @@ struct DiagnosticsTool { #[async_trait] impl Tool for DiagnosticsTool { async fn execute(&self, input: &str, _: ToolExecutionContext) -> Result { - let workspace_id = self - .client - .workspace_id() - .filter(|s| !s.trim().is_empty()) - .ok_or_else(|| { - ToolError::InvalidArgument( - "Notification diagnostics require a Backend Workspace; no local fallback" - .into(), - ) - })?; - let query: server_api::NotificationDiagnosticsQuery = - serde_json::from_str(input).map_err(|e| ToolError::InvalidArgument(e.to_string()))?; - if !(1..=100).contains(&query.limit.unwrap_or(50)) { - return Err(ToolError::InvalidArgument( - "limit must be between 1 and 100".into(), - )); - } - for value in [ - &query.ticket, - &query.merge_request_id, - &query.job_id, - &query.source_event_id, - &query.assignment_id, - &query.delivery_id, - &query.source_id, - &query.cursor, - ] - .into_iter() - .flatten() - { - if value.is_empty() || value.len() > 512 || value.chars().any(char::is_control) { - return Err(ToolError::InvalidArgument( - "Notification filters must be nonempty, control-free and at most 512 bytes" - .into(), - )); - } - } - use server_api::authorization::PermissionResource; - let mut resources = Vec::new(); - if let Some(job_id) = &query.job_id { - resources.push(PermissionResource::Job { - workspace_id: workspace_id.into(), - job_id: job_id.clone(), - }); - } - if let Some(ticket_id) = &query.ticket { - resources.push(PermissionResource::Ticket { - workspace_id: workspace_id.into(), - ticket_id: ticket_id.clone(), - }); - } - if let Some(merge_request_id) = &query.merge_request_id { - resources.push(PermissionResource::MergeRequest { - workspace_id: workspace_id.into(), - merge_request_id: merge_request_id.clone(), - }); - } - // source_checks probes origins independently of the page's other filters. - // Opaque (including hashed and legacy Job) identities must resolve from - // Host metadata, never from a caller-supplied Job filter. - let mut bound_source = None; - if let Some(event) = &query.source_event_id { - // Ticket ordinals are scoped by source_checks to this Ticket. All - // global identities use persisted ownership, including legacy Job - // IDs whose spelling happens to resemble an MR identity. - if !(event.starts_with("ticket-event:") && query.ticket.is_some()) { - bound_source = Some(event.clone()); - } - } - if resources.is_empty() && bound_source.is_none() { - resources.push(PermissionResource::Workspace { - workspace_id: workspace_id.into(), - }); - } - for resource in resources { - self.client - .authorize_operation("notification.diagnostics.read", &resource) - .map_err(|error| ToolError::ExecutionFailed(error.to_string()))?; - } - if let Some(source_event_id) = bound_source { - self.client - .authorize_bound_operation( - "notification.diagnostics.read", - server_api::authorization::BoundAuthorizationTarget::NotificationSource { - source_event_id, - }, - ) - .map_err(|error| ToolError::ExecutionFailed(error.to_string()))?; - } - let response = self - .client - .execute_server_operation(WorkspaceServerOperation::NotificationDiagnostics { query }) - .map_err(|e| ToolError::ExecutionFailed(e.to_string()))?; - if !response.is_success() { - // Do not expose an arbitrary error body from upstream. - return Err(ToolError::ExecutionFailed(format!( - "Notification diagnostics failed with HTTP status {}", - response.status - ))); - } - if response.body.len() > 1024 * 1024 { - return Err(ToolError::ExecutionFailed( - "Notification response exceeds 1 MiB".into(), - )); - } - let page: server_api::NotificationDiagnosticsPage = serde_json::from_str(&response.body) - .map_err(|_| { - ToolError::ExecutionFailed("Invalid notification diagnostics response".into()) - })?; - if page.items.len() > 100 { - return Err(ToolError::ExecutionFailed( - "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![], - }) + let query = serde_json::from_str(input) + .map_err(|error| ToolError::InvalidArgument(error.to_string()))?; + execute_domain(self.client.clone(), query).await } } + +/// Convert an adapter-owned internal command without Tool execution or schema validation. +pub(super) async fn execute_native( + 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, + Err(error) => Err(ToolError::InvalidArgument(error.to_string())), + }) +} + +async fn execute_domain( + client: Arc, + query: server_api::NotificationDiagnosticsQuery, +) -> Result { + let workspace_id = client + .workspace_id() + .filter(|s| !s.trim().is_empty()) + .ok_or_else(|| { + ToolError::InvalidArgument( + "Notification diagnostics require a Backend Workspace; no local fallback".into(), + ) + })?; + if !(1..=100).contains(&query.limit.unwrap_or(50)) { + return Err(ToolError::InvalidArgument( + "limit must be between 1 and 100".into(), + )); + } + for value in [ + &query.ticket, + &query.merge_request_id, + &query.job_id, + &query.source_event_id, + &query.assignment_id, + &query.delivery_id, + &query.source_id, + &query.cursor, + ] + .into_iter() + .flatten() + { + if value.is_empty() || value.len() > 512 || value.chars().any(char::is_control) { + return Err(ToolError::InvalidArgument( + "Notification filters must be nonempty, control-free and at most 512 bytes".into(), + )); + } + } + use server_api::authorization::PermissionResource; + let mut resources = Vec::new(); + if let Some(job_id) = &query.job_id { + resources.push(PermissionResource::Job { + workspace_id: workspace_id.into(), + job_id: job_id.clone(), + }); + } + if let Some(ticket_id) = &query.ticket { + resources.push(PermissionResource::Ticket { + workspace_id: workspace_id.into(), + ticket_id: ticket_id.clone(), + }); + } + if let Some(merge_request_id) = &query.merge_request_id { + resources.push(PermissionResource::MergeRequest { + workspace_id: workspace_id.into(), + merge_request_id: merge_request_id.clone(), + }); + } + // source_checks probes origins independently of the page's other filters. + // Opaque (including hashed and legacy Job) identities must resolve from + // Host metadata, never from a caller-supplied Job filter. + let mut bound_source = None; + if let Some(event) = &query.source_event_id { + // Ticket ordinals are scoped by source_checks to this Ticket. All + // global identities use persisted ownership, including legacy Job + // IDs whose spelling happens to resemble an MR identity. + if !(event.starts_with("ticket-event:") && query.ticket.is_some()) { + bound_source = Some(event.clone()); + } + } + if resources.is_empty() && bound_source.is_none() { + resources.push(PermissionResource::Workspace { + workspace_id: workspace_id.into(), + }); + } + for resource in resources { + client + .authorize_operation("notification.diagnostics.read", &resource) + .map_err(|error| ToolError::ExecutionFailed(error.to_string()))?; + } + if let Some(source_event_id) = bound_source { + client + .authorize_bound_operation( + "notification.diagnostics.read", + server_api::authorization::BoundAuthorizationTarget::NotificationSource { + source_event_id, + }, + ) + .map_err(|error| ToolError::ExecutionFailed(error.to_string()))?; + } + let response = client + .execute_server_operation(WorkspaceServerOperation::NotificationDiagnostics { query }) + .map_err(|e| ToolError::ExecutionFailed(e.to_string()))?; + if !response.is_success() { + // Do not expose an arbitrary error body from upstream. + return Err(ToolError::ExecutionFailed(format!( + "Notification diagnostics failed with HTTP status {}", + response.status + ))); + } + if response.body.len() > 1024 * 1024 { + return Err(ToolError::ExecutionFailed( + "Notification response exceeds 1 MiB".into(), + )); + } + let page: server_api::NotificationDiagnosticsPage = serde_json::from_str(&response.body) + .map_err(|_| { + ToolError::ExecutionFailed("Invalid notification diagnostics response".into()) + })?; + if page.items.len() > 100 { + return Err(ToolError::ExecutionFailed( + "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![], + }) +} #[cfg(test)] mod authorization_tests { use super::*; @@ -234,7 +253,7 @@ mod authorization_tests { async fn notification_ticket_grant_rejects_other_ticket_and_wip_rechecks_revocation() { let client = AuthorizedClient::new(json!({"items":[],"next_cursor":null,"source_checks":[]})); - client.grant("notification.diagnostics.read", ticket("T-A")); + client.grant("notification.diagnostics.read", ticket("T-1")); let (_, tool) = definition(client.clone())(); assert!( tool.execute(r#"{"ticket":"T-B"}"#, ToolExecutionContext::direct()) @@ -252,9 +271,14 @@ mod authorization_tests { ), permissions: None, collection_route: "/tickets".into(), + merge_request_route: "/merge-requests".into(), + objective_route: "/objectives".into(), observations: Arc::new(Mutex::new(TicketObservationState::default())), }; - let args = BTreeMap::from([("ticket".into(), WipValue::String("T-A".into()))]); + let args = BTreeMap::from([( + "filter".into(), + json_to_wip(&json!({"ticket":"/tickets/T-1"})).unwrap(), + )]); handler .call( "notification_diagnostics", @@ -318,9 +342,14 @@ mod authorization_tests { ), permissions: None, collection_route: "/tickets".into(), + merge_request_route: "/merge-requests".into(), + objective_route: "/objectives".into(), observations: Arc::new(Mutex::new(TicketObservationState::default())), }; - let args = BTreeMap::from([("job_id".into(), WipValue::String("Job-A".into()))]); + let args = BTreeMap::from([( + "filter".into(), + json_to_wip(&json!({"job_id":"Job-A"})).unwrap(), + )]); handler .call( "notification_diagnostics", @@ -454,3 +483,85 @@ pub(super) fn definition(client: Arc) -> ToolDefinition { ) }) } + +#[cfg(test)] +mod native_command_tests { + use super::*; + use assignment_tracking::authorization_tests::{AuthorizedClient, ticket}; + use server_api::authorization::PermissionResource; + + #[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 input = json!({"ticket":"T-A","job_id":"Job-A","cursor":"source/7?","limit":2}); + client.grant("notification.diagnostics.read", ticket("T-A")); + assert!( + execute_native( + client.clone(), + NAME, + input.clone(), + ToolExecutionContext::direct() + ) + .await + .unwrap() + .is_err() + ); + assert!(client.requests.lock().unwrap().is_empty()); + client.grant( + "notification.diagnostics.read", + PermissionResource::Job { + workspace_id: "workspace".into(), + job_id: "Job-A".into(), + }, + ); + let (_, tool) = definition(client.clone())(); + let ordinary = tool + .execute(&input.to_string(), ToolExecutionContext::direct()) + .await + .unwrap(); + let native = execute_native( + client.clone(), + NAME, + input.clone(), + ToolExecutionContext::direct(), + ) + .await + .unwrap() + .unwrap(); + assert_eq!(native.content, ordinary.content); + { + let requests = client.requests.lock().unwrap(); + assert_eq!(requests[0], requests[1]); + } + client.grants.lock().unwrap().clear(); + assert!( + execute_native(client.clone(), NAME, input, ToolExecutionContext::direct()) + .await + .unwrap() + .is_err() + ); + assert_eq!(client.requests.lock().unwrap().len(), 2); + assert!( + execute_native( + client.clone(), + "Unknown", + Value::Null, + ToolExecutionContext::direct() + ) + .await + .is_none() + ); + assert!(matches!( + execute_native( + client, + NAME, + json!({"limit":0}), + ToolExecutionContext::direct() + ) + .await + .unwrap(), + Err(ToolError::InvalidArgument(_)) + )); + } +} 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 aeac89adc..f065a8d5e 100644 --- a/crates/worker/src/feature/builtin/ticket/notification_diagnostics/tests.rs +++ b/crates/worker/src/feature/builtin/ticket/notification_diagnostics/tests.rs @@ -21,9 +21,14 @@ async fn notification_wip_executes_same_read_and_honors_profile_denial() { rules: vec![], }), collection_route: "/tickets".into(), + merge_request_route: "/merge-requests".into(), + objective_route: "/objectives".into(), observations: Arc::new(Mutex::new(TicketObservationState::default())), }; - let args = BTreeMap::from([("ticket".into(), WipValue::String("T-755".into()))]); + let args = BTreeMap::from([( + "filter".into(), + json_to_wip(&json!({"ticket":"/tickets/T-755"})).unwrap(), + )]); let result = handler .call( "notification_diagnostics", diff --git a/crates/worker/src/feature/builtin/ticket/tags.rs b/crates/worker/src/feature/builtin/ticket/tags.rs index 46f33cfbb..c65c5c4b8 100644 --- a/crates/worker/src/feature/builtin/ticket/tags.rs +++ b/crates/worker/src/feature/builtin/ticket/tags.rs @@ -142,133 +142,201 @@ pub(super) fn validate_ids(name: &str, ids: &[String]) -> Result<(), ToolError> Ok(()) } -fn parse(input: &str) -> Result { - serde_json::from_str(input).map_err(|error| ToolError::InvalidArgument(error.to_string())) -} - #[derive(Clone)] struct TagTool { client: Arc, kind: TagToolKind, } +// Internal command DTOs belong to the shared execution boundary, not a native schema. +enum Command { + List(ListInput), + Create(CreateInput), + Rename(RenameInput), + Delete(DeleteInput), + Set(SetInput), +} + +impl Command { + fn kind(&self) -> TagToolKind { + match self { + Self::List(..) => TagToolKind::List, + Self::Create(..) => TagToolKind::Create, + Self::Rename(..) => TagToolKind::Rename, + Self::Delete(..) => TagToolKind::Delete, + Self::Set(..) => TagToolKind::Set, + } + } +} + #[async_trait] impl Tool for TagTool { - async fn execute(&self, input: &str, _: ToolExecutionContext) -> Result { - self.client - .workspace_id() - .filter(|id| !id.trim().is_empty()) - .ok_or_else(|| { - ToolError::InvalidArgument( - "Classification tools require Backend Workspace identity; no local fallback" - .into(), - ) - })?; - let backend = WorkspaceHttpTicketBackend::new(self.client.clone()); - let content = match self.kind { - TagToolKind::List => { - let input: ListInput = parse(input)?; - if input.limit.is_some_and(|limit| limit > 200) { - return Err(ToolError::InvalidArgument( - "limit must be at most 200".into(), - )); - } - if let Some(after) = &input.after { - required("after", after)?; - } - let limit = input.limit.unwrap_or(200); - let mut tags = backend - .list_workspace_tags(ticket::TagListRequest { - after: input.after, - limit: Some(limit), - }) - .map_err(|error| ToolError::ExecutionFailed(error.to_string()))?; - tags.truncate(limit); - json!(tags) - } - TagToolKind::Set => { - let input: SetInput = parse(input)?; - required("ticket", &input.ticket)?; - required("operation_key", &input.operation_key)?; - validate_ids("expected_tag_ids", &input.expected_tag_ids)?; - validate_ids("tag_ids", &input.tag_ids)?; - let tags = backend - .set_ticket_tags( - TicketIdOrSlug::from(input.ticket.as_str()), - ticket::TicketTagSetRequest { - expected_tag_ids: input.expected_tag_ids, - tag_ids: input.tag_ids, - operation_key: input.operation_key, - }, - ) - .map_err(|error| ToolError::ExecutionFailed(error.to_string()))?; - if tags.len() > 100 { - return Err(ToolError::ExecutionFailed( - "Backend returned more than 100 Ticket tags".into(), - )); - } - json!(tags) - } - kind => { - let request = match kind { - TagToolKind::Create => { - let input: CreateInput = parse(input)?; - validate_name(&input.name)?; - ticket::WorkspaceTagMutation { - operation_key: input.operation_key, - action: ticket::WorkspaceTagAction::Create { name: input.name }, - } - } - TagToolKind::Rename => { - let input: RenameInput = parse(input)?; - required("tag_id", &input.tag_id)?; - required( - "expected_last_operation_key", - &input.expected_last_operation_key, - )?; - validate_name(&input.name)?; - ticket::WorkspaceTagMutation { - operation_key: input.operation_key, - action: ticket::WorkspaceTagAction::Rename { - tag_id: input.tag_id, - expected_last_operation_key: input.expected_last_operation_key, - name: input.name, - }, - } - } - TagToolKind::Delete => { - let input: DeleteInput = parse(input)?; - required("tag_id", &input.tag_id)?; - required( - "expected_last_operation_key", - &input.expected_last_operation_key, - )?; - ticket::WorkspaceTagMutation { - operation_key: input.operation_key, - action: ticket::WorkspaceTagAction::Delete { - tag_id: input.tag_id, - expected_last_operation_key: input.expected_last_operation_key, - }, - } - } - _ => unreachable!(), - }; - required("operation_key", &request.operation_key)?; - json!( - backend - .mutate_workspace_tag(request) - .map_err(|error| ToolError::ExecutionFailed(error.to_string()))? - ) - } + async fn execute( + &self, + input: &str, + ctx: ToolExecutionContext, + ) -> Result { + let command = match self.kind { + TagToolKind::List => Command::List( + serde_json::from_str(input) + .map_err(|error| ToolError::InvalidArgument(error.to_string()))?, + ), + TagToolKind::Create => Command::Create( + serde_json::from_str(input) + .map_err(|error| ToolError::InvalidArgument(error.to_string()))?, + ), + TagToolKind::Rename => Command::Rename( + serde_json::from_str(input) + .map_err(|error| ToolError::InvalidArgument(error.to_string()))?, + ), + TagToolKind::Delete => Command::Delete( + serde_json::from_str(input) + .map_err(|error| ToolError::InvalidArgument(error.to_string()))?, + ), + TagToolKind::Set => Command::Set( + serde_json::from_str(input) + .map_err(|error| ToolError::InvalidArgument(error.to_string()))?, + ), }; - Ok(ToolOutput { - summary: self.kind.name().into(), - content: Some(content.to_string()), - attachments: Vec::new(), - }) + execute_domain(self.client.clone(), command, ctx).await } } +/// Convert an adapter-owned internal command; never invokes Tool execution or its schema. +pub(super) async fn execute_native( + 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 { + "WorkspaceTagList" => serde_json::from_value(command).map(Command::List), + "WorkspaceTagCreate" => serde_json::from_value(command).map(Command::Create), + "WorkspaceTagRename" => serde_json::from_value(command).map(Command::Rename), + "WorkspaceTagDelete" => serde_json::from_value(command).map(Command::Delete), + "TicketTagsSet" => serde_json::from_value(command).map(Command::Set), + _ => return None, + }; + Some(match command { + Ok(command) => execute_domain(client, command, ctx).await, + Err(error) => Err(parse(error)), + }) +} + +async fn execute_domain( + client: Arc, + command: Command, + _ctx: ToolExecutionContext, +) -> Result { + let kind = command.kind(); + client + .workspace_id() + .filter(|id| !id.trim().is_empty()) + .ok_or_else(|| { + ToolError::InvalidArgument( + "Classification tools require Backend Workspace identity; no local fallback".into(), + ) + })?; + let backend = WorkspaceHttpTicketBackend::new(client.clone()); + let content = match command { + Command::List(input) => { + if input.limit.is_some_and(|limit| limit > 200) { + return Err(ToolError::InvalidArgument( + "limit must be at most 200".into(), + )); + } + if let Some(after) = &input.after { + required("after", after)?; + } + let limit = input.limit.unwrap_or(200); + let mut tags = backend + .list_workspace_tags(ticket::TagListRequest { + after: input.after, + limit: Some(limit), + }) + .map_err(|error| ToolError::ExecutionFailed(error.to_string()))?; + tags.truncate(limit); + json!(tags) + } + Command::Set(input) => { + required("ticket", &input.ticket)?; + required("operation_key", &input.operation_key)?; + validate_ids("expected_tag_ids", &input.expected_tag_ids)?; + validate_ids("tag_ids", &input.tag_ids)?; + let tags = backend + .set_ticket_tags( + TicketIdOrSlug::from(input.ticket.as_str()), + ticket::TicketTagSetRequest { + expected_tag_ids: input.expected_tag_ids, + tag_ids: input.tag_ids, + operation_key: input.operation_key, + }, + ) + .map_err(|error| ToolError::ExecutionFailed(error.to_string()))?; + if tags.len() > 100 { + return Err(ToolError::ExecutionFailed( + "Backend returned more than 100 Ticket tags".into(), + )); + } + json!(tags) + } + mutation @ (Command::Create(_) | Command::Rename(_) | Command::Delete(_)) => { + let request = match mutation { + Command::Create(input) => { + validate_name(&input.name)?; + ticket::WorkspaceTagMutation { + operation_key: input.operation_key, + action: ticket::WorkspaceTagAction::Create { name: input.name }, + } + } + Command::Rename(input) => { + required("tag_id", &input.tag_id)?; + required( + "expected_last_operation_key", + &input.expected_last_operation_key, + )?; + validate_name(&input.name)?; + ticket::WorkspaceTagMutation { + operation_key: input.operation_key, + action: ticket::WorkspaceTagAction::Rename { + tag_id: input.tag_id, + expected_last_operation_key: input.expected_last_operation_key, + name: input.name, + }, + } + } + Command::Delete(input) => { + required("tag_id", &input.tag_id)?; + required( + "expected_last_operation_key", + &input.expected_last_operation_key, + )?; + ticket::WorkspaceTagMutation { + operation_key: input.operation_key, + action: ticket::WorkspaceTagAction::Delete { + tag_id: input.tag_id, + expected_last_operation_key: input.expected_last_operation_key, + }, + } + } + _ => unreachable!(), + }; + required("operation_key", &request.operation_key)?; + json!( + backend + .mutate_workspace_tag(request) + .map_err(|error| ToolError::ExecutionFailed(error.to_string()))? + ) + } + }; + Ok(ToolOutput { + summary: kind.name().into(), + content: Some(content.to_string()), + attachments: Vec::new(), + }) +} + pub(super) fn definition(client: Arc, kind: TagToolKind) -> ToolDefinition { Arc::new(move || { ( @@ -282,3 +350,146 @@ pub(super) fn definition(client: Arc, kind: TagToolKind) -> ) }) } + +#[cfg(test)] +mod native_command_tests { + use super::*; + use assignment_tracking::authorization_tests::{AuthorizedClient, ticket}; + use server_api::authorization::PermissionResource; + + #[tokio::test] + async fn internal_tag_commands_preserve_resource_authorization_cas_and_replay_requests() { + let tag = json!({"workspace_id":"workspace", "id":"tag-1", "name":"Name", "last_operation_key":"observed", "deleted":false}); + for (kind, operation, input, response, resource) in [ + ( + TagToolKind::List, + "ticket.tag.list", + json!({"limit":2,"after":"tag/0?"}), + json!([tag.clone()]), + PermissionResource::Workspace { + workspace_id: "workspace".into(), + }, + ), + ( + TagToolKind::Create, + "ticket.tag.create", + json!({"name":"Name","operation_key":"stable"}), + tag.clone(), + PermissionResource::Workspace { + workspace_id: "workspace".into(), + }, + ), + ( + TagToolKind::Rename, + "ticket.tag.rename", + json!({"tag_id":"tag-1","name":"Name","expected_last_operation_key":"observed","operation_key":"stable"}), + tag.clone(), + PermissionResource::Workspace { + workspace_id: "workspace".into(), + }, + ), + ( + TagToolKind::Delete, + "ticket.tag.delete", + json!({"tag_id":"tag-1","expected_last_operation_key":"observed","operation_key":"stable"}), + tag.clone(), + PermissionResource::Workspace { + workspace_id: "workspace".into(), + }, + ), + ( + TagToolKind::Set, + "ticket.tags.set", + json!({"ticket":"T-A","expected_tag_ids":["tag-0"],"tag_ids":["tag-1"],"operation_key":"stable"}), + json!([tag]), + ticket("T-A"), + ), + ] { + let client = AuthorizedClient::new(response); + assert!( + execute_native( + client.clone(), + kind.name(), + input.clone(), + ToolExecutionContext::direct() + ) + .await + .unwrap() + .is_err() + ); + assert!(client.requests.lock().unwrap().is_empty()); + client.grant(operation, resource); + let (_, tool) = definition(client.clone(), kind)(); + let ordinary = tool + .execute(&input.to_string(), ToolExecutionContext::direct()) + .await + .unwrap(); + let native = execute_native( + client.clone(), + kind.name(), + input.clone(), + ToolExecutionContext::direct(), + ) + .await + .unwrap() + .unwrap(); + assert_eq!(native.content, ordinary.content); + { + let requests = client.requests.lock().unwrap(); + assert_eq!(requests[0], requests[1]); + } + client.grants.lock().unwrap().clear(); + assert!( + execute_native( + client.clone(), + kind.name(), + input, + ToolExecutionContext::direct() + ) + .await + .unwrap() + .is_err() + ); + assert_eq!(client.requests.lock().unwrap().len(), 2); + assert!( + execute_native( + client, + "Unknown", + Value::Null, + ToolExecutionContext::direct() + ) + .await + .is_none() + ); + } + } + + #[tokio::test] + async fn internal_tag_commands_apply_business_validation_before_dispatch() { + let client = AuthorizedClient::new(json!({})); + for (name, input) in [ + ( + "WorkspaceTagCreate", + json!({"name":" ","operation_key":"stable"}), + ), + ("WorkspaceTagList", json!({"limit":201})), + ( + "TicketTagsSet", + json!({"ticket":"T-A","expected_tag_ids":[],"tag_ids":["duplicate","duplicate"],"operation_key":"stable"}), + ), + ( + "WorkspaceTagRename", + json!({"tag_id":"tag-1","name":"Name","operation_key":"stable"}), + ), + ] { + assert!(matches!( + execute_native(client.clone(), name, input, ToolExecutionContext::direct()) + .await + .unwrap(), + Err(ToolError::InvalidArgument(_)) + )); + } + assert!(client.requests.lock().unwrap().is_empty()); + assert!(client.authorizations.lock().unwrap().is_empty()); + } +} diff --git a/crates/worker/src/feature/builtin/ticket/tags/tests.rs b/crates/worker/src/feature/builtin/ticket/tags/tests.rs index 6cfe30d54..31cdadd1a 100644 --- a/crates/worker/src/feature/builtin/ticket/tags/tests.rs +++ b/crates/worker/src/feature/builtin/ticket/tags/tests.rs @@ -358,6 +358,9 @@ async fn classification_wip_uses_normal_permissions_and_route_bound_ticket_ident ), permissions: None, ticket_reference: "T-740".into(), + collection_route: "/tickets".into(), + merge_request_route: "/merge-requests".into(), + objective_route: "/objectives".into(), observations: Arc::new(Mutex::new(TicketObservationState::default())), }; handler @@ -368,8 +371,8 @@ async fn classification_wip_uses_normal_permissions_and_route_bound_ticket_ident .insert("T-740".into(), "content-1".into()); let mut args = BTreeMap::from([ ("operation_key".into(), WipValue::String("set-1".into())), - ("tag_ids".into(), json_to_wip(&json!(["tag-1"])).unwrap()), - ("expected_tag_ids".into(), json_to_wip(&json!([])).unwrap()), + ("tags".into(), json_to_wip(&json!(["tag-1"])).unwrap()), + ("observed".into(), json_to_wip(&json!({"tags":[]})).unwrap()), ]); assert!( handler diff --git a/crates/worker/src/wip.rs b/crates/worker/src/wip.rs index 5a8c9e664..f1e989446 100644 --- a/crates/worker/src/wip.rs +++ b/crates/worker/src/wip.rs @@ -45,8 +45,10 @@ //! Acquisition, display, and calls are separate. Tree follows only indexable edges; unobserved //! children are not empty sets. Inspect displays only the selected Object or Interface in the //! official Text View. Displays that cannot meet completeness, freshness, or bounds fail rather than -//! silently truncate. Displays are untrusted data; signatures are never reparsed as call targets. -//! The binding defines details. +//! silently truncate. An explicit Interface operation selector can retrieve a complete operation +//! contract with partial-Interface coverage marked. It changes display only, retaining the full +//! observed call descriptor and validating the original SDK input limits before selection. +//! Displays are untrusted data; signatures are never reparsed as call targets. //! //! # Post-dispatch outcomes and cancellation //! @@ -564,6 +566,17 @@ impl WipMountRegistry { Ok(WipNamespaceRoute { root }) } + /// Return the unique Host allocation for a provider, not an authorization grant. + /// Ambiguous allocations cannot be used to interpret cross-domain Entry input. + pub(crate) fn namespace_root(&self, owner: &str) -> Option<&str> { + let mut roots = self + .namespaces + .iter() + .filter(|(_, provider)| provider.as_str() == owner); + let (root, _) = roots.next()?; + roots.next().is_none().then_some(root.as_str()) + } + pub fn mount( &mut self, projection: WipProjection, @@ -3240,12 +3253,20 @@ mod tests { ["Inspect", "Invoke", "Tree"] ); let metrics = runtime.metrics(); - assert!( - metrics.ordinary_schema_bytes > metrics.wip_schema_bytes, - "ordinary={} WIP={}", - metrics.ordinary_schema_bytes, - metrics.wip_schema_bytes - ); + assert!(metrics.ordinary_schema_bytes > 0); + assert!(metrics.wip_schema_bytes > 0); + + // Binding overhead is fixed, not a promise to save bytes for every tiny Tool set. + let mut larger = Engine::<_, Mutable, ()>::new_annotated(DummyClient); + for index in 0..48 { + larger.register_tool(echo_definition(format!("Echo{index}"), Arc::clone(&calls))); + } + let larger = install_wip_mode(&mut larger, None, "worker-b".into()) + .unwrap() + .metrics(); + assert!(larger.ordinary_schema_bytes > metrics.ordinary_schema_bytes); + assert_eq!(larger.wip_schema_bytes, metrics.wip_schema_bytes); + assert!(larger.ordinary_schema_bytes > larger.wip_schema_bytes); } #[tokio::test] @@ -3904,7 +3925,7 @@ mod tests { let error = runtime .invoke( "/features/ticket/tickets/T-1".into(), - crate::wip::root_reference("yoi.ticket/item/v1"), + crate::wip::root_reference("yoi.ticket/item/v2"), "read".into(), json!({}), ToolExecutionContext::direct(), @@ -6194,7 +6215,7 @@ mod tests { authoring .call( collection.into(), - crate::wip::root_reference("yoi.ticket/collection/v1"), + crate::wip::root_reference("yoi.ticket/collection/v2"), "query".into(), json!({}), ToolExecutionContext::direct(), @@ -6204,9 +6225,9 @@ mod tests { let created = authoring .call( collection.into(), - crate::wip::root_reference("yoi.ticket/collection/v1"), + crate::wip::root_reference("yoi.ticket/collection/v2"), "create".into(), - json!({"title": "New ticket"}), + json!({"content":{"title": "New ticket"}}), ToolExecutionContext::direct(), ) .await @@ -6221,7 +6242,7 @@ mod tests { authoring .call( item.into(), - crate::wip::root_reference("yoi.ticket/item/v1"), + crate::wip::root_reference("yoi.ticket/item/v2"), "comment".into(), json!({"body": "Native comment"}), ToolExecutionContext::direct(), @@ -6322,9 +6343,9 @@ mod tests { let result = workflow .call( item.into(), - crate::wip::root_reference("yoi.ticket/item/v1"), + crate::wip::root_reference("yoi.ticket/item/v2"), "record_relation".into(), - json!({"kind": "depends_on", "target": "T-10"}), + json!({"kind": "depends_on", "target": "/tickets/T-10"}), ToolExecutionContext::direct(), ) .await @@ -6402,6 +6423,9 @@ mod tests { coder_engine.register_tool(definition); } let mut coder_registry = WipMountRegistry::new(); + coder_registry + .allocate_namespace("ticket", "cases") + .unwrap(); let route = coder_registry .allocate_namespace("merge-request", "merge-requests") .unwrap(); @@ -6433,13 +6457,11 @@ mod tests { let opened = coder .call( collection.into(), - crate::wip::root_reference("yoi.merge-request/collection/v1"), + crate::wip::root_reference("yoi.merge-request/collection/v2"), "open".into(), json!({ - "ticket": "T-685", - "repository_key": "main", - "selector_from": "work/T-685-native-mr", - "selector_to": "develop" + "ticket": "/cases/T-685", + "repository": {"key":"main","source":"work/T-685-native-mr","target":"develop"} }), ToolExecutionContext::direct(), ) @@ -6460,9 +6482,9 @@ mod tests { coder .call( item.clone(), - crate::wip::root_reference("yoi.merge-request/item/v1"), + crate::wip::root_reference("yoi.merge-request/item/v2"), "read".into(), - json!({"after": 7, "limit": 25}), + json!({"page":{"after":{"$case":"value","value":7},"limit":{"$case":"value","value":25}}}), ToolExecutionContext::direct(), ) .await @@ -6553,7 +6575,7 @@ mod tests { reviewer .call( item.clone(), - crate::wip::root_reference("yoi.merge-request/item/v1"), + crate::wip::root_reference("yoi.merge-request/item/v2"), "review".into(), json!({"decision": "approve", "body": "approved", "findings": []}), ToolExecutionContext::direct(), @@ -6646,8 +6668,8 @@ mod tests { orchestrator .call( item.clone(), - crate::wip::root_reference("yoi.merge-request/item/v1"), - "check_readiness".into(), + crate::wip::root_reference("yoi.merge-request/item/v2"), + "readiness".into(), json!({}), ToolExecutionContext::direct(), ) @@ -6657,15 +6679,13 @@ mod tests { orchestrator .call( item.clone(), - crate::wip::root_reference("yoi.merge-request/item/v1"), + crate::wip::root_reference("yoi.merge-request/item/v2"), "complete".into(), json!({ "operation_id": "merge-op-1", - "approval_event_id": "review-1", - "target_ref_before": "target-1", - "target_ref_after": "result-1", - "strategy": "fast_forward", - "resolution": "clean" + "integration": {"approval_event_id": "review-1", + "target": {"before": "target-1", "after": "result-1"}, + "strategy": "fast_forward", "resolution": "clean"} }), ToolExecutionContext::direct(), ) @@ -6682,7 +6702,7 @@ mod tests { let rejected = orchestrator .call( collection.into(), - crate::wip::root_reference("yoi.merge-request/collection/v1"), + crate::wip::root_reference("yoi.merge-request/collection/v2"), "complete_ticket".into(), json!({}), ToolExecutionContext::direct(), @@ -7244,6 +7264,7 @@ mod tests { assert!(error.to_string().contains("PermissionDenied")); assert_eq!(calls.load(Ordering::SeqCst), 0); } + include!("wip/ticket_native_tests.rs"); } #[cfg(test)] diff --git a/crates/worker/src/wip/binding.rs b/crates/worker/src/wip/binding.rs index 8beb42454..d871bbc16 100644 --- a/crates/worker/src/wip/binding.rs +++ b/crates/worker/src/wip/binding.rs @@ -10,6 +10,9 @@ //! It neither expands referenced descriptors automatically nor requires a prior Tree. Official Text //! View signatures remain unchanged; paths and structured references sit in the outer envelope. //! Rendering/envelope bound violations fail the whole display; incomplete contracts never return success. +//! An oversized Interface display supplies a complete operation-name index when it fits. An explicit +//! operation selector retrieves that operation's full contract, marking partial-Interface coverage; +//! it does not alter the observed descriptor used for calls or bypass SDK input limits. //! //! Invoke uses structured Interface references and named arguments. Missing, stale, or failed //! observations get at most one pre-dispatch acquisition attempt per target; dispatched calls never @@ -313,8 +316,25 @@ impl WipRuntime { /// Inspect exactly one Object or explicitly addressed Interface. /// Compact references are decoded only at this input boundary, never from signatures. pub async fn inspect(&self, path: String, refresh: bool) -> Result { + self.inspect_operation(path, refresh, None).await + } + + // Selection changes only the display coverage, never the stored call contract or validators. + // Render the full input first so selecting an operation cannot evade SDK descriptor limits. + pub(crate) async fn inspect_operation( + &self, + path: String, + refresh: bool, + operation: Option<&str>, + ) -> Result { match inspect_target::parse(&path)? { inspect_target::Target::Object(object_path) => { + if operation.is_some() { + return Err(ToolError::InvalidArgument( + "operation selection requires an explicit Interface address from Inspect" + .into(), + )); + } self.binding_observe(&object_path, 0, refresh, false) .await?; let observation = { @@ -359,15 +379,52 @@ impl WipRuntime { error, ) })?; - self.binding_output( - "Inspected WIP Interface".into(), - json!({ - "path": inspect_target::interface_path(&reference), - "reference": reference_json(&reference), - "interface_signature": signature, - "metrics": metrics_json(self.metrics.snapshot()), - }), - ) + let signature = if let Some(selected) = operation { + let mut descriptor = observation + .descriptor + .as_ref() + .expect("fresh Interface") + .clone(); + let selected = descriptor + .operations + .iter() + .find(|op| op.name == selected) + .cloned() + .ok_or_else(|| { + ToolError::InvalidArgument( + "operation is not declared by the selected Interface".into(), + ) + })?; + descriptor.operations = vec![selected]; + render_interface(&Interface { + reference: &reference, + descriptor: &descriptor, + }) + .map_err(|error| binding_render_error(json!({"path":path}), error))? + } else { + signature + }; + let mut response = json!({ + "path": inspect_target::interface_path(&reference), + "reference": reference_json(&reference), + "interface_signature": signature, + "metrics": metrics_json(self.metrics.snapshot()), + }); + if let Some(operation) = operation { + response["coverage"] = json!({"operation":operation,"complete_interface":false,"complete_operation":true}); + } + self.binding_output("Inspected WIP Interface".into(), response).map_err(|error| { + if operation.is_none() && matches!(&error, ToolError::ExecutionFailed(message) if message.contains("response limit")) { + let names = observation.descriptor.as_ref().expect("fresh Interface").operations.iter().map(|op| op.name.as_str()).collect::>(); + let index = serde_json::to_string(&names).expect("operation names serialize"); + let index = if index.len() <= self.wire_limits.max_response_bytes() / 2 { + format!(" Available operations (complete index): {index}") + } else { + " Operation-name index also exceeds the display limit.".into() + }; + ToolError::ExecutionFailed(format!("{error}; no partial signature returned. Inspect this Interface with an exact operation selector to retrieve its complete contract.{index}")) + } else { error } + }) } } } @@ -479,6 +536,10 @@ struct WipInspectInput { /// Explicitly refresh only the selected Object or Interface. #[serde(default)] refresh: bool, + /// Optional exact operation on an explicit Interface address. Returns its complete signature + /// and types with partial-Interface coverage marked; use when a full Interface exceeds display limits. + #[serde(default)] + operation: Option, } /// Protocol references deliberately do not derive provider schema traits. @@ -548,7 +609,9 @@ impl Tool for WipInspectTool { ) -> Result { let input: WipInspectInput = serde_json::from_str(input_json) .map_err(|error| ToolError::InvalidArgument(error.to_string()))?; - self.runtime.inspect(input.path, input.refresh).await + self.runtime + .inspect_operation(input.path, input.refresh, input.operation.as_deref()) + .await } } @@ -2091,7 +2154,7 @@ pub(super) fn wip_tool_definitions(runtime: Arc) -> Vec, ) diff --git a/crates/worker/src/wip/ticket_native_tests.rs b/crates/worker/src/wip/ticket_native_tests.rs new file mode 100644 index 000000000..708e7428f --- /dev/null +++ b/crates/worker/src/wip/ticket_native_tests.rs @@ -0,0 +1,257 @@ +// Model-facing integration: production Inspect, official Text View, Invoke, native conversion, +// shared domain authorization/forwarding and output projection. Only the Workspace transport is +// scripted; this test does not claim persistence or server-side policy evaluation. +#[tokio::test] +async fn ticket_inspect_contract_drives_query_read_and_comment_without_tool_schema() { + let detail = json!({ + "resource_key":"T-42", "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 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":[],"relation_count":0,"blocker_count":0,"unresolved_blocker_count":0,"unresolved_review_count":0,"merge_requests":[]}); + let client = Arc::new( + ScriptedWorkspaceClient::new([ + crate::worker::WorkspaceResponse { + status: 200, + body: json!({"items":[query_item],"page":{"next_cursor":null,"has_more":false}}) + .to_string(), + }, + crate::worker::WorkspaceResponse { + status: 200, + body: detail.to_string(), + }, + crate::worker::WorkspaceResponse { + status: 204, + body: String::new(), + }, + ]) + .with_authorizations([ + expected_authorization("ticket.query", "workspace", None), + expected_authorization("ticket.read", "ticket", Some(("ticket_id", "T-42"))), + expected_authorization("ticket.event.add", "ticket", Some(("ticket_id", "T-42"))), + ]), + ); + let mut engine = Engine::<_, Mutable, ()>::new_annotated(DummyClient); + let mut registry = WipMountRegistry::new(); + let namespace = 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, + &namespace, + ) + .unwrap(); + let runtime = + install_wip_mode_with_mounts(&mut engine, None, "model".into(), registry).unwrap(); + async fn inspect(runtime: &WipRuntime, path: &str) -> Json { + serde_json::from_str( + &runtime + .inspect(path.into(), false) + .await + .unwrap() + .content + .unwrap(), + ) + .unwrap() + } + let object = inspect(&runtime, "/cases").await; + let interface = &object["interfaces"][0]; + let declaration = inspect(&runtime, interface["path"].as_str().unwrap()).await; + let signature = declaration["interface_signature"].as_str().unwrap(); + assert!(signature.contains("query(")); + assert!(signature.contains("filter?")); + assert!(signature.contains("states?")); + assert!(signature.contains("open")); + assert!(!signature.contains("Tool schema")); + let reference = InterfaceReference { + scope: interface["reference"]["scope"].as_str().unwrap().into(), + name: interface["reference"]["name"].as_str().unwrap().into(), + }; + let output = runtime + .invoke( + "/cases".into(), + reference, + "query".into(), + json!({"filter":{"states":["open"]},"page":{"limit":3}}), + ToolExecutionContext::direct(), + ) + .await + .unwrap(); + let output: Json = serde_json::from_str(&output.content.unwrap()).unwrap(); + let path = output["tickets"][0]["path"].as_str().unwrap(); + assert_eq!(path, "/cases/T-42"); + let object = inspect(&runtime, path).await; + let interface = &object["interfaces"][0]; + let declaration = inspect(&runtime, interface["path"].as_str().unwrap()).await; + let signature = declaration["interface_signature"].as_str().unwrap(); + assert!(signature.contains("read(")); + assert!(signature.contains("page?")); + assert!(signature.contains("comment(")); + assert!(signature.contains("body: string")); + assert!(!signature.contains("ticket: string")); + let reference = InterfaceReference { + scope: interface["reference"]["scope"].as_str().unwrap().into(), + name: interface["reference"]["name"].as_str().unwrap().into(), + }; + runtime + .invoke( + path.into(), + reference.clone(), + "read".into(), + json!({"page":{"limit":2}}), + ToolExecutionContext::direct(), + ) + .await + .unwrap(); + runtime.inspect(path.into(), true).await.unwrap(); + runtime + .invoke( + path.into(), + reference, + "comment".into(), + json!({"body":"Confirmed requirement"}), + ToolExecutionContext::direct(), + ) + .await + .unwrap(); + client.assert_authorizations_consumed(); + let requests = client.requests(); + assert_eq!(requests.len(), 3); + let query: Json = serde_json::from_str(requests[0].body.as_deref().unwrap()).unwrap(); + assert_eq!(query["states"], json!(["open"])); + assert_eq!(query["limit"], 3); + let read: Json = serde_json::from_str(requests[1].body.as_deref().unwrap()).unwrap(); + assert_eq!(read["event_limit"], 2); + assert!(requests[2].path.ends_with("/tickets/T-42/thread-events")); +} + +#[test] +fn native_namespace_reference_lookup_is_explicit_unique_and_not_publication() { + let mut registry = WipMountRegistry::new(); + assert_eq!(registry.namespace_root("ticket"), None); + registry.allocate_namespace("ticket", "cases").unwrap(); + assert_eq!(registry.namespace_root("ticket"), Some("/cases")); + assert!( + registry.routes().next().is_none(), + "allocation does not publish a provider" + ); + registry + .allocate_namespace("ticket", "other-cases") + .unwrap(); + assert_eq!( + registry.namespace_root("ticket"), + None, + "ambiguous input coordinates fail closed" + ); +} + +#[tokio::test] +async fn large_ticket_interface_is_explicitly_bounded_and_selected_operation_remains_invokable() { + let client = Arc::new( + ScriptedWorkspaceClient::new([crate::worker::WorkspaceResponse { + status: 204, + body: String::new(), + }]) + .with_authorizations([expected_authorization( + "ticket.event.add", + "ticket", + Some(("ticket_id", "T-42")), + )]), + ); + let mut registry = WipMountRegistry::new(); + let namespace = registry.allocate_namespace("ticket", "cases").unwrap(); + crate::feature::builtin::ticket::mount_workspace_http_ticket_wip( + &mut registry, + client.clone(), + crate::feature::builtin::ticket::TicketFeatureAccess { + authoring: true, + thread: true, + intake: true, + workflow: true, + }, + None, + &namespace, + ) + .unwrap(); + let mut runtime = WipRuntime::from_mounts(registry, "selection".into()).unwrap(); + let object: Json = serde_json::from_str( + &runtime + .inspect("/cases/T-42".into(), false) + .await + .unwrap() + .content + .unwrap(), + ) + .unwrap(); + let shown = &object["interfaces"][0]; + let address = shown["path"].as_str().unwrap(); + let reference = InterfaceReference { + scope: shown["reference"]["scope"].as_str().unwrap().into(), + name: shown["reference"]["name"].as_str().unwrap().into(), + }; + let complete = runtime.inspect(address.into(), false).await.unwrap(); + assert!(complete.content.unwrap().len() > 1800); + runtime.wire_limits = Limits::new(2 * 1024 * 1024, 1800, 128).unwrap(); + let error = runtime + .inspect(address.into(), false) + .await + .unwrap_err() + .to_string(); + assert!(error.contains("no partial signature returned"), "{error}"); + let names = error + .split("Available operations (complete index): ") + .nth(1) + .unwrap(); + let names: Vec = serde_json::from_str(names).unwrap(); + assert!(names.iter().any(|name| name == "comment")); + assert!(names.iter().any(|name| name == "case_update")); + let selected = runtime + .inspect_operation(address.into(), false, Some("comment")) + .await + .unwrap(); + let selected: Json = serde_json::from_str(&selected.content.unwrap()).unwrap(); + assert_eq!( + selected["coverage"], + json!({"operation":"comment","complete_interface":false,"complete_operation":true}) + ); + let signature = selected["interface_signature"].as_str().unwrap(); + assert!(signature.contains("comment(")); + assert!(signature.contains("body: string")); + assert!(!signature.contains("case_update(")); + assert!( + runtime + .inspect_operation("/cases/T-42".into(), false, Some("comment")) + .await + .is_err() + ); + assert!( + runtime + .inspect_operation(address.into(), false, Some("missing")) + .await + .is_err() + ); + let projection = runtime.host.projection("/cases/T-42").unwrap(); + assert!( + projection + .descriptor + .operations + .iter() + .any(|op| op.name == "case_update"), + "selection must not replace the call descriptor" + ); + runtime + .invoke( + "/cases/T-42".into(), + reference, + "comment".into(), + json!({"body":"Bounded contract"}), + ToolExecutionContext::direct(), + ) + .await + .unwrap(); + assert_eq!(client.requests().len(), 1); + client.assert_authorizations_consumed(); +}