Use the same z-stream branch creation logic as rhpkg implementation

[antbob]

The rhpkg is the reference implementation for z-stream branch creation logic. As we're still seeing a great deal of z-stream branch creation failures i've decided doing exactly what rhpkg does is the best way to address those issues. The main changes to the current logic are:

• mirrors rhpkg --allow-pending behavior
• tries -z-pending first then falls back to -z
• added commit verification
• added post-push ls-remote check
• restricted NVR walk to main branches only (note that rhpkg does not do the NVR walk)

Also some changes related to logging so when these kinda errors occur the MCP logs actually have some useful data/output to troubleshoot.

[qodo-for-packit]

PR Summary by Qodo

Align z-stream branch creation with rhpkg (-z-pending first, verify push/commit)

🐞 Bug fix ✨ Enhancement 🧪 Tests 🕐 40+ Minutes

AI Description

• Mirror rhpkg z-stream build selection: prefer -z-pending, fall back to -z.
• Verify Brew commit exists locally, push via git, then confirm branch via ls-remote.
• Restrict NVR walk to *-main sources and expand unit test coverage.

Diagram

graph TD
  A["CreateZstreamBranchTool"] --> B["get_latest_z_build"] --> C["Clone dist-git"] --> D["Verify commit exists"] --> E{"Source is *-main?"}
  E -- "yes" --> F["NVR walk (advance ref)"] --> G["git push branch"] --> H["ls-remote verify"]
  E -- "no" --> G["git push branch"] --> H["ls-remote verify"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Shell out to rhpkg for branch creation

• ➕ Guaranteed behavioral parity with the reference implementation
• ➕ Avoids re-implementing edge cases in Python
• ➖ Adds runtime dependency on rhpkg availability/version
• ➖ Harder to unit test and to run in minimal/container environments
• ➖ More brittle error handling/telemetry integration

2. Centralize z-stream logic in a shared library/module

• ➕ Keeps distgit tool thin and easier to reason about
• ➕ Encourages reuse by other tools that need z-stream build selection
• ➖ Still requires implementing and maintaining logic (no free parity)
• ➖ May be overkill unless multiple call sites exist

3. Pre-flight server permission/ACL check before push

• ➕ Could fail fast without attempting pushes that will be rejected
• ➕ May reduce noisy retries when permissions are missing
• ➖ Dist-git ACL signals can be inconsistent; may not fully predict hook/ACL behavior
• ➖ Still needs post-push verification to catch silent failures

Recommendation: Current approach is the right tradeoff: directly mirroring rhpkg’s tag preference (-z-pending then -z) while adding explicit commit existence checks and post-push ls-remote verification addresses the observed failure modes without adding an external rhpkg dependency. The only follow-up worth considering is extracting the z-stream selection/push-verification steps into a small reusable helper if additional tools need the same behavior.

Files changed (4) +278 / -129

Enhancement (1) +25 / -0

utils.pyIntroduce get_latest_z_build helper matching rhpkg allow-pending behavior +25/-0

Introduce get_latest_z_build helper matching rhpkg allow-pending behavior

• Adds get_latest_z_build() to query Koji for the latest build from {branch}-z-pending first, then {branch}-z, using inherit=True. Fetches build metadata to extract and log the source commit ref used for branch creation.

ymir/common/utils.py

Bug fix (1) +18 / -13

distgit.pyRework z-stream branch creation to use get_latest_z_build + push/commit verification +18/-13

Rework z-stream branch creation to use get_latest_z_build + push/commit verification

• Switches branch creation to always use get_latest_z_build and limits the NVR-walk ref advancement to cases where the source branch ends with -main. Adds commit existence verification before pushing, uses git push (command) and follows with ls-remote to detect silent server-side rejections.

ymir/tools/privileged/distgit.py

Tests (2) +235 / -116

test_utils.pyAdd coverage for get_latest_z_build tag preference and fallback +47/-0

Add coverage for get_latest_z_build tag preference and fallback

• Imports and adds async unit tests for a new helper that prefers {branch}-z-pending builds and falls back to {branch}-z. Verifies correct EVR extraction and errors when no builds exist.

ymir/common/tests/unit/test_utils.py

test_distgit.pyUpdate distgit z-stream tests for new build selection and push verification behavior +188/-116

Update distgit z-stream tests for new build selection and push verification behavior

• Replaces older/candidate selection mocks with get_latest_z_build and updates push mocking to use repo.git.push. Adds new tests for missing commit in clone, hook rejection error surfacing, and skipping NVR walk when the source is a z-stream branch (not -main).

ymir/tools/privileged/tests/unit/test_distgit.py

[qodo-for-packit]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. Unretriable ls-remote verification ✓ Resolved 🐞 Bug ☼ Reliability

Description

After pushing the branch, CreateZstreamBranchTool runs repo.git.ls_remote without
_retry_transient, so transient SSH/network errors during verification can abort the run even if
the push actually succeeded. This can produce false ToolError failures and leave the remote branch
created while the tool reports failure.

Code

ymir/tools/privileged/distgit.py[R239-244]

+                    await _retry_transient(
+                        lambda: asyncio.to_thread(repo.git.push, "origin", f"{ref}:refs/heads/{branch}"),
                        f"push {branch} to dist-git",
                    )
-                    for info in push_infos:
-                        if info.flags & git.remote.PushInfo.ERROR:
-                            raise RuntimeError(f"Push rejected: {info.summary.strip()}")
+                    if not await asyncio.to_thread(repo.git.ls_remote, "--heads", "origin", branch):
+                        raise RuntimeError(

Relevance

●●● Strong

Repo precedent adds _retry_transient around distgit git ops (incl ls_remote) to avoid flaky
network/SSH failures.

PR-#581

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The code introduces a new post-push ls_remote call that can raise GitCommandError on transient
transport failures, but unlike the clone/push/GitLab ls_remote calls it is not wrapped in
_retry_transient. _retry_transient is explicitly designed to retry git operations when
_is_transient_git_error matches known network/SSH failure patterns, so leaving this verification
outside retries creates an avoidable flake point.

ymir/tools/privileged/distgit.py[51-69]
ymir/tools/privileged/distgit.py[192-247]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`CreateZstreamBranchTool` wraps clone/push/GitLab polling in `_retry_transient` but performs the new post-push dist-git verification (`git ls-remote --heads origin <branch>`) without retries. If that verification hits a transient SSH/network failure, the tool fails even though the push may have already succeeded.

### Issue Context
The module already defines `_is_transient_git_error()` and `_retry_transient()` and uses them for other networked git operations, so this new network call should be treated consistently.

### Fix Focus Areas
- ymir/tools/privileged/distgit.py[239-247]
- ymir/tools/privileged/distgit.py[51-69]

### Suggested change
Wrap the post-push `repo.git.ls_remote("--heads", "origin", branch)` call in `_retry_transient` (or equivalently retry on `_is_transient_git_error`) and only raise the "silent rejection" RuntimeError when `ls-remote` succeeds but returns empty output.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context used

✅ Compliance rules (platform): 7 rules

Tip of the day

💡 Did you know, you can group findings by type and pick your Finding display, from Minimal to Full

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[antbob]

this reverts previous changes and instead applies only to older z-streams. it now behaves identically to rhpkg --create-z-stream-branch --allow-pending for older z-streams only, for leading z-streams the behavior stays as it was before so this should address most (all?) the concerns related to the original patch.

[antbob]

added z-stream branch creation info propagation to MR notes so the branching point is clear to reviewer/s.

[nforro]

It was established that -z-pending is correct also for leading Z-Streams, so we can get rid of the condition and get_latest_candidate_build(). And it would probably make sense to ditch the NVR walk (_find_latest_same_nvr_ref()) as well, @TomasKorbar WDYT?