BERTE-612: detect cross-branch contamination before merge - #279
Merged
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
|
LGTM |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #279 +/- ##
==========================================
+ Coverage 90.02% 90.21% +0.19%
==========================================
Files 81 82 +1
Lines 11064 11293 +229
==========================================
+ Hits 9960 10188 +228
- Misses 1104 1105 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Add check_source_branch_lineage(), called before jira_checks in _handle_pull_request, that detects when a source branch shares history with a higher release line that has not yet been cascaded into the target branch. The root cause this addresses: if a feature branch is rebased on a Bert-E integration commit from a higher release line (e.g. w/4) rather than directly on the target branch (e.g. development/4.3), git will silently fast-forward the target into the higher line on merge. Detection mechanism: for each branch in cascade.dst_branches that is higher than the target, compute git merge-base(src, higher). If that common ancestor is not already in the target branch's history, the source branch carries foreign commits and ForeignCommitsInSourceBranch (code 137) is raised with a clear user message. Backport guard: if src's current tip is already an ancestor of a higher branch (legitimate backport), that branch is skipped to avoid false positives. Known limitations documented in the function docstring: - No-op for hotfix PR targets (cascade.dst_branches contains only the single hotfix branch, so the loop always exits immediately) - Extended-backport false positive: if a branch was previously merged into a higher release and then extended with new commits, the backport guard misses it and a false ForeignCommitsInSourceBranch may be raised Robustness: - src.get_latest_commit() failure → skip entire check (fail open) - higher.get_latest_commit() failure → skip that branch (fail open) - git merge-base failure → skip that branch (fail open) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
matthiasL-scality
force-pushed
the
BERTE-612/check-source-branch-lineage
branch
from
August 24, 2026 16:08
3b7160a to
8a19a9c
Compare
|
LGTM |
ezekiel-alexrod
left a comment
ezekiel-alexrod
left a comment
There was a problem hiding this comment.
Since I'm not so familiar with the repo, here are my two cents.
- Add `bypass_source_branch_lineage` privileged option (utils.py, commands.py, settings.py BYPASS_LIST) so administrators can unblock false positives without manual intervention; mention it in the error template - Collect all contaminated branches before raising (single PR comment listing every foreign branch instead of one per bert-e run) - Pass pre-resolved SHAs to git merge-base for consistency with the backport guard snapshot - Fix backport guard false negative: add `src_sha != higher_tip` pre-condition so a branch created directly from a higher release line is not silently let through (git --is-ancestor is reflexive) - Add `has_cascade_higher` / `had_higher` flags for accurate no-op diagnostics (hotfix target vs all branches errored) - Move `had_higher = True` after get_latest_commit() so transient failures on all higher branches trigger the 'check was not performed' log path - Add defensive `if not merge_base: continue` guard; add explicit comment on the merge_base == higher_tip optimisation and its dependency on the prior guard - Upgrade USER_DOC.md: options table, check description with bypass note, error code 137 - README: add 'Contributing a new check' checklist making bypass a hard requirement for any new blocking check - Tests: update _make_job with settings/author_bypass stubs; update all merge_base_map keys from branch names to SHA values (implementation now passes SHAs); add tests for bypass, both-branches-contaminated, and the src_sha == higher_tip case; remove empty untracked test_compare_branches.py Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
LGTM |
…ERTE-612) Without --all, git picks a single best common ancestor. In a criss-cross merge graph two incomparable ancestors can exist; git might return the one already in dst, silently passing a contaminated PR. --all returns every merge-base, and the check now flags contamination if any of them falls outside dst. Also updates openwiki/architecture/gitwaterflow.md with the new pipeline step and adds two unit tests for the multi-merge-base code path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
LGTM |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
LGTM |
charlesprost
approved these changes
Aug 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
check_source_branch_lineage()ingitwaterflow/__init__.py, called beforecheck_integration_branchesin_handle_pull_request()ForeignCommitsInSourceBranchexception (code 137) when a source branch carries commits from a higher release line that are absent from the target branchforeign_commits_in_source_branch.mdto notify the author to rebase on the target branch directlyContext
A silent git fast-forward on
development/4.3introduced 635 commits fromdevelopment/4via ARTESCA-17922. The root cause: the feature branch had been rebased on a bert-e integration commit (w/4) instead of directly ondevelopment/4.3. From git's perspective the fast-forward was legal — no conflicts, no merge commit, no warning.How the check works
For each development branch in the cascade that is higher than
dst, computegit merge-base(src, higher). If that commit is not an ancestor ofdst, the source branch shares history with a release line thatdstdoesn't know about → block the merge.Cost: 2 git commands per higher branch in the cascade — negligible.
Test plan
test_no_higher_branches— single branch in cascade, no errortest_higher_branch_but_merge_base_in_dst— common ancestor already in dst, no errortest_merge_base_command_raises— git command failure is silently skippedtest_raises_when_merge_base_not_in_dst— contamination detected, exception raisedtest_error_contains_branch_names— exception kwargs carry src, dst, foreign branch names🤖 Generated with Claude Code