Hare/yoi
1
1
Fork 0
Code Actions Packages Releases Activity

fix: unblock MR follow-up and simplify orchestration guidance

Browse Source
This commit is contained in:
Hare
2026-10-17 03:35:58 +09:00
parent be1c030350
commit b1808b7c4d
15 changed files with 260 additions and 206 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/workspace-server/src/authority.rs
+1 -1
View File
@@ -169,7 +169,7 @@ impl merge_request::AssignmentSource for AuthorityMergeRequestSource {
ticket_id: &str,
) -> std::result::Result<Option<merge_request::CurrentAssignment>, String> {
self.store
.get_active_ticket_worker_assignment(workspace_id, ticket_id)
.get_current_ticket_worker_assignment(workspace_id, ticket_id)
.map(|assignment| {
assignment.map(|assignment| merge_request::CurrentAssignment {
assignment_id: assignment.assignment_id,
crates/workspace-server/src/server.rs
+61 -39
View File
@@ -17176,7 +17176,7 @@ impl merge_request::AssignmentSource for MergeRequestAssignmentSource {
ticket_id: &str,
) -> std::result::Result<Option<merge_request::CurrentAssignment>, String> {
self.store
.get_active_ticket_worker_assignment(workspace_id, ticket_id)
.get_current_ticket_worker_assignment(workspace_id, ticket_id)
.map(|value| {
value.map(|assignment| merge_request::CurrentAssignment {
assignment_id: assignment.assignment_id,
@@ -17210,7 +17210,7 @@ impl TicketSourceCommitResolver for RuntimeTicketSourceCommitResolver {
let assignment = self
.api
.store
.get_active_ticket_worker_assignment(self.api.workspace_id(), ticket_id)
.get_current_ticket_worker_assignment(self.api.workspace_id(), ticket_id)
.map_err(|error| merge_ref_diagnostic(error.into()))?
.ok_or_else(|| MergeRequestRefDiagnostic {
code: "source_ref_runtime_unavailable".into(),
@@ -17905,7 +17905,7 @@ async fn scoped_list_merge_requests(
) {
(Some(selector), Some(ticket_id)) => match api
.store
.get_active_ticket_worker_assignment(&workspace_id, ticket_id)?
.get_current_ticket_worker_assignment(&workspace_id, ticket_id)?
{
Some(assignment) => match observe_published_source_ref(
&api,
@@ -18086,7 +18086,7 @@ async fn scoped_show_merge_request(
})?;
let assignment = api
.store
.get_active_ticket_worker_assignment(&workspace_id, ticket_id)?;
.get_current_ticket_worker_assignment(&workspace_id, ticket_id)?;
let (source, target) = if let Some(refs) = integrated_refs {
refs
} else {
@@ -18170,7 +18170,7 @@ async fn scoped_merge_request_readiness(
let ticket_id = merge_request_ticket_id(&mr)?;
let assignment = api
.store
.get_active_ticket_worker_assignment(&workspace_id, &ticket_id)?;
.get_current_ticket_worker_assignment(&workspace_id, &ticket_id)?;
let (current_subject_ref, source_blocker) =
if mr.state == merge_request::MergeRequestState::Merged {
(
@@ -18245,10 +18245,10 @@ async fn scoped_open_merge_request(
let source = authenticate_worker_mutation_source(&api, &workspace_id, &headers)?;
let assignment = api
.store
.get_active_ticket_worker_assignment(&workspace_id, &ticket_id)?
.get_current_ticket_worker_assignment(&workspace_id, &ticket_id)?
.ok_or_else(|| {
Error::TicketAssignmentConflict(
"Ticket has no active Coder work assignment".into(),
"Ticket has no current implementation assignment".into(),
)
})?;
if assignment.worker.runtime_id != source.runtime_id
@@ -18370,10 +18370,10 @@ async fn scoped_repair_merge_request_selector(
let ticket_id = merge_request_ticket_id(&mr)?;
let assignment = api
.store
.get_active_ticket_worker_assignment(&workspace_id, &ticket_id)?
.get_current_ticket_worker_assignment(&workspace_id, &ticket_id)?
.ok_or_else(|| {
Error::TicketAssignmentConflict(
"Ticket has no active Coder work assignment".into(),
"Ticket has no current implementation assignment".into(),
)
})?;
let resolved_subject_ref = observe_published_source_ref(
@@ -18454,20 +18454,12 @@ async fn scoped_register_merge_request_review_capability(
}
let assignment = api
.store
.get_active_ticket_worker_assignment(&workspace_id, &ticket_id)?
.get_current_ticket_worker_assignment(&workspace_id, &ticket_id)?
.ok_or_else(|| {
Error::TicketAssignmentConflict(
"Ticket has no active Coder work assignment".into(),
"Ticket has no current implementation assignment".into(),
)
})?;
if assignment.worker.runtime_id != source.runtime_id
|| assignment.worker.worker_id != source.worker_id
{
return Err(Error::TicketAssignmentConflict(
"authenticated Worker is not current assignee".into(),
)
.into());
}
let subject_ref = match mr.state {
merge_request::MergeRequestState::Merged => merged_result_source_ref(&mr)?,
merge_request::MergeRequestState::Open => {
@@ -18572,10 +18564,10 @@ async fn scoped_submit_merge_request_review(
let mr = store.get_by_id(&workspace_id, &merge_request_id)?;
let assignment = api
.store
.get_active_ticket_worker_assignment(&workspace_id, &ticket_id)?
.get_current_ticket_worker_assignment(&workspace_id, &ticket_id)?
.ok_or_else(|| {
Error::TicketAssignmentConflict(
"Ticket has no active Coder work assignment".into(),
"Ticket has no current implementation assignment".into(),
)
})?;
let current_subject_ref = match mr.state {
@@ -18641,20 +18633,12 @@ async fn scoped_revoke_merge_request_review(
let ticket_id = merge_request_ticket_id(&mr)?;
let assignment = api
.store
.get_active_ticket_worker_assignment(&workspace_id, &ticket_id)?
.get_current_ticket_worker_assignment(&workspace_id, &ticket_id)?
.ok_or_else(|| {
Error::TicketAssignmentConflict(
"Ticket has no active Coder work assignment".into(),
"Ticket has no current implementation assignment".into(),
)
})?;
if assignment.worker.runtime_id != source.runtime_id
|| assignment.worker.worker_id != source.worker_id
{
return Err(Error::TicketAssignmentConflict(
"authenticated Worker is not current assignee".into(),
)
.into());
}
let event = store.revoke_review(merge_request::RevokeMergeRequestReview {
merge_request_id,
ticket_id,
@@ -18718,10 +18702,10 @@ async fn scoped_complete_merge_request(
}
let assignment = api
.store
.get_active_ticket_worker_assignment(&workspace_id, &ticket_id)?
.get_current_ticket_worker_assignment(&workspace_id, &ticket_id)?
.ok_or_else(|| {
Error::TicketAssignmentConflict(
"Ticket has no active Coder work assignment".into(),
"Ticket has no current implementation assignment".into(),
)
})?;
let selector = mr
@@ -54587,6 +54571,25 @@ mod tests {
.unwrap();
let orchestrator = started.worker.unwrap();
let orchestrator = RuntimeWorkerRef::new(orchestrator.runtime_id, orchestrator.worker_id);
merge_request_store(&api, TEST_WORKSPACE_ID)
.unwrap()
.open_merge_request(merge_request::OpenMergeRequest {
merge_request_id: "retained-review-context".into(),
ticket_id: ticket.id.clone(),
repository_id: test_repository_id(&api),
selector_from: "work/old-coder".into(),
selector_to: "develop".into(),
summary: String::new(),
auth: merge_request::MergeRequestAuth {
workspace_id: TEST_WORKSPACE_ID.into(),
repository_id: test_repository_id(&api),
runtime_id: old_coder.runtime_id.clone(),
worker_id: old_coder.worker_id.clone(),
assignment_id: "old-coder".into(),
},
now: Utc::now(),
})
.unwrap();
execution.take_inputs();
let before = browser_ticket_backend(&api)
.unwrap()
@@ -54704,7 +54707,7 @@ mod tests {
repository_key: "test-repository".to_string(),
selector_from: "work/old-coder".to_string(),
selector_to: "develop".to_string(),
summary: "Old responsibility is not implementation authority".to_string(),
summary: "Retained context supports MR processing after closure".to_string(),
}),
)
.await
@@ -54713,11 +54716,30 @@ mod tests {
let status = response.status();
let bytes = to_bytes(response.into_body(), usize::MAX).await.unwrap();
let body = String::from_utf8_lossy(&bytes);
assert_eq!(status, StatusCode::CONFLICT, "state={state}: {body}");
assert_eq!(status, StatusCode::BAD_GATEWAY, "state={state}: {body}");
assert!(
body.contains("Ticket has no active Coder work assignment"),
body.contains("source_ref_provider_unavailable"),
"state={state}: {body}"
);
// The Orchestrator can initiate review itself after close/reopen.
// This backend deliberately lacks ref observation: failure must be
// at the provider, not a Coder-only or active-assignment guard.
let response = scoped_register_merge_request_review_capability(
State(api.clone()),
ticket_check_headers(&orchestrator),
AxumPath((TEST_WORKSPACE_ID.into(), "retained-review-context".into())),
Json(server_api::RegisterMergeRequestReviewCapabilityRequest {
ticket_id: ticket.id.clone(),
child_session_id: "orchestrator-reviewer".into(),
capability_token: "test-review-token".into(),
}),
)
.await
.unwrap_err()
.into_response();
assert_eq!(response.status(), StatusCode::BAD_GATEWAY);
let bytes = to_bytes(response.into_body(), usize::MAX).await.unwrap();
assert!(String::from_utf8_lossy(&bytes).contains("source_ref_provider_unavailable"));
}
execute_ticket_rest_operation(
&api,
@@ -61664,7 +61686,7 @@ mod tests {
}
#[tokio::test]
async fn merge_request_assignment_authority_rejects_terminal_ticket_responsibility() {
async fn merge_request_assignment_authority_retains_closed_and_reopened_ticket_context() {
let workspace = tempfile::tempdir().unwrap();
let api = test_api(workspace.path()).await;
let worker_id = seed_cleanup_worker(&api, 3, "normal");
@@ -61705,8 +61727,8 @@ mod tests {
ticket_id,
)
.unwrap()
.is_none(),
"terminal responsibility must not authorize MR operations: {state}"
.is_some(),
"MR operations retain current implementation context: {state}"
);
}
}
crates/workspace-server/src/server/tests/ticket_evidence_tests.rs
+131 -16
View File
@@ -541,12 +541,17 @@ async fn latest_postmerge_approval_requires_exact_content_and_stored_source_resu
}
#[tokio::test]
async fn open_source_without_runtime_is_typed_unavailable_not_stale_after_rescope() {
async fn released_assignment_still_observes_provider_and_reports_real_unavailability() {
let fixture = EvidenceApiFixture::new().await;
let approval = fixture.approve("open-review");
fixture.rescope();
// End this assignment identity's work without erasing its responsibility.
// A retained Coder must not silently supply a Runtime for an open source.
// Work release must not short-circuit MR provider observation. This fixture
// has no live Runtime, so retain the same real provider failure after release.
let baseline = fixture.detail(approval.sequence, 1).await;
let source_code = baseline.source.diagnostic.unwrap().code;
let target_code = baseline.target.diagnostic.unwrap().code;
assert_ne!(source_code, "source_ref_runtime_unavailable");
assert_ne!(target_code, "target_ref_runtime_unavailable");
let connection = Connection::open(&fixture.api.config.database_path).unwrap();
assert_eq!(connection.execute(
"INSERT INTO ticket_assignment_work_releases(workspace_id,assignment_id,released_at) VALUES(?1,?2,?3)",
@@ -577,7 +582,7 @@ async fn open_source_without_runtime_is_typed_unavailable_not_stale_after_rescop
assert_eq!(
summary.source_ref_observation,
TicketSourceRefObservation::Unavailable {
code: "source_ref_runtime_unavailable".into(),
code: source_code.clone(),
}
);
assert_eq!(shown.evidence.review_status.as_deref(), Some("unknown"));
@@ -598,23 +603,14 @@ async fn open_source_without_runtime_is_typed_unavailable_not_stale_after_rescop
let listed = fixture.list().await;
assert_eq!(listed.items.len(), 1);
assert_eq!(&listed.items[0].summary, summary);
assert_eq!(
listed.items[0].ref_diagnostics[0].code,
"source_ref_runtime_unavailable"
);
assert_eq!(listed.items[0].ref_diagnostics[0].code, source_code);
let detail = fixture.detail(approval.sequence, 1).await;
assert!(detail.merge_request.thread.is_empty());
assert_eq!(detail.source.status, "unknown");
assert_eq!(detail.source.resolved_ref, None);
assert_eq!(
detail.source.diagnostic.unwrap().code,
"source_ref_runtime_unavailable"
);
assert_eq!(detail.source.diagnostic.unwrap().code, source_code);
assert_eq!(detail.target.status, "unknown");
assert_eq!(
detail.target.diagnostic.unwrap().code,
"target_ref_runtime_unavailable"
);
assert_eq!(detail.target.diagnostic.unwrap().code, target_code);
}
#[tokio::test]
@@ -1134,3 +1130,122 @@ async fn case_route_rejects_stale_content_state_and_invalid_actions_without_audi
.await;
assert_eq!(backend.show(reference.id.into()).unwrap(), closed);
}
// Production case transition, assignment adapter, review store and completion
// transaction. Provider refs are supplied at the store boundary in this fixture.
#[tokio::test]
async fn closed_and_reopened_tickets_allow_orchestrator_review_and_integration_without_reassignment()
{
for reopen in [false, true] {
let mut fixture = EvidenceApiFixture::new().await;
fixture
.backend
.update_case(&fixture.ticket_id, fixture.closure())
.unwrap();
if reopen {
fixture
.backend
.update_case(
&fixture.ticket_id,
ticket::TicketCaseUpdate {
operation_key: "reopen-for-test".into(),
expected_content_digest: fixture.content_digest(),
expected_state: ticket::TicketState::Closed,
expected_closure_operation_key: Some("evidence-ticket-closure".into()),
action: ticket::TicketCaseAction::Reopen,
explanation: "Continue the case without replacing implementation context"
.into(),
references: vec![],
author: Some("fixture".into()),
},
)
.unwrap();
}
assert!(
fixture
.api
.store
.get_active_ticket_worker_assignment(TEST_WORKSPACE_ID, &fixture.ticket_id)
.unwrap()
.is_none()
);
// The review owner is not the implementing Coder. Its own attested child
// still owns the verdict; changing the owner never grants self-review.
fixture
.store
.register_reviewer_child_session(RegisterReviewerChildSession {
workspace_id: TEST_WORKSPACE_ID.into(),
parent_runtime_id: fixture.auth.runtime_id.clone(),
parent_worker_id: fixture.auth.worker_id.clone(),
child_session_id: "coder-owned-child".into(),
reviewer_profile: "builtin:reviewer".into(),
now: evidence_time(),
})
.unwrap();
fixture.auth.runtime_id = "orchestrator-runtime".into();
fixture.auth.worker_id = "orchestrator".into();
let foreign_child = fixture.store.request_review(RequestMergeRequestReview {
merge_request_id: EVIDENCE_MR.into(),
ticket_id: fixture.ticket_id.clone(),
ticket_content_digest: fixture.content_digest(),
ticket_merge_request_subjects: fixture.subjects(),
subject_ref: EVIDENCE_SOURCE.into(),
child_session_id: "coder-owned-child".into(),
capability_token: "foreign-child-token".into(),
auth: fixture.auth.clone(),
now: evidence_time(),
});
assert!(matches!(
foreign_child,
Err(merge_request::MergeRequestError::Unauthorized(_))
));
let old = fixture.approve("old-review");
let revoked = fixture
.store
.revoke_review(merge_request::RevokeMergeRequestReview {
merge_request_id: EVIDENCE_MR.into(),
ticket_id: fixture.ticket_id.clone(),
review_event_id: old.event_id,
reason: "Request a fresh independent review".into(),
auth: fixture.auth.clone(),
now: evidence_time(),
})
.unwrap();
assert_eq!(revoked.revoked_by.worker_id, "orchestrator");
let approval = fixture.approve("fresh-review");
let ready = fixture
.store
.readiness(merge_request::ReadinessCheck {
merge_request_id: EVIDENCE_MR.into(),
ticket_id: fixture.ticket_id.clone(),
current_subject_ref: Some(EVIDENCE_SOURCE.into()),
auth: fixture.auth.clone(),
})
.unwrap();
assert!(ready.ready, "{:?}", ready.blockers);
let integrated = fixture.integrate(&approval);
assert_eq!(integrated.merged_by.worker_id, "orchestrator");
assert_eq!(integrated.approval_event_id, approval.event_id);
let shown = fixture.show().await;
assert_eq!(shown.state.as_str(), if reopen { "open" } else { "closed" });
assert_eq!(shown.merge_requests[0].state, "merged");
assert!(
fixture
.api
.store
.get_active_ticket_worker_assignment(TEST_WORKSPACE_ID, &fixture.ticket_id)
.unwrap()
.is_none()
);
assert_eq!(
fixture
.api
.store
.get_current_ticket_worker_assignment(TEST_WORKSPACE_ID, &fixture.ticket_id)
.unwrap()
.unwrap()
.assignment_id,
fixture.auth.assignment_id
);
}
}