fix(create-pr): retry with live base_commit on force-push scenarios

[jamesadevine]

Closes #193.

Problem

When self.base_commit is recorded at Stage 1 (merge-base SHA), it's used as the parent commit in the ADO Push API. If the target branch is force-pushed between Stage 1 and Stage 2, that SHA may become unreachable — the push fails with HTTP 400.

The existing fallback (resolving live target branch HEAD when base_commit is None) doesn't help because base_commit is Some.

Fix

If the push fails with HTTP 400 and a recorded base_commit was used:

1. Log a warning explaining the likely cause (force-push)
2. Fall back to resolving the live target branch HEAD via the ADO refs API
3. Retry the push with the resolved commit

Testing

All 810 tests pass. This is a runtime-only change (ADO API interaction), so the fix is tested by inspection + the existing integration test coverage for the push flow.

[github-actions]

🔍 Rust PR Review

Summary: Logic is sound and solves the stated problem, but there are two issues worth addressing.

Findings

🐛 Bugs / Logic Issues

• src/safeoutputs/create_pr.rs:1235 — 400 condition is too broad

  The guard status.as_u16() == 400 && self.base_commit.is_some() triggers the fallback for any HTTP 400, not just "unreachable parent" 400s. ADO returns 400 for unrelated reasons (malformed request body, policy violations, quota errors, etc.). In those cases, the code silently swallows the real error, attempts a refs lookup, and re-submits an identical push body — which will fail again with a different or identical 400.

  The body is already in scope at this point; it's safe to add a content filter:

  } else if status.as_u16() == 400
      && self.base_commit.is_some()
      && (body.contains("unreachable") || body.contains("parent") || body.contains("invalid"))
  {

  Or at minimum, include the original body in the fallback warning so operators can tell what triggered it (it is already logged — good — but the condition itself should be narrower).

• src/safeoutputs/create_pr.rs:1262 — fallback SHA not validated

  The initial base_commit from Stage 1 is validated against the 40-char hex format (lines 1058–1062), but the fallback-resolved effective_base_commit is not. If the ADO refs API returns a malformed objectId, it gets passed straight to the push API. Consistent with the existing live-resolve path at line 1089, but this PR is a good opportunity to centralise that validation:

  let resolved = fallback_data["value"][0]["objectId"]
      .as_str()
      .context("Could not resolve target branch commit for fallback")?;
  if resolved.len() != 40 || !resolved.chars().all(|c| c.is_ascii_hexdigit()) {
      anyhow::bail!("Invalid objectId from fallback refs API: {:?}", resolved);
  }
  effective_base_commit = resolved.to_string();

✅ What Looks Good

• The fallback correctly uses Ok(ExecutionResult::failure(...)) for the ADO non-success response, consistent with the rest of the function.
• effective_base_commit as a mut local that shadows base_commit is the right pattern — base_commit stays immutable as a reference point.
• warn! before the fallback fetch and before the retry are both useful for diagnosing the race in production logs.
• The existing 400+"already exists" branch-collision guard is unaffected — the else if only fires when that guard didn't match, which is the right ordering.

Generated by Rust PR Reviewer for issue #201 · ● 394.7K · ◷

[jamesadevine]

Closing — the base_commit is a merge-base (ancestor of the target branch). Force-pushing the target branch would need to rewrite history past the merge-base AND have ADO GC the old objects between Stage 1 and Stage 2. This is effectively impossible in practice. A FIXME comment is more appropriate than retry logic.