Skip to content
This repository was archived by the owner on May 19, 2026. It is now read-only.

fix(collect): gate ADO merge_commit_sha by merge strategy (closes #96)#102

Merged
bobmatnyc merged 1 commit into
bobmatnyc:mainfrom
maui314159:list-open-issues
May 19, 2026
Merged

fix(collect): gate ADO merge_commit_sha by merge strategy (closes #96)#102
bobmatnyc merged 1 commit into
bobmatnyc:mainfrom
maui314159:list-open-issues

Conversation

@maui314159
Copy link
Copy Markdown
Contributor

Summary

Narrows the ADO merge_commit_sha emission gate so non-joinable SHAs from squash / rebase completions aren't silently persisted into pull_requests.commit_shas (issue #96, follow-up to #94).

ADO's lastMergeCommit.commitId is actually the preview merge ref (refs/pull/N/merge). For noFastForward (true merge) completions it equals the commit that lands on target. For squash and rebase, ADO creates new commit(s) on target whose SHA is not lastMergeCommit.commitId. The previous code wrote those non-joinable SHAs into commit_shas, producing rows that never match against the commits table.

Strategy matrix

status mergeStrategy merge_commit_sha
completed noFastForward or absent Some(lastMergeCommit.commitId) (joinable)
completed squash / rebase / rebaseMerge None (would not join)
active / abandoned / anything else any None (existing #94 behavior)

The None outcome maps to commit_shas = "[]" in upsert_pr, which is the same pre-#94 default — no regression for downstream consumers.

Changes

Single file: src/collect/azdo/pr_fetcher.rs (+210 / -16).

  • PrRaw now deserializes both top-level mergeStrategy and nested completionOptions.mergeStrategy, preferring the top-level field when both are present.
  • From<PrRaw> for AdoPullRequest gates merge_commit_sha emission per the matrix above. Comparison is case-insensitive (mirrors the existing status check style).
  • Module-level doc block at the top of the file documents the matrix and the rationale (refs/pull/N/merge semantics).
  • Existing pr_raw_drops_merge_sha_for_non_completed_status test fixtures updated to include "mergeStrategy": "noFastForward" so the completed-status sanity branch still emits a SHA after the gate change.
  • 7 new fixture tests covering every strategy plus absent and case-insensitive variants.

Out of scope (deferred)

Test plan

  • cargo test --lib azdo::pr_fetcher — 35 / 35 pass
  • cargo test — full suite passes
  • cargo clippy --all-targets -- -D warnings — clean
  • cargo fmt --check — clean

Closes #96


Pull Request opened by Augment Code with guidance from the PR author

Agent-Id: agent-aaa02b14-3155-4b9c-ac67-3cb7f5dadff2
Linked-Note-Id: 31c3fe18-add5-4954-8f52-1a2f839d12d3
@bobmatnyc bobmatnyc marked this pull request as ready for review May 19, 2026 01:32
@bobmatnyc bobmatnyc self-requested a review as a code owner May 19, 2026 01:32
@bobmatnyc bobmatnyc merged commit 18ab9bc into bobmatnyc:main May 19, 2026
4 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Verify ADO lastMergeCommit.commitId against merge/squash/rebase completion strategies

2 participants