Support multiple merge requests per ticket

This commit is contained in:
2026-09-29 01:08:46 +09:00
parent ac7a6027bd
commit 590abf3142
26 changed files with 1539 additions and 723 deletions
@@ -28,12 +28,13 @@ fn workflow_instruction() -> FeatureInstructionDeclaration {
.expect("static Merge Request workflow instruction declaration is valid")
}
const ALL_KINDS: [Kind; 5] = [
const ALL_KINDS: [Kind; 6] = [
Kind::Show,
Kind::Open,
Kind::Review,
Kind::Readiness,
Kind::Complete,
Kind::CompleteTicket,
];
#[derive(Clone, Copy)]
@@ -42,6 +43,7 @@ enum Kind {
Readiness,
Open,
Complete,
CompleteTicket,
Review,
}
#[derive(Clone)]
@@ -50,8 +52,8 @@ struct MergeRequestTool {
kind: Kind,
}
#[derive(Debug, Deserialize, JsonSchema)]
struct TicketInput {
ticket: String,
struct MergeRequestInput {
merge_request_id: String,
}
#[derive(Debug, Deserialize, JsonSchema)]
struct OpenMergeRequestInput {
@@ -64,7 +66,7 @@ struct OpenMergeRequestInput {
}
#[derive(Debug, Deserialize, JsonSchema)]
struct CompleteMergeRequestInput {
ticket: String,
merge_request_id: String,
operation_id: String,
approval_event_id: String,
target_ref_before: String,
@@ -73,6 +75,13 @@ struct CompleteMergeRequestInput {
resolution: MergeResolutionInput,
}
#[derive(Debug, Deserialize, JsonSchema)]
struct CompleteTicketInput {
ticket: String,
operation_id: String,
item_revision: String,
merge_request_ids: Vec<String>,
}
#[derive(Debug, Deserialize, JsonSchema)]
#[serde(rename_all = "snake_case")]
enum MergeStrategyInput {
FastForward,
@@ -110,16 +119,6 @@ struct ReviewFindingInput {
line: Option<u32>,
body: String,
}
#[derive(Debug, Deserialize)]
struct TicketMergeRequestProjection {
merge_request: Option<TicketMergeRequestReference>,
}
#[derive(Debug, Deserialize)]
struct TicketMergeRequestReference {
merge_request_id: String,
}
impl Kind {
fn enabled(self, config: MergeRequestFeatureConfig) -> bool {
match self {
@@ -127,7 +126,7 @@ impl Kind {
Self::Open => config.open,
Self::Review => config.review,
Self::Readiness => config.readiness_check,
Self::Complete => config.complete,
Self::Complete | Self::CompleteTicket => config.complete,
}
}
@@ -137,14 +136,16 @@ impl Kind {
Self::Readiness => "CheckMergeRequestReadiness",
Self::Open => "OpenMergeRequest",
Self::Complete => "CompleteMergeRequest",
Self::CompleteTicket => "CompleteTicket",
Self::Review => "ReviewMergeRequest",
}
}
fn schema(self) -> serde_json::Value {
match self {
Self::Show | Self::Readiness => json!(schemars::schema_for!(TicketInput)),
Self::Show | Self::Readiness => json!(schemars::schema_for!(MergeRequestInput)),
Self::Open => json!(schemars::schema_for!(OpenMergeRequestInput)),
Self::Complete => json!(schemars::schema_for!(CompleteMergeRequestInput)),
Self::CompleteTicket => json!(schemars::schema_for!(CompleteTicketInput)),
Self::Review => json!(schemars::schema_for!(ReviewMergeRequestInput)),
}
}
@@ -156,17 +157,20 @@ impl Tool for MergeRequestTool {
ToolError::ExecutionFailed("Merge Request tools require Workspace identity".into())
})?;
if matches!(self.kind, Kind::Show) {
let value: TicketInput = parse(input)?;
nonempty(&value.ticket)?;
return self.show_current_merge_request(ws, &value.ticket);
let value: MergeRequestInput = parse(input)?;
nonempty_named("merge_request_id", &value.merge_request_id)?;
return self.show_merge_request(ws, &value.merge_request_id);
}
let (method, path, body) = match self.kind {
Kind::Readiness => {
let v: TicketInput = parse(input)?;
nonempty(&v.ticket)?;
let v: MergeRequestInput = parse(input)?;
nonempty_named("merge_request_id", &v.merge_request_id)?;
(
WorkspaceRequestMethod::Get,
format!("/api/w/{ws}/tickets/{}/merge-request/readiness", v.ticket),
format!(
"/api/w/{ws}/merge-requests/{}/readiness",
encode_path_segment(&v.merge_request_id)
),
None,
)
}
@@ -184,15 +188,34 @@ impl Tool for MergeRequestTool {
}
Kind::Complete => {
let v: CompleteMergeRequestInput = parse(input)?;
nonempty(&v.ticket)?;
nonempty_named("merge_request_id", &v.merge_request_id)?;
(
WorkspaceRequestMethod::Post,
format!("/api/w/{ws}/tickets/{}/merge-request/complete", v.ticket),
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::CompleteTicket => {
let v: CompleteTicketInput = parse(input)?;
nonempty_named("ticket", &v.ticket)?;
(
WorkspaceRequestMethod::Post,
format!(
"/api/w/{ws}/tickets/{}/complete",
encode_path_segment(&v.ticket)
),
Some(json!({
"operation_id": v.operation_id,
"item_revision": v.item_revision,
"merge_request_ids": v.merge_request_ids,
})),
)
}
Kind::Review => {
let v: ReviewMergeRequestInput = parse(input)?;
let ctx = self.client.reviewer_context().ok_or_else(|| {
@@ -203,8 +226,8 @@ impl Tool for MergeRequestTool {
(
WorkspaceRequestMethod::Post,
format!(
"/api/w/{ws}/tickets/{}/merge-request/reviews",
ctx.ticket_id
"/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::<Vec<_>>() }),
@@ -235,35 +258,12 @@ impl Tool for MergeRequestTool {
}
impl MergeRequestTool {
fn show_current_merge_request(
fn show_merge_request(
&self,
workspace_id: &str,
ticket: &str,
merge_request_id: &str,
) -> Result<ToolOutput, ToolError> {
let ticket_path = encode_path_segment(ticket);
let show_response = self
.client
.execute(WorkspaceRequest::json(
WorkspaceRequestMethod::Post,
format!("/api/w/{workspace_id}/tickets/{ticket_path}/show"),
json!({"event_limit": 1}).to_string(),
))
.map_err(|error| ToolError::ExecutionFailed(error.to_string()))?;
if !show_response.is_success() {
return Err(api_error("Ticket Show API", &show_response));
}
let projection: TicketMergeRequestProjection = serde_json::from_str(&show_response.body)
.map_err(|error| {
ToolError::ExecutionFailed(format!(
"Ticket Show API returned a malformed Merge Request projection: {error}"
))
})?;
let merge_request = projection.merge_request.ok_or_else(|| {
ToolError::ExecutionFailed(format!("Ticket `{ticket}` has no current Merge Request"))
})?;
nonempty_id("merge_request_id", &merge_request.merge_request_id)?;
let merge_request_id = encode_path_segment(&merge_request.merge_request_id);
let merge_request_id = encode_path_segment(merge_request_id);
let response = self
.client
.execute(WorkspaceRequest::get(format!(
@@ -313,10 +313,10 @@ fn encode_path_segment(value: &str) -> String {
encoded
}
fn nonempty_id(name: &str, value: &str) -> Result<(), ToolError> {
fn nonempty_named(name: &str, value: &str) -> Result<(), ToolError> {
if value.trim().is_empty() || value.chars().any(char::is_control) {
Err(ToolError::ExecutionFailed(format!(
"Ticket Show API returned an invalid {name}"
Err(ToolError::InvalidArgument(format!(
"{name} must not be empty"
)))
} else {
Ok(())
@@ -404,10 +404,13 @@ pub fn description(n: &str) -> Option<&'static str> {
"Resolve current provider refs and derive readiness from exact-source review evidence; source movement requires fresh review while target-only movement preserves unchanged-source approval.",
),
"OpenMergeRequest" => Some(
"Open the Ticket's one Merge Request with immutable source and target selectors; reuse it and advance only selector_from with a normal non-force push for later fixes.",
"Open one repository-scoped Merge Request linked to the Ticket; reuse an existing Merge Request for that repository and advance only its selector_from with a normal non-force push.",
),
"CompleteMergeRequest" => Some(
"Record Orchestrator-owned integration using unchanged-source approval and refreshed final target-ref evidence.",
"Record Orchestrator-owned integration for the explicitly addressed Merge Request without completing the Ticket or releasing its assignment.",
),
"CompleteTicket" => Some(
"Complete the Ticket only after reviewing its current item revision and exact linked Merge Request result set; this releases the current assignment atomically.",
),
"ReviewMergeRequest" => Some(
"Submit the injected Reviewer capability result for its captured exact source ref; source movement cancels it, while target-only movement does not.",
@@ -478,6 +481,7 @@ mod tests {
"CheckMergeRequestReadiness",
"OpenMergeRequest",
"CompleteMergeRequest",
"CompleteTicket",
"ReviewMergeRequest",
] {
assert!(description(name).is_some(), "missing operation {name}");
@@ -497,18 +501,20 @@ mod tests {
}
#[tokio::test]
async fn show_resolves_ticket_projection_then_reads_canonical_resource() {
let client = Arc::new(RecordingWorkspaceClient::new(vec![
response(json!({"merge_request":{"merge_request_id":"MR/1"}})),
response(json!({"merge_request_id":"MR/1","state":"open"})),
]));
async fn show_reads_explicit_merge_request_resource() {
let client = Arc::new(RecordingWorkspaceClient::new(vec![response(
json!({"merge_request_id":"MR/1","state":"open"}),
)]));
let tool = MergeRequestTool {
kind: Kind::Show,
client: client.clone(),
};
let output = tool
.execute(r#"{"ticket":"T/1"}"#, ToolExecutionContext::default())
.execute(
r#"{"merge_request_id":"MR/1"}"#,
ToolExecutionContext::default(),
)
.await
.expect("show should succeed");
@@ -517,18 +523,14 @@ mod tests {
Some(r#"{"merge_request_id":"MR/1","state":"open"}"#)
);
let requests = client.requests.lock().expect("request lock");
assert_eq!(requests.len(), 2);
assert_eq!(requests[0].method, WorkspaceRequestMethod::Post);
assert_eq!(requests[0].path, "/api/w/ws/tickets/T%2F1/show");
assert_eq!(requests[1].method, WorkspaceRequestMethod::Get);
assert_eq!(requests[1].path, "/api/w/ws/merge-requests/MR%2F1");
assert_eq!(requests.len(), 1);
assert_eq!(requests[0].method, WorkspaceRequestMethod::Get);
assert_eq!(requests[0].path, "/api/w/ws/merge-requests/MR%2F1");
}
#[tokio::test]
async fn show_fails_closed_when_ticket_has_no_current_merge_request() {
let client = Arc::new(RecordingWorkspaceClient::new(vec![response(
json!({"merge_request":null}),
)]));
async fn show_requires_explicit_merge_request_identity() {
let client = Arc::new(RecordingWorkspaceClient::new(vec![]));
let tool = MergeRequestTool {
kind: Kind::Show,
client: client.clone(),
@@ -537,10 +539,10 @@ mod tests {
let error = tool
.execute(r#"{"ticket":"T1"}"#, ToolExecutionContext::default())
.await
.expect_err("missing Merge Request must fail");
.expect_err("ticket-only lookup must fail");
assert!(error.to_string().contains("no current Merge Request"));
assert_eq!(client.requests.lock().expect("request lock").len(), 1);
assert!(error.to_string().contains("merge_request_id"));
assert!(client.requests.lock().expect("request lock").is_empty());
}
fn install(config: MergeRequestFeatureConfig) -> (Vec<String>, Vec<String>) {
@@ -596,7 +598,8 @@ mod tests {
[
"ShowMergeRequest",
"CheckMergeRequestReadiness",
"CompleteMergeRequest"
"CompleteMergeRequest",
"CompleteTicket"
]
);
assert_eq!(install(coder).1, [FEATURE_PROMPT_REF]);
@@ -27,6 +27,7 @@ struct ModelTicketQueryItem {
unresolved_blocker_count: usize,
unresolved_review_count: usize,
evidence: Option<ModelTicketEvidence>,
merge_requests: Vec<ModelMergeRequest>,
merge_request: Option<ModelMergeRequest>,
}
@@ -46,6 +47,7 @@ pub(super) struct ModelTicketDetail {
assignments: Vec<ModelAssignment>,
current_coder: Option<ModelWorkerSummary>,
implementation_reports: Vec<ModelEvidenceEvent>,
merge_requests: Vec<ModelMergeRequest>,
merge_request: Option<ModelMergeRequest>,
evidence: Option<ModelTicketEvidence>,
actions: Option<ModelTicketActions>,
@@ -162,6 +164,8 @@ struct ModelEvidenceEvent {
#[derive(Debug, Serialize)]
struct ModelMergeRequest {
merge_request_id: String,
repository_key: String,
state: String,
selector_from: Option<String>,
selector_to: String,
@@ -251,6 +255,10 @@ fn project_ticket_query_item(value: &Value) -> Result<ModelTicketQueryItem, Stri
unresolved_blocker_count: usize_field(item, "unresolved_blocker_count")?,
unresolved_review_count: usize_field(item, "unresolved_review_count")?,
evidence: item.get("evidence").map(project_evidence).transpose()?,
merge_requests: array_field(item, "merge_requests")?
.iter()
.map(project_merge_request)
.collect::<Result<Vec<_>, _>>()?,
merge_request: item
.get("merge_request")
.filter(|value| !value.is_null())
@@ -295,6 +303,10 @@ pub(super) fn project_ticket_detail(value: Value) -> Result<ModelTicketDetail, S
.iter()
.map(project_evidence_event)
.collect::<Result<Vec<_>, _>>()?,
merge_requests: array_field(root, "merge_requests")?
.iter()
.map(project_merge_request)
.collect::<Result<Vec<_>, _>>()?,
merge_request: root
.get("merge_request")
.filter(|value| !value.is_null())
@@ -498,11 +510,13 @@ fn project_evidence_event(value: &Value) -> Result<ModelEvidenceEvent, String> {
fn project_merge_request(value: &Value) -> Result<ModelMergeRequest, String> {
let merge = object(value, "Merge Request summary")?;
Ok(ModelMergeRequest {
merge_request_id: string_field(merge, "merge_request_id")?,
repository_key: string_field(merge, "repository_key")?,
state: string_field(merge, "state")?,
selector_from: optional_string(merge, "selector_from")?,
selector_to: string_field(merge, "selector_to")?,
review_status: string_field(merge, "review_status")?,
subject_ref: optional_string(merge, "subject_ref")?,
subject_ref: optional_string(merge, "current_subject_ref")?,
review_excerpt: optional_string(merge, "review_excerpt")?,
})
}
@@ -721,6 +735,7 @@ mod tests {
"complete_for_integration": false,
"missing": ["merge_request"]
},
"merge_requests": [],
"merge_request": null
}]
})).expect("Ticket query projection");
@@ -788,6 +803,7 @@ mod tests {
"implementation_reports": [],
"assignments": [],
"current_coder": null,
"merge_requests": [],
"merge_request": null,
"evidence": {
"has_merge_request": false,
+1
View File
@@ -721,6 +721,7 @@ mod tests {
"ReviewMergeRequest",
"CheckMergeRequestReadiness",
"CompleteMergeRequest",
"CompleteTicket",
] {
assert!(combined.contains(operation), "missing {operation}");
}
+12 -9
View File
@@ -78,6 +78,7 @@ struct SubWorkerSpawnInput {
#[derive(Debug, Deserialize, schemars::JsonSchema)]
struct ReviewerHandoffInput {
ticket_id: String,
merge_request_id: String,
}
#[derive(Debug, Deserialize, schemars::JsonSchema)]
@@ -369,9 +370,9 @@ fn validate_reviewer_handoff(input: &SubWorkerSpawnInput) -> Result<(), ToolErro
let Some(review) = &input.review else {
return Ok(());
};
if review.ticket_id.trim().is_empty() {
if review.ticket_id.trim().is_empty() || review.merge_request_id.trim().is_empty() {
return Err(ToolError::InvalidArgument(
"reviewer handoff requires non-empty ticket_id".to_string(),
"reviewer handoff requires non-empty ticket_id and merge_request_id".to_string(),
));
}
if input.profile.as_deref() != Some("builtin:reviewer") {
@@ -484,6 +485,7 @@ impl Tool for SubWorkerSpawnTool {
let reviewer_capability = input.review.as_ref().map(|review| {
(
review.ticket_id.clone(),
review.merge_request_id.clone(),
format!(
"{}{}",
uuid::Uuid::now_v7().simple(),
@@ -492,7 +494,7 @@ impl Tool for SubWorkerSpawnTool {
)
});
let child_workspace_context =
if let Some((ticket_id, capability_token)) = &reviewer_capability {
if let Some((ticket_id, merge_request_id, capability_token)) = &reviewer_capability {
let workspace_id =
self.workspace_context
.workspace_id()
@@ -513,6 +515,7 @@ impl Tool for SubWorkerSpawnTool {
parent_client.clone(),
ReviewerContext {
ticket_id: ticket_id.clone(),
merge_request_id: merge_request_id.clone(),
},
capability_token.clone(),
));
@@ -610,7 +613,7 @@ impl Tool for SubWorkerSpawnTool {
child_registry
.attach_parent_protocol(session.protocol_sender(), session.session_id_string());
if let Some((ticket_id, capability_token)) = &reviewer_capability {
if let Some((_ticket_id, merge_request_id, capability_token)) = &reviewer_capability {
let workspace_id = self.workspace_context.workspace_id().ok_or_else(|| {
ToolError::ExecutionFailed("review capability lost Workspace identity".to_string())
})?;
@@ -646,9 +649,9 @@ impl Tool for SubWorkerSpawnTool {
let request = WorkspaceRequest::json(
WorkspaceRequestMethod::Post,
format!(
"/api/w/{}/tickets/{}/merge-request/review-capabilities",
"/api/w/{}/merge-requests/{}/review-capabilities",
workspace_id.as_str(),
ticket_id
merge_request_id
),
body.to_string(),
);
@@ -1170,21 +1173,21 @@ mod tests {
"name":"reviewer","task":"review","profile":"builtin:reviewer",
"scope":[{"target":"work","permission":"write"}],
"command":true,
"review":{"ticket_id":"T1"}
"review":{"ticket_id":"T1","merge_request_id":"MR1"}
}))
.unwrap();
assert!(validate_reviewer_handoff(&valid).is_ok());
let wrong_profile: SubWorkerSpawnInput = serde_json::from_value(serde_json::json!({
"name":"reviewer","task":"review","profile":"builtin:coder",
"scope":[{"target":"work","permission":"write"}],
"review":{"ticket_id":"T1"}
"review":{"ticket_id":"T1","merge_request_id":"MR1"}
}))
.unwrap();
assert!(validate_reviewer_handoff(&wrong_profile).is_err());
let read_only: SubWorkerSpawnInput = serde_json::from_value(serde_json::json!({
"name":"reviewer","task":"review","profile":"builtin:reviewer",
"scope":[{"target":"work","permission":"read"}],
"review":{"ticket_id":"T1"}
"review":{"ticket_id":"T1","merge_request_id":"MR1"}
}))
.unwrap();
assert!(validate_reviewer_handoff(&read_only).is_err());
+5 -2
View File
@@ -719,6 +719,7 @@ pub trait WorkspaceClient: std::fmt::Debug + Send + Sync {
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct ReviewerContext {
pub ticket_id: String,
pub merge_request_id: String,
}
#[derive(Debug)]
@@ -771,8 +772,9 @@ impl WorkspaceClient for ReviewerChildWorkspaceClient {
let ticket_id = &self.context.ticket_id;
let ticket_query_path = format!("/api/w/{workspace_id}/tickets/query");
let ticket_show_path = format!("/api/w/{workspace_id}/tickets/{ticket_id}/show");
let merge_request_id = &self.context.merge_request_id;
let review_path =
format!("/api/w/{workspace_id}/tickets/{ticket_id}/merge-request/reviews");
format!("/api/w/{workspace_id}/merge-requests/{merge_request_id}/reviews");
let read_allowed = request.method == WorkspaceRequestMethod::Get
|| (request.method == WorkspaceRequestMethod::Post
&& (request.path == ticket_query_path || request.path == ticket_show_path));
@@ -952,6 +954,7 @@ mod reviewer_client_tests {
inner,
ReviewerContext {
ticket_id: "T1".into(),
merge_request_id: "MR1".into(),
},
"secret".into(),
)
@@ -1015,7 +1018,7 @@ mod reviewer_client_tests {
client
.execute(WorkspaceRequest::json(
WorkspaceRequestMethod::Post,
"/api/w/ws/tickets/T1/merge-request/reviews",
"/api/w/ws/merge-requests/MR1/reviews",
r#"{"decision":"approve"}"#.to_string(),
))
.expect("attested review should be forwarded");