From 3f115fcf107d940677694f5ad3a49d51afb4b8d1 Mon Sep 17 00:00:00 2001 From: Anton Bobrov Date: Fri, 7 Aug 2026 18:23:18 +0200 Subject: [PATCH 1/3] Use the same z-stream branch creation logic as rhpkg implementation --- ymir/tools/privileged/distgit.py | 28 +- .../privileged/tests/unit/test_distgit.py | 256 +++++++++++++----- 2 files changed, 209 insertions(+), 75 deletions(-) diff --git a/ymir/tools/privileged/distgit.py b/ymir/tools/privileged/distgit.py index ddf824931..be941317b 100644 --- a/ymir/tools/privileged/distgit.py +++ b/ymir/tools/privileged/distgit.py @@ -37,7 +37,6 @@ "no route to host", "broken pipe", "ssh_exchange_identification", - "failed to push some refs", ) _T = TypeVar("_T") @@ -230,7 +229,8 @@ async def _clone(): _, ref = await get_latest_z_pending_build(package, branch) else: _, ref = await get_latest_candidate_build(package, branch) - if source_branch := self._find_source_branch(repo, branch): + source_branch = self._find_source_branch(repo, branch) + if source_branch and source_branch.endswith("-main"): ref = await self._find_latest_same_nvr_ref( repo, package, @@ -240,16 +240,24 @@ async def _clone(): with tool_error_context( "Failed to push branch to dist-git", package=package, branch=branch, ref=ref ): - push_infos = await _retry_transient( - lambda: asyncio.to_thread(repo.remotes.origin.push, f"{ref}:refs/heads/{branch}"), + try: + await asyncio.to_thread(repo.commit, ref) + except Exception: + raise ToolError( + f"Commit {ref} (from latest Brew build) not found in dist-git clone of {package}" + ) from None + await _retry_transient( + lambda: asyncio.to_thread(repo.git.push, "origin", f"{ref}:refs/heads/{branch}"), f"push {branch} to dist-git", ) - if getattr(push_infos, "error", None): - logger.error("git push stderr: %s", sanitize_url(str(push_infos.error))) - for info in push_infos: - if info.flags & git.remote.PushInfo.ERROR: - logger.error("Push to dist-git rejected: %s", info.summary.strip()) - raise ToolError("Push to dist-git was rejected") + if not await _retry_transient( + lambda: asyncio.to_thread(repo.git.ls_remote, "--heads", "origin", branch), + f"verify {branch} on dist-git", + ): + raise ToolError( + f"Push appeared to succeed but branch {branch} not found " + f"on dist-git — possible silent rejection by server ACL" + ) start_time = time.monotonic() while time.monotonic() - start_time < SYNC_TIMEOUT: try: diff --git a/ymir/tools/privileged/tests/unit/test_distgit.py b/ymir/tools/privileged/tests/unit/test_distgit.py index 54c64f35c..fac6b689c 100644 --- a/ymir/tools/privileged/tests/unit/test_distgit.py +++ b/ymir/tools/privileged/tests/unit/test_distgit.py @@ -38,19 +38,17 @@ async def init_kerberos_ticket(): gitcmd = flexmock().should_receive("ls_remote").and_return(branch_exists).and_return(True).mock() flexmock(git.cmd.Git).new_instances(gitcmd) - flexmock(git.Repo).should_receive("clone_from").and_return( - flexmock( - git=gitcmd, - remotes=flexmock( - origin=flexmock(refs=[]) - .should_receive("push") - .with_args(f"{ref}:refs/heads/{branch}") - .times(0 if branch_exists else 1) - .and_return([]) - .mock(), - ), + gitcmd.should_receive("push").with_args("origin", f"{ref}:refs/heads/{branch}").times( + 0 if branch_exists else 1 + ).and_return("") + mock_repo = flexmock( + git=gitcmd, + remotes=flexmock( + origin=flexmock(refs=[]), ), ) + mock_repo.should_receive("commit").with_args(ref).and_return(flexmock()).times(0 if branch_exists else 1) + flexmock(git.Repo).should_receive("clone_from").and_return(mock_repo) flexmock(distgit_tools).should_receive("is_older_zstream").replace_with( _mock_is_older_zstream(False) @@ -108,8 +106,8 @@ async def init_kerberos_ticket(): @pytest.mark.asyncio -async def test_create_zstream_branch_push_rejected(monkeypatch): - """Push is silently rejected by gitolite — ToolError must be raised immediately.""" +async def test_create_zstream_branch_push_silently_rejected(monkeypatch): + """Push exits 0 but branch doesn't appear on dist-git — silent rejection by server ACL.""" package = "bash" branch = "rhel-10.0" user = "bot" @@ -120,23 +118,18 @@ async def init_kerberos_ticket(): flexmock(distgit_tools).should_receive("init_kerberos_ticket").replace_with(init_kerberos_ticket).once() - gitcmd = flexmock().should_receive("ls_remote").and_return(False).mock() + gitcmd = flexmock().should_receive("ls_remote").and_return(False).and_return("").mock() + gitcmd.should_receive("push").with_args("origin", f"{ref}:refs/heads/{branch}").once().and_return("") flexmock(git.cmd.Git).new_instances(gitcmd) - mock_push_info = flexmock(flags=git.remote.PushInfo.ERROR, summary="access denied") - flexmock(git.Repo).should_receive("clone_from").and_return( - flexmock( - git=gitcmd, - remotes=flexmock( - origin=flexmock(refs=[]) - .should_receive("push") - .with_args(f"{ref}:refs/heads/{branch}") - .once() - .and_return([mock_push_info]) - .mock(), - ), + mock_repo = flexmock( + git=gitcmd, + remotes=flexmock( + origin=flexmock(refs=[]), ), ) + mock_repo.should_receive("commit").with_args(ref).and_return(flexmock()).once() + flexmock(git.Repo).should_receive("clone_from").and_return(mock_repo) flexmock(distgit_tools).should_receive("is_older_zstream").replace_with( _mock_is_older_zstream(False) @@ -161,7 +154,7 @@ async def mock_get_latest_candidate_build(package, dist_git_branch): ("Connection closed by 10.2.32.39 port 22\nfatal: Could not read from remote repository.", True), ("Connection reset by peer", True), ("ssh_exchange_identification: Connection closed by remote host", True), - ("error: failed to push some refs to 'ssh://pkgs.devel.redhat.com/rpms/ruby'", True), + ("error: failed to push some refs to 'ssh://pkgs.devel.redhat.com/rpms/ruby'", False), ("Permission denied (publickey)", False), ("fatal: Authentication failed for 'https://example.com/'", False), ("fatal: Could not read from remote repository.", False), @@ -225,10 +218,10 @@ def test_find_source_branch(branch, remote_branches, expected): @pytest.mark.asyncio -async def test_create_zstream_branch_advances_ref(monkeypatch): - """When a source branch exists, the ref is advanced to the latest same-NVR commit.""" +async def test_create_zstream_branch_advances_ref_on_main(monkeypatch): + """NVR walk advances the ref when the source branch is rhel-X-main.""" package = "bash" - branch = "rhel-10.0" + branch = "rhel-10.2" user = "bot" build_ref = "aaa111" # pragma: allowlist secret advanced_ref = "bbb222" # pragma: allowlist secret @@ -252,25 +245,23 @@ async def mock_get_latest_candidate_build(package, dist_git_branch): mock_get_latest_candidate_build ).once() - mock_higher_ref = flexmock(name="origin/rhel-10.1") - flexmock(git.Repo).should_receive("clone_from").and_return( - flexmock( - git=gitcmd, - remotes=flexmock( - origin=flexmock(refs=[mock_higher_ref]) - .should_receive("push") - .with_args(f"{advanced_ref}:refs/heads/{branch}") - .once() - .and_return([]) - .mock(), - ), + mock_main_ref = flexmock(name="origin/rhel-10-main") + gitcmd.should_receive("push").with_args( + "origin", f"{advanced_ref}:refs/heads/{branch}" + ).once().and_return("") + mock_repo = flexmock( + git=gitcmd, + remotes=flexmock( + origin=flexmock(refs=[mock_main_ref]), ), ) + mock_repo.should_receive("commit").with_args(advanced_ref).and_return(flexmock()).once() + flexmock(git.Repo).should_receive("clone_from").and_return(mock_repo) async def mock_find_latest_same_nvr_ref(repo, pkg, ref, source): assert pkg == package assert ref == build_ref - assert source == "rhel-10.1" + assert source == "rhel-10-main" return advanced_ref flexmock(CreateZstreamBranchTool).should_receive("_find_latest_same_nvr_ref").replace_with( @@ -283,6 +274,56 @@ async def mock_find_latest_same_nvr_ref(repo, pkg, ref, source): assert result.startswith("Successfully") +@pytest.mark.asyncio +async def test_create_zstream_branch_skips_nvr_walk_on_zstream_source(monkeypatch): + """NVR walk is skipped when source branch is a z-stream branch (not -main).""" + package = "isns-utils" + branch = "rhel-10.0" + user = "bot" + build_ref = "a3276f38" # pragma: allowlist secret + + async def init_kerberos_ticket(): + return f"{user}@EXAMPLE.COM" + + flexmock(distgit_tools).should_receive("init_kerberos_ticket").replace_with(init_kerberos_ticket).once() + + gitcmd = flexmock().should_receive("ls_remote").and_return(False).and_return(True).mock() + flexmock(git.cmd.Git).new_instances(gitcmd) + + flexmock(distgit_tools).should_receive("is_older_zstream").replace_with( + _mock_is_older_zstream(False) + ).once() + + async def mock_get_latest_candidate_build(package, dist_git_branch): + return EVR(version="0.103", release="1.el10"), build_ref + + flexmock(distgit_tools).should_receive("get_latest_candidate_build").replace_with( + mock_get_latest_candidate_build + ).once() + + # Source branch is rhel-10.2 (a z-stream, not -main) → NVR walk must be skipped + mock_higher_ref = flexmock(name="origin/rhel-10.2") + gitcmd.should_receive("push").with_args("origin", f"{build_ref}:refs/heads/{branch}").once().and_return( + "" + ) + mock_repo = flexmock( + git=gitcmd, + remotes=flexmock( + origin=flexmock(refs=[mock_higher_ref]), + ), + ) + mock_repo.should_receive("commit").with_args(build_ref).and_return(flexmock()).once() + flexmock(git.Repo).should_receive("clone_from").and_return(mock_repo) + + # _find_latest_same_nvr_ref must NOT be called + flexmock(CreateZstreamBranchTool).should_receive("_find_latest_same_nvr_ref").times(0) + + monkeypatch.setenv("GITLAB_TOKEN", "") + + result = (await CreateZstreamBranchTool().run(input={"package": package, "branch": branch})).result + assert result.startswith("Successfully") + + def _mock_spec_commit(hexsha, spec_content): """Create a mock commit whose tree contains a spec file with given content.""" @@ -387,19 +428,15 @@ async def mock_get_latest_candidate_build(package, dist_git_branch): ).once() # No higher branches and no rhel-X-main - flexmock(git.Repo).should_receive("clone_from").and_return( - flexmock( - git=gitcmd, - remotes=flexmock( - origin=flexmock(refs=[]) - .should_receive("push") - .with_args(f"{ref}:refs/heads/{branch}") - .once() - .and_return([]) - .mock(), - ), + gitcmd.should_receive("push").with_args("origin", f"{ref}:refs/heads/{branch}").once().and_return("") + mock_repo = flexmock( + git=gitcmd, + remotes=flexmock( + origin=flexmock(refs=[]), ), ) + mock_repo.should_receive("commit").with_args(ref).and_return(flexmock()).once() + flexmock(git.Repo).should_receive("clone_from").and_return(mock_repo) monkeypatch.setenv("GITLAB_TOKEN", "") @@ -435,21 +472,110 @@ async def mock_get_latest_z_pending_build(package, dist_git_branch): mock_get_latest_z_pending_build ).once() - flexmock(git.Repo).should_receive("clone_from").and_return( - flexmock( - git=gitcmd, - remotes=flexmock( - origin=flexmock(refs=[]) - .should_receive("push") - .with_args(f"{ref}:refs/heads/{branch}") - .once() - .and_return([]) - .mock(), - ), + gitcmd.should_receive("push").with_args("origin", f"{ref}:refs/heads/{branch}").once().and_return("") + mock_repo = flexmock( + git=gitcmd, + remotes=flexmock( + origin=flexmock(refs=[]), ), ) + mock_repo.should_receive("commit").with_args(ref).and_return(flexmock()).once() + flexmock(git.Repo).should_receive("clone_from").and_return(mock_repo) monkeypatch.setenv("GITLAB_TOKEN", "") result = (await CreateZstreamBranchTool().run(input={"package": package, "branch": branch})).result assert result.startswith("Successfully") + + +@pytest.mark.asyncio +async def test_create_zstream_branch_commit_not_in_clone(monkeypatch): + """ToolError with clear message when the Brew build commit is missing from the dist-git clone.""" + package = "bash" + branch = "rhel-10.0" + user = "bot" + ref = "deadbeef1234" # pragma: allowlist secret + + async def init_kerberos_ticket(): + return f"{user}@EXAMPLE.COM" + + flexmock(distgit_tools).should_receive("init_kerberos_ticket").replace_with(init_kerberos_ticket).once() + + gitcmd = flexmock().should_receive("ls_remote").and_return(False).mock() + flexmock(git.cmd.Git).new_instances(gitcmd) + + mock_repo = flexmock( + git=gitcmd, + remotes=flexmock( + origin=flexmock(refs=[]).should_receive("push").times(0).mock(), + ), + ) + mock_repo.should_receive("commit").with_args(ref).and_raise( + git.exc.GitCommandError(["git", "rev-parse"], status=128, stderr="bad object") + ).once() + flexmock(git.Repo).should_receive("clone_from").and_return(mock_repo) + + flexmock(distgit_tools).should_receive("is_older_zstream").replace_with( + _mock_is_older_zstream(False) + ).once() + + async def mock_get_latest_candidate_build(package, dist_git_branch): + return EVR(version="1.0", release="1.el10"), ref + + flexmock(distgit_tools).should_receive("get_latest_candidate_build").replace_with( + mock_get_latest_candidate_build + ).once() + + monkeypatch.setenv("GITLAB_TOKEN", "") + + with pytest.raises(ToolError, match="not found in dist-git clone"): + await CreateZstreamBranchTool().run(input={"package": package, "branch": branch}) + + +@pytest.mark.asyncio +async def test_create_zstream_branch_push_hook_rejection(monkeypatch): + """Push rejected by server hook — full stderr surfaces in ToolError.""" + package = "bash" + branch = "rhel-10.0" + user = "bot" + ref = "123456abcdef" # pragma: allowlist secret + + async def init_kerberos_ticket(): + return f"{user}@EXAMPLE.COM" + + flexmock(distgit_tools).should_receive("init_kerberos_ticket").replace_with(init_kerberos_ticket).once() + + gitcmd = flexmock().should_receive("ls_remote").and_return(False).mock() + gitcmd.should_receive("push").with_args("origin", f"{ref}:refs/heads/{branch}").once().and_raise( + git.exc.GitCommandError( + ["git", "push"], + status=1, + stderr="remote: error: hook declined to update refs/heads/rhel-10.0", + ) + ) + flexmock(git.cmd.Git).new_instances(gitcmd) + + mock_repo = flexmock( + git=gitcmd, + remotes=flexmock( + origin=flexmock(refs=[]), + ), + ) + mock_repo.should_receive("commit").with_args(ref).and_return(flexmock()).once() + flexmock(git.Repo).should_receive("clone_from").and_return(mock_repo) + + flexmock(distgit_tools).should_receive("is_older_zstream").replace_with( + _mock_is_older_zstream(False) + ).once() + + async def mock_get_latest_candidate_build(package, dist_git_branch): + return EVR(version="1.0", release="1.el10"), ref + + flexmock(distgit_tools).should_receive("get_latest_candidate_build").replace_with( + mock_get_latest_candidate_build + ).once() + + monkeypatch.setenv("GITLAB_TOKEN", "") + + with pytest.raises(ToolError, match="hook declined"): + await CreateZstreamBranchTool().run(input={"package": package, "branch": branch}) From a63b724096b3e2977b2c144202fe24846ebde312 Mon Sep 17 00:00:00 2001 From: Anton Bobrov Date: Thu, 3 Sep 2026 16:25:03 +0200 Subject: [PATCH 2/3] make z-stream branch creation visible in MR notes --- ymir/agents/backport_agent.py | 8 ++++++-- ymir/agents/constants.py | 6 ++++++ ymir/agents/mr_consolidation_agent.py | 1 + ymir/agents/package_update_steps.py | 1 + ymir/agents/rebase_agent.py | 5 ++++- ymir/agents/rebuild_agent.py | 5 ++++- ymir/agents/tasks.py | 9 ++++++--- ymir/tools/privileged/distgit.py | 10 +++++++++- ymir/tools/privileged/tests/unit/test_distgit.py | 7 +++++-- 9 files changed, 42 insertions(+), 10 deletions(-) diff --git a/ymir/agents/backport_agent.py b/ymir/agents/backport_agent.py index f2cd88a53..71ddc1b78 100644 --- a/ymir/agents/backport_agent.py +++ b/ymir/agents/backport_agent.py @@ -29,6 +29,7 @@ I_AM_YMIR, ZSTREAM_TARGET_LABEL, format_jira_links_for_mr, + format_zstream_branch_note, mr_description_footer, ) from ymir.agents.log_agent import create_log_agent @@ -667,6 +668,7 @@ async def fork_and_prepare_dist_git(state): state.update_branch, state.fork_url, _, + state.zstream_branch_created, ) = await tasks.fork_and_prepare_dist_git( jira_issue=state.jira_issue, package=state.package, @@ -932,7 +934,8 @@ async def evaluate_inherit_source(state): f"{triage_details_text}" f"{format_jira_links_for_mr(state.jira_issue)}\n" f"{wrap_details('Backporting steps', state.backport_log[-1])}" - f"\n\n{mr_description_footer(state.package)}" + f"\n\n{format_zstream_branch_note(state.zstream_branch_created)}" + f"{mr_description_footer(state.package)}" ) state.backport_result = BackportOutputSchema( success=True, @@ -1405,7 +1408,8 @@ async def commit_push_and_open_mr(state): f"{triage_details_text}" f"{format_jira_links_for_mr(state.jira_issue)}\n" f"{wrap_details('Backporting steps', state.backport_log[-1])}" - f"\n\n{mr_description_footer(state.package)}" + f"\n\n{format_zstream_branch_note(state.zstream_branch_created)}" + f"{mr_description_footer(state.package)}" ) ( state.merge_request_url, diff --git a/ymir/agents/constants.py b/ymir/agents/constants.py index 2b1e05971..4027eaeff 100644 --- a/ymir/agents/constants.py +++ b/ymir/agents/constants.py @@ -70,6 +70,12 @@ def strip_resolves_from_mr_text(text: str) -> str: return "\n".join(result).strip("\n") +def format_zstream_branch_note(note: str | None) -> str: + if not note: + return "" + return f"> **Note:** {note}\n\n" + + def mr_description_footer(package: str) -> str: return ( "---\n" # noqa: S608 diff --git a/ymir/agents/mr_consolidation_agent.py b/ymir/agents/mr_consolidation_agent.py index db8ea5a31..19f19af04 100644 --- a/ymir/agents/mr_consolidation_agent.py +++ b/ymir/agents/mr_consolidation_agent.py @@ -510,6 +510,7 @@ async def fork_and_prepare_dist_git(state): state.update_branch, state.fork_url, _, + _, ) = await tasks.fork_and_prepare_dist_git( jira_issue=working_id, package=package, diff --git a/ymir/agents/package_update_steps.py b/ymir/agents/package_update_steps.py index 8d432ee70..4857d2ef6 100644 --- a/ymir/agents/package_update_steps.py +++ b/ymir/agents/package_update_steps.py @@ -26,6 +26,7 @@ class PackageUpdateState(BaseModel): log_result: LogOutputSchema | None = Field(default=None) merge_request_url: str | None = Field(default=None) merge_request_newly_created: bool = Field(default=False) # was the MR newly created? + zstream_branch_created: str | None = Field(default=None) class PackageUpdateStep: diff --git a/ymir/agents/rebase_agent.py b/ymir/agents/rebase_agent.py index 3caab8a79..ecfe0e16a 100644 --- a/ymir/agents/rebase_agent.py +++ b/ymir/agents/rebase_agent.py @@ -25,6 +25,7 @@ I_AM_YMIR, ZSTREAM_TARGET_LABEL, format_jira_links_for_mr, + format_zstream_branch_note, mr_description_footer, ) from ymir.agents.log_agent import create_log_agent @@ -356,6 +357,7 @@ async def fork_and_prepare_dist_git(state): state.update_branch, state.fork_url, state.fedora_clone, + state.zstream_branch_created, ) = await tasks.fork_and_prepare_dist_git( jira_issue=state.jira_issue, package=state.package, @@ -527,7 +529,8 @@ async def commit_push_and_open_mr(state): f"{format_jira_links_for_mr(all_issues)}\n" f"{wrap_details('Rebase status', state.rebase_log[-1])}" f"{consolidation_text}" - f"\n\n{mr_description_footer(state.package)}" + f"\n\n{format_zstream_branch_note(state.zstream_branch_created)}" + f"{mr_description_footer(state.package)}" ), available_tools=gateway_tools, commit_only=dry_run, diff --git a/ymir/agents/rebuild_agent.py b/ymir/agents/rebuild_agent.py index 72edafcd7..60470c4ee 100644 --- a/ymir/agents/rebuild_agent.py +++ b/ymir/agents/rebuild_agent.py @@ -14,6 +14,7 @@ I_AM_YMIR, ZSTREAM_TARGET_LABEL, format_jira_links_for_mr, + format_zstream_branch_note, mr_description_footer, ) from ymir.agents.log_agent import create_log_agent @@ -105,6 +106,7 @@ async def fork_and_prepare_dist_git(state): state.update_branch, state.fork_url, _, + state.zstream_branch_created, ) = await tasks.fork_and_prepare_dist_git( jira_issue=state.jira_issue, package=state.package, @@ -259,7 +261,8 @@ async def commit_push_and_open_mr(state): f"{side_tag_text}\n" f"{triage_details_text}" f"{consolidation_text}" - f"\n\n{mr_description_footer(state.package)}" + f"\n\n{format_zstream_branch_note(state.zstream_branch_created)}" + f"{mr_description_footer(state.package)}" ), available_tools=gateway_tools, commit_only=dry_run, diff --git a/ymir/agents/tasks.py b/ymir/agents/tasks.py index 5956c4bad..64a5740b1 100644 --- a/ymir/agents/tasks.py +++ b/ymir/agents/tasks.py @@ -239,7 +239,7 @@ async def fork_and_prepare_dist_git( agent_type: str, with_fedora: bool = False, dist_git_namespace: str | None = None, -) -> tuple[Path, str, str, Path | None]: +) -> tuple[Path, str, str, Path | None, str | None]: if not jira_issue or Path(jira_issue).is_absolute() or ".." in jira_issue: raise ValueError(f"Invalid jira_issue: {jira_issue}") # Scoped by agent_type so different agent types processing the same @@ -255,13 +255,16 @@ async def fork_and_prepare_dist_git( local_clone = working_dir / package # create_zstream_branch only applies to plain internal rhel-X.Y[.0] branches; # modular stream-* branches already exist in the rhel project. + zstream_branch_created = None if not is_cs_branch(dist_git_branch) and not is_modular_branch(dist_git_branch): - await run_tool( + result = await run_tool( "create_zstream_branch", package=package, branch=dist_git_branch, available_tools=available_tools, ) + if "already exists" not in result: + zstream_branch_created = result if await is_older_zstream(dist_git_branch): await run_tool( "clone_repository", @@ -286,7 +289,7 @@ async def fork_and_prepare_dist_git( fedora_clone = working_dir / f"{package}-fedora" if not await _clone_fedora_dist_git(package, fedora_clone): fedora_clone = None - return local_clone, update_branch, fork_url, fedora_clone + return local_clone, update_branch, fork_url, fedora_clone, zstream_branch_created async def find_leading_zstream_branch(dist_git_branch: str) -> str | None: diff --git a/ymir/tools/privileged/distgit.py b/ymir/tools/privileged/distgit.py index be941317b..e6379a1aa 100644 --- a/ymir/tools/privileged/distgit.py +++ b/ymir/tools/privileged/distgit.py @@ -214,6 +214,7 @@ async def _clone(): with tool_error_context("Failed to clone dist-git repo", package=package, clone_url=clone_url): repo = await _retry_transient(_clone, f"clone {package} from dist-git") + branch_creation_details = None if branch in [ref.name.split("/")[-1] for ref in repo.remotes.origin.refs]: # Branch already exists in dist-git but not yet mirrored to GitLab. # This happens when a previous push succeeded server-side but the SSH @@ -237,6 +238,10 @@ async def _clone(): ref, source_branch, ) + if source_branch and source_branch.endswith("-main"): + branch_creation_details = f"from {source_branch} at {ref[:12]}" + else: + branch_creation_details = f"at {ref[:12]}" with tool_error_context( "Failed to push branch to dist-git", package=package, branch=branch, ref=ref ): @@ -262,7 +267,10 @@ async def _clone(): while time.monotonic() - start_time < SYNC_TIMEOUT: try: if await asyncio.to_thread(repo.git.ls_remote, gitlab_repo_url, branch, branches=True): - return StringToolOutput(result=f"Successfully created Z-Stream branch {branch}") + msg = f"Successfully created Z-Stream branch {branch}" + if branch_creation_details: + msg += f" ({branch_creation_details})" + return StringToolOutput(result=msg) except git.exc.GitCommandError as e: if not _is_transient_git_error(e): logger.error( diff --git a/ymir/tools/privileged/tests/unit/test_distgit.py b/ymir/tools/privileged/tests/unit/test_distgit.py index fac6b689c..57092d5d2 100644 --- a/ymir/tools/privileged/tests/unit/test_distgit.py +++ b/ymir/tools/privileged/tests/unit/test_distgit.py @@ -68,6 +68,7 @@ async def mock_get_latest_candidate_build(package, dist_git_branch): assert "already exists" in result else: assert result.startswith("Successfully") + assert f"at {ref[:12]}" in result @pytest.mark.asyncio @@ -272,6 +273,8 @@ async def mock_find_latest_same_nvr_ref(repo, pkg, ref, source): result = (await CreateZstreamBranchTool().run(input={"package": package, "branch": branch})).result assert result.startswith("Successfully") + assert "from rhel-10-main" in result + assert f"at {advanced_ref[:12]}" in result @pytest.mark.asyncio @@ -528,7 +531,7 @@ async def mock_get_latest_candidate_build(package, dist_git_branch): monkeypatch.setenv("GITLAB_TOKEN", "") - with pytest.raises(ToolError, match="not found in dist-git clone"): + with pytest.raises(ToolError, match="Failed to push branch to dist-git"): await CreateZstreamBranchTool().run(input={"package": package, "branch": branch}) @@ -577,5 +580,5 @@ async def mock_get_latest_candidate_build(package, dist_git_branch): monkeypatch.setenv("GITLAB_TOKEN", "") - with pytest.raises(ToolError, match="hook declined"): + with pytest.raises(ToolError, match="Failed to push branch to dist-git"): await CreateZstreamBranchTool().run(input={"package": package, "branch": branch}) From f9b71e5acea245aad706b518c667aa7268c32c95 Mon Sep 17 00:00:00 2001 From: Anton Bobrov Date: Thu, 3 Sep 2026 17:10:42 +0200 Subject: [PATCH 3/3] do transient retries on ls_remote --- ymir/tools/privileged/distgit.py | 15 +++++++++++++-- ymir/tools/privileged/tests/unit/test_distgit.py | 12 +++++++++++- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/ymir/tools/privileged/distgit.py b/ymir/tools/privileged/distgit.py index e6379a1aa..1e58fad55 100644 --- a/ymir/tools/privileged/distgit.py +++ b/ymir/tools/privileged/distgit.py @@ -54,10 +54,11 @@ async def _retry_transient( label: str, max_retries: int = _TRANSIENT_MAX_RETRIES, base_delay: int = _TRANSIENT_BASE_DELAY, + retry_on_empty: bool = False, ) -> _T: for attempt in range(max_retries): try: - return await fn() + result = await fn() except Exception as e: if attempt < max_retries - 1 and _is_transient_git_error(e): backoff = random.uniform(0, base_delay * 2**attempt) # noqa: S311 @@ -68,6 +69,16 @@ async def _retry_transient( await asyncio.sleep(backoff) else: raise + else: + if retry_on_empty and not result and attempt < max_retries - 1: + backoff = random.uniform(0, base_delay * 2**attempt) # noqa: S311 + logger.warning( + f"{label} returned empty (attempt {attempt + 1}/{max_retries}); " + f"retrying in {backoff:.1f}s" + ) + await asyncio.sleep(backoff) + else: + return result raise AssertionError("unreachable") @@ -238,7 +249,6 @@ async def _clone(): ref, source_branch, ) - if source_branch and source_branch.endswith("-main"): branch_creation_details = f"from {source_branch} at {ref[:12]}" else: branch_creation_details = f"at {ref[:12]}" @@ -258,6 +268,7 @@ async def _clone(): if not await _retry_transient( lambda: asyncio.to_thread(repo.git.ls_remote, "--heads", "origin", branch), f"verify {branch} on dist-git", + retry_on_empty=True, ): raise ToolError( f"Push appeared to succeed but branch {branch} not found " diff --git a/ymir/tools/privileged/tests/unit/test_distgit.py b/ymir/tools/privileged/tests/unit/test_distgit.py index 57092d5d2..fbe68e63f 100644 --- a/ymir/tools/privileged/tests/unit/test_distgit.py +++ b/ymir/tools/privileged/tests/unit/test_distgit.py @@ -119,7 +119,17 @@ async def init_kerberos_ticket(): flexmock(distgit_tools).should_receive("init_kerberos_ticket").replace_with(init_kerberos_ticket).once() - gitcmd = flexmock().should_receive("ls_remote").and_return(False).and_return("").mock() + # First ls_remote checks GitLab (branch not there), subsequent calls verify + # dist-git after push — all return empty to simulate silent ACL rejection. + gitcmd = ( + flexmock() + .should_receive("ls_remote") + .and_return(False) + .and_return("") + .and_return("") + .and_return("") + .mock() + ) gitcmd.should_receive("push").with_args("origin", f"{ref}:refs/heads/{branch}").once().and_return("") flexmock(git.cmd.Git).new_instances(gitcmd)