fix: preserve completed merge request replays

This commit is contained in:
2026-08-18 08:14:43 +09:00
parent f86ae6d52f
commit 981c422122
3 changed files with 45 additions and 17 deletions
+1
View File
@@ -670,6 +670,7 @@ mod tests {
assert!(prompt.contains("guarded non-force push")); assert!(prompt.contains("guarded non-force push"));
assert!(prompt.contains("Never mutate a Server-side repository path")); assert!(prompt.contains("Never mutate a Server-side repository path"));
assert!(prompt.contains("If the repository push succeeds but completion recording fails")); assert!(prompt.contains("If the repository push succeeds but completion recording fails"));
assert!(prompt.contains("later target movement does not invalidate an idempotent replay"));
assert!(prompt.contains("closes the current assignment atomically")); assert!(prompt.contains("closes the current assignment atomically"));
assert!(!prompt.contains("sibling Coder/Reviewer Workers")); assert!(!prompt.contains("sibling Coder/Reviewer Workers"));
} }
+43 -16
View File
@@ -3893,6 +3893,20 @@ fn repository_merge_evidence_error(error: RepositoryLookupError) -> ApiError {
.into() .into()
} }
fn recorded_merge_completion<'a>(
thread: &'a [merge_request::MergeRequestThreadEvent],
operation_id: &str,
) -> Option<&'a merge_request::MergeEvent> {
thread.iter().find_map(|event| match event {
merge_request::MergeRequestThreadEvent::Merge(event)
if event.operation_id == operation_id =>
{
Some(event)
}
_ => None,
})
}
fn require_completed_target_observation( fn require_completed_target_observation(
observed: &str, observed: &str,
target_ref_before: &str, target_ref_before: &str,
@@ -4246,22 +4260,7 @@ async fn scoped_complete_merge_request(
let store = merge_request_store(&api, &workspace_id)?; let store = merge_request_store(&api, &workspace_id)?;
let mr = store.get(&workspace_id, &ticket_id)?; let mr = store.get(&workspace_id, &ticket_id)?;
let repositories = api.repository_reader(); let repositories = api.repository_reader();
if let Some(existing) = mr.thread.iter().find_map(|event| match event { if let Some(existing) = recorded_merge_completion(&mr.thread, &input.operation_id) {
merge_request::MergeRequestThreadEvent::Merge(event)
if event.operation_id == input.operation_id =>
{
Some(event)
}
_ => None,
}) {
let observed = repositories
.observe_merge_target(&mr.repository_id, Some(&mr.selector_to))
.map_err(repository_merge_evidence_error)?;
require_completed_target_observation(
&observed.commit,
&input.target_ref_before,
&input.target_ref_after,
)?;
let replay = merge_request::CompleteMergeRequest { let replay = merge_request::CompleteMergeRequest {
ticket_id, ticket_id,
operation_id: input.operation_id, operation_id: input.operation_id,
@@ -12632,6 +12631,34 @@ mod tests {
); );
} }
#[test]
fn recorded_completion_replay_is_identified_before_later_target_observation() {
let event = merge_request::MergeEvent {
event_id: "merge-event".into(),
sequence: 1,
operation_id: "operation".into(),
approval_event_id: "approval".into(),
approved_source_ref: "source".into(),
target_ref_before: "before".into(),
target_ref_after: "after".into(),
strategy: merge_request::MergeStrategy::FastForward,
resolution: merge_request::ConflictResolution::None,
merged_by: merge_request::WorkerIdentity {
runtime_id: "runtime".into(),
worker_id: "orchestrator".into(),
},
created_at: Utc::now(),
};
let thread = vec![merge_request::MergeRequestThreadEvent::Merge(event.clone())];
assert_eq!(
recorded_merge_completion(&thread, "operation"),
Some(&event)
);
assert!(recorded_merge_completion(&thread, "different").is_none());
assert!(require_completed_target_observation("later", "before", "after").is_err());
}
#[test] #[test]
fn merge_request_completion_records_only_an_observed_remote_target_update() { fn merge_request_completion_records_only_an_observed_remote_target_update() {
require_completed_target_observation("after", "before", "after").unwrap(); require_completed_target_observation("after", "before", "after").unwrap();
+1 -1
View File
@@ -8,7 +8,7 @@ The assigned Coder owns its review/fix loop and launches Reviewer SubWorkers its
Before integration, run `MergeRequestReadinessCheck` and reread the Ticket, current assignment, and exact approved subject. In the Orchestrator Workdir, use the Ticket repository `origin` transport to fetch the current target selector and immutable source selector, verify both against readiness evidence, apply the selected fast-forward or merge strategy, and validate the resulting tree. Push only a result that descends from the observed target, using a guarded non-force push whose expected old target is `target_ref_before`; reject target movement and conflicts rather than rewriting the remote. Verify the remote target now resolves exactly to `target_ref_after`, then call `MergeRequestComplete` with that before/after evidence and the authoritative approval event. Never mutate a Server-side repository path or use local `git update-ref` as integration authority. Before integration, run `MergeRequestReadinessCheck` and reread the Ticket, current assignment, and exact approved subject. In the Orchestrator Workdir, use the Ticket repository `origin` transport to fetch the current target selector and immutable source selector, verify both against readiness evidence, apply the selected fast-forward or merge strategy, and validate the resulting tree. Push only a result that descends from the observed target, using a guarded non-force push whose expected old target is `target_ref_before`; reject target movement and conflicts rather than rewriting the remote. Verify the remote target now resolves exactly to `target_ref_after`, then call `MergeRequestComplete` with that before/after evidence and the authoritative approval event. Never mutate a Server-side repository path or use local `git update-ref` as integration authority.
If the repository push succeeds but completion recording fails, do not push again or invent a new result. Retry the same completion operation and evidence: the Server accepts the already-observed exact `target_ref_after`, records `MergeResult`, moves the Ticket to `done`, and closes the current assignment atomically. Any other observed target is a stale/conflicting completion and must fail closed. If the repository push succeeds but completion recording fails, do not push again or invent a new result. Retry the same completion operation and evidence: while no completion event exists, the Server requires the target to remain at the exact `target_ref_after` before it records `MergeResult`, moves the Ticket to `done`, and closes the current assignment atomically. Once that exact operation is recorded, later target movement does not invalidate an idempotent replay of the recorded result. Before recording, any other observed target is a stale/conflicting completion and must fail closed.
Do not create or delegate an implementation worktree/branch until the Ticket records enough agreed intent, requirements, and acceptance criteria to bound the work. Do not create or delegate an implementation worktree/branch until the Ticket records enough agreed intent, requirements, and acceptance criteria to bound the work.