diff --git a/crates/worker/src/prompt/catalog.rs b/crates/worker/src/prompt/catalog.rs index 7fb93c81..44f617d2 100644 --- a/crates/worker/src/prompt/catalog.rs +++ b/crates/worker/src/prompt/catalog.rs @@ -670,6 +670,7 @@ mod tests { assert!(prompt.contains("guarded non-force push")); 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("later target movement does not invalidate an idempotent replay")); assert!(prompt.contains("closes the current assignment atomically")); assert!(!prompt.contains("sibling Coder/Reviewer Workers")); } diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index b12b3c14..5f69c5e8 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -3893,6 +3893,20 @@ fn repository_merge_evidence_error(error: RepositoryLookupError) -> ApiError { .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( observed: &str, target_ref_before: &str, @@ -4246,22 +4260,7 @@ async fn scoped_complete_merge_request( let store = merge_request_store(&api, &workspace_id)?; let mr = store.get(&workspace_id, &ticket_id)?; let repositories = api.repository_reader(); - if let Some(existing) = mr.thread.iter().find_map(|event| match event { - 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, - )?; + if let Some(existing) = recorded_merge_completion(&mr.thread, &input.operation_id) { let replay = merge_request::CompleteMergeRequest { ticket_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] fn merge_request_completion_records_only_an_observed_remote_target_update() { require_completed_target_observation("after", "before", "after").unwrap(); diff --git a/resources/prompts/role/orchestrator.md b/resources/prompts/role/orchestrator.md index 68b4d9f5..31999379 100644 --- a/resources/prompts/role/orchestrator.md +++ b/resources/prompts/role/orchestrator.md @@ -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. -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.