From 41af319b2235633febc158ff43e6261e1668cf65 Mon Sep 17 00:00:00 2001 From: Guillaume Mazoyer Date: Thu, 4 Jun 2026 13:30:08 +0200 Subject: [PATCH 1/7] test: add failing test for repo import with conflicting branch --- backend/tests/unit/git/test_git_repository.py | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/backend/tests/unit/git/test_git_repository.py b/backend/tests/unit/git/test_git_repository.py index 739cdf7af72..8731e19f281 100644 --- a/backend/tests/unit/git/test_git_repository.py +++ b/backend/tests/unit/git/test_git_repository.py @@ -12,6 +12,63 @@ from tests.helpers.test_client import dummy_async_request +async def test_create_branch_in_git_imports_branch_conflicting_with_default( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A remote branch that conflicts with the default branch must still be imported locally. + + The local branch should land at the remote tip so that downstream merge attempts can + surface the conflict at merge time, rather than aborting the entire import. + """ + repos_dir = tmp_path / "repositories" + repos_dir.mkdir() + monkeypatch.setattr(registry, "_default_branch", "main") + monkeypatch.setattr(config.SETTINGS.git, "repositories_directory", str(repos_dir)) + + source_dir = tmp_path / "source-repo" + source_dir.mkdir() + source = Repo.init(source_dir, initial_branch="main") + with source.config_writer() as cfg: + cfg.set_value("user", "name", "Test") + cfg.set_value("user", "email", "test@test.local") + target = source_dir / "data.txt" + target.write_text("line 1\nline 2\nline 3\n", encoding="utf-8") + source.index.add(["data.txt"]) + base_commit = source.index.commit("base") + + source.git.checkout("-b", "change1") + target.write_text("change1 version\nline 2\nline 3\n", encoding="utf-8") + source.index.add(["data.txt"]) + source.index.commit("change on change1") + + source.git.checkout("main") + source.git.reset("--hard", base_commit.hexsha) + target.write_text("main version\nline 2\nline 3\n", encoding="utf-8") + source.index.add(["data.txt"]) + source.index.commit("change on main") + + repository = await InfrahubRepository.new( + id=UUIDT.new(), + name="conflicting-import", + location=str(source_dir), + default_branch_name="main", + client=InfrahubClient(config=Config(requester=dummy_async_request)), + ) + + # Pre-condition: the two branches must actually conflict for this test to be meaningful + assert repository.has_conflicting_changes(target_branch="main", source_branch="change1") + + remote_branches = repository.get_branches_from_remote() + expected_commit = remote_branches["change1"].commit + + await repository.create_branch_in_git(branch_name="change1", branch_id=str(UUIDT.new()), push_origin=False) + + local_branches = repository.get_branches_from_local(include_worktree=False) + assert "change1" in local_branches + assert local_branches["change1"].commit == expected_commit + + async def test_has_conflicting_changes_no_false_positive( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, From 3482995a183197ecf3667acbb09fc4462523409a Mon Sep 17 00:00:00 2001 From: Guillaume Mazoyer Date: Thu, 4 Jun 2026 13:36:26 +0200 Subject: [PATCH 2/7] fix(git): import branches that conflict with the default branch 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. --- backend/infrahub/git/base.py | 60 +++++++++++++++++------------------- changelog/2293.fixed.md | 1 + 2 files changed, 29 insertions(+), 32 deletions(-) create mode 100644 changelog/2293.fixed.md diff --git a/backend/infrahub/git/base.py b/backend/infrahub/git/base.py index 32da2c63a50..f1ad17434b1 100644 --- a/backend/infrahub/git/base.py +++ b/backend/infrahub/git/base.py @@ -666,14 +666,24 @@ async def create_branch_in_git(self, branch_name: str, branch_id: str | None = N if branch_name in local_branches: return False - # TODO Catch potential exceptions coming from repo.git.branch & repo.git.worktree - repo.git.branch(branch_name) + remote_branch = None + if self.has_origin: + remote_branch = next( + (br for br in repo.remotes.origin.refs if br.name == f"origin/{branch_name}"), + None, + ) + + # When a matching remote branch exists, point the local branch directly at the + # remote tip. Branching from HEAD and then pulling would fail whenever the remote + # branch's history diverges from the default branch. + if remote_branch is not None: + repo.git.branch(branch_name, remote_branch.name) + else: + repo.git.branch(branch_name) + self.create_branch_worktree(branch_name=branch_name, branch_id=branch_id or branch_name) - # If there is not remote configured, we are done - # Since the branch is a match for the main branch we don't need to create a commit worktree - # If there is a remote, Check if there is an existing remote branch with the same name and if so track it. - if not self.has_origin: + if remote_branch is None: log.debug( f"Branch {branch_name} created in Git without tracking a remote branch.", repository=self.name, @@ -681,24 +691,14 @@ async def create_branch_in_git(self, branch_name: str, branch_id: str | None = N ) return True - remote_branch = [br for br in repo.remotes.origin.refs if br.name == f"origin/{branch_name}"] - - if remote_branch: - br_repo = self.get_git_repo_worktree(identifier=branch_name) - br_repo.head.reference.set_tracking_branch(remote_branch[0]) - try: - br_repo.remotes.origin.pull(branch_name) - except GitCommandError as exc: - await self._raise_enriched_error(error=exc, branch_name=branch_name) - self.create_commit_worktree(str(br_repo.head.reference.commit)) - log.debug( - f"Branch {branch_name} created in Git, tracking remote branch {remote_branch[0]}.", - repository=self.name, - branch=branch_name, - ) - else: - log.debug(f"Branch {branch_name} created in Git without tracking a remote branch.", repository=self.name) - + br_repo = self.get_git_repo_worktree(identifier=branch_name) + br_repo.head.reference.set_tracking_branch(remote_branch) + self.create_commit_worktree(str(br_repo.head.reference.commit)) + log.debug( + f"Branch {branch_name} created in Git, tracking remote branch {remote_branch}.", + repository=self.name, + branch=branch_name, + ) return True def create_commit_worktree(self, commit: str) -> bool | Worktree: @@ -900,7 +900,8 @@ def validate_remote_branch(self, branch_name: str) -> bool: ) return False - # Make sure the branch won't conflict on merge + # Surface a warning when the branch conflicts with the default branch so users + # know a future merge will be rejected, but still allow the import to proceed. try: has_conflicts = self.has_conflicting_changes(target_branch=self.default_branch, source_branch=branch_name) except GitCommandError as exc: @@ -910,17 +911,12 @@ def validate_remote_branch(self, branch_name: str) -> bool: repository=self.name, error=str(exc), ) - return False + return True if has_conflicts: get_run_logger().warning( - f"Remote branch {branch_name} will cause conflicts, they need to be resolved before importing the branch into Infrahub" + f"Remote branch {branch_name} conflicts with {self.default_branch} and will be rejected on merge until the conflict is resolved upstream" ) - return False - - # Find the commit on the remote branch - # Check out the commit in a worktree - # Validate return True diff --git a/changelog/2293.fixed.md b/changelog/2293.fixed.md new file mode 100644 index 00000000000..d808860ca1a --- /dev/null +++ b/changelog/2293.fixed.md @@ -0,0 +1 @@ +Importing a Git repository now succeeds even when one of its branches has a merge conflict against the default branch. The conflicting branch is imported as-is and a warning is logged; the conflict is reported when a user later attempts to merge the branch. From 04e239b118717678dccbcd5c1a18ba208a0909fc Mon Sep 17 00:00:00 2001 From: Guillaume Mazoyer Date: Thu, 4 Jun 2026 13:40:11 +0200 Subject: [PATCH 3/7] fix(git): address review feedback on conflict-import fix --- backend/infrahub/git/base.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/backend/infrahub/git/base.py b/backend/infrahub/git/base.py index f1ad17434b1..2c8392b04b0 100644 --- a/backend/infrahub/git/base.py +++ b/backend/infrahub/git/base.py @@ -695,7 +695,7 @@ async def create_branch_in_git(self, branch_name: str, branch_id: str | None = N br_repo.head.reference.set_tracking_branch(remote_branch) self.create_commit_worktree(str(br_repo.head.reference.commit)) log.debug( - f"Branch {branch_name} created in Git, tracking remote branch {remote_branch}.", + f"Branch {branch_name} created in Git, tracking remote branch {remote_branch.name}.", repository=self.name, branch=branch_name, ) @@ -881,8 +881,8 @@ def validate_remote_branch(self, branch_name: str) -> bool: """Process a remote branch to validate that we can use it safely. - Make sure that the branch name won't conflict with infrahub's default branch - - Make sure that a representation if the branch can be created in the database - - Make sure that there are no conflicts that would prevent it from being merged + - Make sure that a representation of the branch can be created in the database + - Warn (but do not block) when the branch would conflict with the default branch on merge """ if branch_name == registry.default_branch and branch_name != self.default_branch: # If the default branch of Infrahub and the git repository differs we map the repository From 4332e48b607ab7ff2e32f36fea5bb1d644d4ae73 Mon Sep 17 00:00:00 2001 From: Guillaume Mazoyer Date: Thu, 4 Jun 2026 15:57:57 +0200 Subject: [PATCH 4/7] test: address review nits on conflict-import tests 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. --- backend/tests/unit/git/test_git_repository.py | 82 +++++++++++++++---- 1 file changed, 67 insertions(+), 15 deletions(-) diff --git a/backend/tests/unit/git/test_git_repository.py b/backend/tests/unit/git/test_git_repository.py index 8731e19f281..3857cf37e5e 100644 --- a/backend/tests/unit/git/test_git_repository.py +++ b/backend/tests/unit/git/test_git_repository.py @@ -1,4 +1,7 @@ +import logging from pathlib import Path +from typing import Any +from unittest.mock import patch import pytest from git import Repo @@ -11,23 +14,21 @@ from tests.helpers.file_repo import MultipleStagesFileRepo from tests.helpers.test_client import dummy_async_request +PREFECT_LOGGER_NAME = "infrahub.git.base" -async def test_create_branch_in_git_imports_branch_conflicting_with_default( - tmp_path: Path, - monkeypatch: pytest.MonkeyPatch, -) -> None: - """A remote branch that conflicts with the default branch must still be imported locally. - The local branch should land at the remote tip so that downstream merge attempts can - surface the conflict at merge time, rather than aborting the entire import. - """ - repos_dir = tmp_path / "repositories" - repos_dir.mkdir() - monkeypatch.setattr(registry, "_default_branch", "main") - monkeypatch.setattr(config.SETTINGS.git, "repositories_directory", str(repos_dir)) +@pytest.fixture +def patch_prefect_logger() -> Any: + """Replace Prefect's `get_run_logger` with a stdlib logger so calls outside a flow context succeed.""" + with patch( + "infrahub.git.base.get_run_logger", + return_value=logging.getLogger(PREFECT_LOGGER_NAME), + ): + yield - source_dir = tmp_path / "source-repo" - source_dir.mkdir() + +def _build_source_with_conflicting_branches(source_dir: Path) -> Repo: + """Initialize a git source repo with `main` and `change1` whose tips edit the same lines.""" source = Repo.init(source_dir, initial_branch="main") with source.config_writer() as cfg: cfg.set_value("user", "name", "Test") @@ -47,6 +48,26 @@ async def test_create_branch_in_git_imports_branch_conflicting_with_default( target.write_text("main version\nline 2\nline 3\n", encoding="utf-8") source.index.add(["data.txt"]) source.index.commit("change on main") + return source + + +async def test_create_branch_in_git_with_conflicting_remote_lands_at_remote_tip( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A remote branch that conflicts with the default branch must still be imported locally. + + The local branch should land at the remote tip so that downstream merge attempts can + surface the conflict at merge time, rather than aborting the entire import. + """ + repos_dir = tmp_path / "repositories" + repos_dir.mkdir() + monkeypatch.setattr(registry, "_default_branch", "main") + monkeypatch.setattr(config.SETTINGS.git, "repositories_directory", str(repos_dir)) + + source_dir = tmp_path / "source-repo" + source_dir.mkdir() + _build_source_with_conflicting_branches(source_dir) repository = await InfrahubRepository.new( id=UUIDT.new(), @@ -62,13 +83,44 @@ async def test_create_branch_in_git_imports_branch_conflicting_with_default( remote_branches = repository.get_branches_from_remote() expected_commit = remote_branches["change1"].commit - await repository.create_branch_in_git(branch_name="change1", branch_id=str(UUIDT.new()), push_origin=False) + await repository.create_branch_in_git(branch_name="change1", branch_id=str(UUIDT.new())) local_branches = repository.get_branches_from_local(include_worktree=False) assert "change1" in local_branches assert local_branches["change1"].commit == expected_commit +async def test_validate_remote_branch_allows_conflicting_branch( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + patch_prefect_logger: Any, +) -> None: + """validate_remote_branch must accept a branch that conflicts with the default branch. + + Skipping the branch would prevent it from being imported. The conflict is surfaced at + merge time instead. + """ + repos_dir = tmp_path / "repositories" + repos_dir.mkdir() + monkeypatch.setattr(registry, "_default_branch", "main") + monkeypatch.setattr(config.SETTINGS.git, "repositories_directory", str(repos_dir)) + + source_dir = tmp_path / "source-repo" + source_dir.mkdir() + _build_source_with_conflicting_branches(source_dir) + + repository = await InfrahubRepository.new( + id=UUIDT.new(), + name="conflicting-validate", + location=str(source_dir), + default_branch_name="main", + client=InfrahubClient(config=Config(requester=dummy_async_request)), + ) + + assert repository.has_conflicting_changes(target_branch="main", source_branch="change1") + assert repository.validate_remote_branch(branch_name="change1") is True + + async def test_has_conflicting_changes_no_false_positive( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, From a0009a63734dd35308f7ed0011f8bd8d2bae93ca Mon Sep 17 00:00:00 2001 From: Guillaume Mazoyer Date: Thu, 4 Jun 2026 16:03:16 +0200 Subject: [PATCH 5/7] test: collapse duplicated setup, fix docstring, document Prefect exception 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. --- backend/tests/unit/git/test_git_repository.py | 58 ++++++++----------- dev/guidelines/backend/testing.md | 1 + dev/rules/testing-python.md | 1 + 3 files changed, 25 insertions(+), 35 deletions(-) diff --git a/backend/tests/unit/git/test_git_repository.py b/backend/tests/unit/git/test_git_repository.py index 3857cf37e5e..c4af10ce34d 100644 --- a/backend/tests/unit/git/test_git_repository.py +++ b/backend/tests/unit/git/test_git_repository.py @@ -27,7 +27,7 @@ def patch_prefect_logger() -> Any: yield -def _build_source_with_conflicting_branches(source_dir: Path) -> Repo: +def _build_source_with_conflicting_branches(source_dir: Path) -> None: """Initialize a git source repo with `main` and `change1` whose tips edit the same lines.""" source = Repo.init(source_dir, initial_branch="main") with source.config_writer() as cfg: @@ -48,17 +48,15 @@ def _build_source_with_conflicting_branches(source_dir: Path) -> Repo: target.write_text("main version\nline 2\nline 3\n", encoding="utf-8") source.index.add(["data.txt"]) source.index.commit("change on main") - return source -async def test_create_branch_in_git_with_conflicting_remote_lands_at_remote_tip( - tmp_path: Path, - monkeypatch: pytest.MonkeyPatch, -) -> None: - """A remote branch that conflicts with the default branch must still be imported locally. +async def _build_repository_with_conflict( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, *, name: str +) -> InfrahubRepository: + """Build an `InfrahubRepository` whose remote has `main` and a divergent `change1`. - The local branch should land at the remote tip so that downstream merge attempts can - surface the conflict at merge time, rather than aborting the entire import. + Asserts the pre-condition that the two branches actually conflict so a helper drift + fails loudly instead of silently making downstream assertions meaningless. """ repos_dir = tmp_path / "repositories" repos_dir.mkdir() @@ -71,14 +69,25 @@ async def test_create_branch_in_git_with_conflicting_remote_lands_at_remote_tip( repository = await InfrahubRepository.new( id=UUIDT.new(), - name="conflicting-import", + name=name, location=str(source_dir), default_branch_name="main", client=InfrahubClient(config=Config(requester=dummy_async_request)), ) - - # Pre-condition: the two branches must actually conflict for this test to be meaningful assert repository.has_conflicting_changes(target_branch="main", source_branch="change1") + return repository + + +async def test_create_branch_in_git_with_conflicting_remote_lands_at_remote_tip( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A remote branch that conflicts with the default branch must still be imported locally. + + The local branch should land at the remote tip so that downstream merge attempts can + surface the conflict at merge time, rather than aborting the entire import. + """ + repository = await _build_repository_with_conflict(tmp_path, monkeypatch, name="conflicting-import") remote_branches = repository.get_branches_from_remote() expected_commit = remote_branches["change1"].commit @@ -100,24 +109,7 @@ async def test_validate_remote_branch_allows_conflicting_branch( Skipping the branch would prevent it from being imported. The conflict is surfaced at merge time instead. """ - repos_dir = tmp_path / "repositories" - repos_dir.mkdir() - monkeypatch.setattr(registry, "_default_branch", "main") - monkeypatch.setattr(config.SETTINGS.git, "repositories_directory", str(repos_dir)) - - source_dir = tmp_path / "source-repo" - source_dir.mkdir() - _build_source_with_conflicting_branches(source_dir) - - repository = await InfrahubRepository.new( - id=UUIDT.new(), - name="conflicting-validate", - location=str(source_dir), - default_branch_name="main", - client=InfrahubClient(config=Config(requester=dummy_async_request)), - ) - - assert repository.has_conflicting_changes(target_branch="main", source_branch="change1") + repository = await _build_repository_with_conflict(tmp_path, monkeypatch, name="conflicting-validate") assert repository.validate_remote_branch(branch_name="change1") is True @@ -125,11 +117,7 @@ async def test_has_conflicting_changes_no_false_positive( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, ) -> None: - """has_conflicting_changes() should not report false positives when. - - file content contains conflict marker characters like '======='. - - """ + """has_conflicting_changes() must not flag a diff that only adds lines containing '======='.""" repos_dir = tmp_path / "repositories" repos_dir.mkdir() monkeypatch.setattr(registry, "_default_branch", "main") diff --git a/dev/guidelines/backend/testing.md b/dev/guidelines/backend/testing.md index 9bfd018b59c..101dd02d0dc 100644 --- a/dev/guidelines/backend/testing.md +++ b/dev/guidelines/backend/testing.md @@ -240,6 +240,7 @@ If you find yourself wanting to mock: - External HTTP APIs with no test mode (use `responses` or `httpx_mock` sparingly) - Time-dependent behavior (`freezegun`) +- Prefect's `get_run_logger` when calling a `.fn` outside a flow context — patch it to return a stdlib `logging.getLogger(...)` so `caplog` can capture output. See [Backend Testing — Logging](../../knowledge/backend/testing.md#logging-use-caplog-instead-of-mocking-get_run_logger) for the pattern. Even in these cases, prefer adapter patterns when the dependency is used widely. diff --git a/dev/rules/testing-python.md b/dev/rules/testing-python.md index ec1ee893c59..39af0e456b6 100644 --- a/dev/rules/testing-python.md +++ b/dev/rules/testing-python.md @@ -30,6 +30,7 @@ Acceptable exceptions only: - External HTTP APIs with no test mode: use `httpx_mock` or `responses` - Time-dependent behavior: `freezegun` +- Prefect's `get_run_logger`: when calling a Prefect-decorated function via `.fn` outside a flow context, patch `get_run_logger` to return a stdlib `logging.getLogger(...)` so `caplog` can capture output. See `dev/knowledge/backend/testing.md` for the full pattern. ## Parametrized tests From f4859fd9f0884c817828935c3c8087ca1edeaf6f Mon Sep 17 00:00:00 2001 From: Guillaume Mazoyer Date: Thu, 4 Jun 2026 16:09:19 +0200 Subject: [PATCH 6/7] test: tighten patch_prefect_logger type and default the repo name --- backend/tests/unit/git/test_git_repository.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/backend/tests/unit/git/test_git_repository.py b/backend/tests/unit/git/test_git_repository.py index c4af10ce34d..1ba483ef611 100644 --- a/backend/tests/unit/git/test_git_repository.py +++ b/backend/tests/unit/git/test_git_repository.py @@ -1,6 +1,6 @@ import logging +from collections.abc import Iterator from pathlib import Path -from typing import Any from unittest.mock import patch import pytest @@ -18,7 +18,7 @@ @pytest.fixture -def patch_prefect_logger() -> Any: +def patch_prefect_logger() -> Iterator[None]: """Replace Prefect's `get_run_logger` with a stdlib logger so calls outside a flow context succeed.""" with patch( "infrahub.git.base.get_run_logger", @@ -51,7 +51,7 @@ def _build_source_with_conflicting_branches(source_dir: Path) -> None: async def _build_repository_with_conflict( - tmp_path: Path, monkeypatch: pytest.MonkeyPatch, *, name: str + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, *, name: str = "conflicting-repo" ) -> InfrahubRepository: """Build an `InfrahubRepository` whose remote has `main` and a divergent `change1`. @@ -87,7 +87,7 @@ async def test_create_branch_in_git_with_conflicting_remote_lands_at_remote_tip( The local branch should land at the remote tip so that downstream merge attempts can surface the conflict at merge time, rather than aborting the entire import. """ - repository = await _build_repository_with_conflict(tmp_path, monkeypatch, name="conflicting-import") + repository = await _build_repository_with_conflict(tmp_path, monkeypatch) remote_branches = repository.get_branches_from_remote() expected_commit = remote_branches["change1"].commit @@ -102,14 +102,14 @@ async def test_create_branch_in_git_with_conflicting_remote_lands_at_remote_tip( async def test_validate_remote_branch_allows_conflicting_branch( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, - patch_prefect_logger: Any, + patch_prefect_logger: None, ) -> None: """validate_remote_branch must accept a branch that conflicts with the default branch. Skipping the branch would prevent it from being imported. The conflict is surfaced at merge time instead. """ - repository = await _build_repository_with_conflict(tmp_path, monkeypatch, name="conflicting-validate") + repository = await _build_repository_with_conflict(tmp_path, monkeypatch) assert repository.validate_remote_branch(branch_name="change1") is True From 9986232e5aca2966b8dacdc25beadd318055e6dc Mon Sep 17 00:00:00 2001 From: Guillaume Mazoyer Date: Thu, 4 Jun 2026 16:32:15 +0200 Subject: [PATCH 7/7] fix(git): wrap warning string to satisfy line-length limit --- backend/infrahub/git/base.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/backend/infrahub/git/base.py b/backend/infrahub/git/base.py index 2c8392b04b0..22c7c4fed51 100644 --- a/backend/infrahub/git/base.py +++ b/backend/infrahub/git/base.py @@ -915,7 +915,8 @@ def validate_remote_branch(self, branch_name: str) -> bool: if has_conflicts: get_run_logger().warning( - f"Remote branch {branch_name} conflicts with {self.default_branch} and will be rejected on merge until the conflict is resolved upstream" + f"Remote branch {branch_name} conflicts with {self.default_branch}; " + "the merge will be rejected until the conflict is resolved upstream" ) return True