fix(git): import branches that conflict with the default#9462
Conversation
Create the local tracking branch directly at the remote tip instead of branching from HEAD and pulling, which fails on divergent histories. The conflict is now surfaced when the branch is merged rather than silently skipping the import.
Rename the import test to make its postcondition explicit, factor the source setup into a helper, and add a direct test for validate_remote_branch's new non-skipping behavior on conflicts.
…ption Move the InfrahubRepository construction and conflict pre-condition into a shared factory used by both conflict-import tests, rewrite the awkward no-false-positive docstring, and add the Prefect get_run_logger exception to the no-mocking rule and guideline so the patch_prefect_logger fixture matches a documented exception.
github-actions
Bot
added
the
group/backend
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 5 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would require human review. This PR modifies core Git import logic to handle conflicting branches, which alters behavior during repository synchronization and merge validation; given the potential impact on data integrity and production infrastructure, human review is warranted.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Shadow auto-approve: would require human review. This PR alters core git branch creation and conflict validation logic, changing from failing on conflict to warning, which has a meaningful blast radius and could introduce subtle regressions in repository import behavior.
Re-trigger cubic
gmazoyer
force-pushed
the
gma-20260604-2293
branch
2 times, most recently
from
f4b2219 to
9986232
Compare
polmichel
left a comment
polmichel
left a comment
There was a problem hiding this comment.
LGTM, only one optional comment about assertions inside test helpers.
| default_branch_name="main", | ||
| client=InfrahubClient(config=Config(requester=dummy_async_request)), | ||
| ) | ||
| assert repository.has_conflicting_changes(target_branch="main", source_branch="change1") |
There was a problem hiding this comment.
Optional comment: I am not sure about having hidden assertions in a test helper. Either this is testing the has_conflicting_changes method of the repository in some way, in which case it should be a test, or I think we should remove this assertion since the code is already self-explanatory. Another option would be to move the assertions into the tests using this helper or/and turn them into pre-check errors. The point is that the test related assertions should fail for a reason tied to what the test is asserting against.
| error=str(exc), | ||
| ) | ||
| return False | ||
| return True |
There was a problem hiding this comment.
More a thought than a feedback: in this situation, True means "Git is not able to even determine if there is a conflict". But it also means "All good, branch is clean" and "There are merge conflicts". If we want to take actions in the future, maybe discretizing these different states besides pushing logs could be useful.
2 participants
Adding a repository whose branches don't all cleanly merge into the default branch used to either crash the git-agent at startup or silently drop the conflicting branch from the import. The repository simply couldn't be added without first fixing the upstream history.
A branch that conflicts with the default is a normal state of a real Git repo. The right time to notice the conflict is when someone actually tries to merge it, not at import. After this change the repository imports completely, the conflict is logged as a warning at import, and the existing merge path rejects the merge with the conflict surfaced to the user.
Closes #2293.
Summary by cubic
Allow importing Git repositories even when a remote branch conflicts with the default branch by creating the local branch at the remote tip and logging a warning instead of failing or skipping. Merge attempts still reject conflicts and surface them to the user. Closes #2293.
validate_remote_branchwarns on default-branch conflicts and allows import; on Git errors during the conflict check, log and continue.get_run_loggerindev/guidelines/backend/testing.mdanddev/rules/testing-python.md.Written for commit 9986232. Summary will update on new commits.