Skip to content

Abort early when z-stream dist-git branches lag Brew builds - #733

Merged
opohorel merged 1 commit into
packit:mainfrom
opohorel:branch_consistency
Aug 6, 2026
Merged

opohorel merged 1 commit into
packit:mainfrom
opohorel:branch_consistency

Conversation

@opohorel

@opohorel opohorel commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Verify branch HEAD contains the latest candidate/z-pending source ref after clone so rebase, backport, and rebuild do not open MRs on stale z-stream state. Fail with ymir_*_errored, always comment (unless dry-run), and skip retries.

Assisted-by: Composer (Cursor)

@qodo-for-packit

Copy link
Copy Markdown

PR Summary by Qodo

Abort early when z-stream dist-git branch HEAD lags latest Brew build

🐞 Bug fix 🧪 Tests 🕐 40+ Minutes

Grey Divider

AI Description

• Add a post-clone z-stream HEAD check against the latest Brew build source ref.
• Fail fast on stale branches with consistent Jira labeling/commenting and no retries.
• Add unit coverage for stale/healthy/soft-fail consistency scenarios.
Diagram

graph TD
  A["Backport/Rebase/Rebuild agents"] --> B["fork_and_prepare_dist_git()"] --> C["Check z-stream consistency"]
  C --> D{{"Brew (candidate/z-pending)"}}
  C -->|"HEAD contains ref"| E["Proceed with workflow"]
  C -->|"stale branch"| F["Stale branch handler"] --> G["Jira label + comment"] --> H[("Redis ERROR_LIST")]

  subgraph Legend
    direction LR
    _int["Internal"] ~~~ _ext{{"External"}} ~~~ _db[("Queue")]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Auto-rebase/fast-forward branch to Brew ref
  • ➕ Could self-heal certain stale cases without human intervention
  • ➕ Avoids blocking the agent when the fix is straightforward
  • ➖ Risky: mutates dist-git branches automatically and may violate maintainer expectations
  • ➖ Hard to do safely across namespaces/permissions; increases blast radius
2. Preflight check before cloning (Brew-only)
  • ➕ Avoids cloning when the branch is known stale
  • ➕ Potentially cheaper/faster in large repos
  • ➖ Still needs a clone/fetch to validate ancestry unless you trust metadata mapping
  • ➖ More moving parts to reliably map Brew source refs to dist-git commits without local git context
3. Fetch the build ref explicitly before merge-base check
  • ➕ Distinguishes 'missing ref in clone' from 'true divergence' more accurately
  • ➕ Could reduce false positives if the clone is shallow or missing objects
  • ➖ Adds complexity and more network operations; still ends in a terminal failure for real staleness
  • ➖ Requires careful handling of auth/remotes for different dist-git namespaces

Recommendation: Current approach is the best default: validate consistency using local git ancestry after clone, and fail fast with a clear maintainer-action message. The terminal handler centralizes labeling/commenting/error-queue behavior and intentionally avoids retries since only maintainers can fix a stale branch; alternatives mainly add risk (auto-mutation) or complexity (extra fetch/mapping) for limited benefit.

Files changed (5) +353 / -3

Bug fix (4) +153 / -2
backport_agent.pyTreat stale z-stream branches as terminal backport failures +12/-0

Treat stale z-stream branches as terminal backport failures

• Catches ZStreamBranchStaleError during backport retry processing and routes it to shared terminal handling. Ensures the job marks the Jira issue as errored and records the failure rather than retrying on a maintainer-only fix.

ymir/agents/backport_agent.py

rebase_agent.pyAbort rebase retries on stale z-stream branches +12/-0

Abort rebase retries on stale z-stream branches

• Adds a dedicated exception path for ZStreamBranchStaleError to trigger consistent Jira/Redis terminal handling. Prevents opening MRs or continuing retries when the dist-git branch is behind Brew.

ymir/agents/rebase_agent.py

rebuild_agent.pyStop rebuild processing when z-stream branch HEAD is stale +12/-0

Stop rebuild processing when z-stream branch HEAD is stale

• Intercepts ZStreamBranchStaleError and applies shared terminal handling across all consolidated Jira issues. Avoids retrying rebuilds that cannot succeed until the branch is updated by maintainers.

ymir/agents/rebuild_agent.py

tasks.pyAdd z-stream branch/Brew consistency check and terminal stale-branch handler +117/-2

Add z-stream branch/Brew consistency check and terminal stale-branch handler

• Introduces ZStreamBranchStaleError plus a post-clone consistency check that compares Brew’s latest candidate/z-pending source ref to the local branch HEAD via git merge-base. Adds a shared handler to set errored labels, always comment (unless dry-run), and push structured error data to Redis ERROR_LIST without re-queuing.

ymir/agents/tasks.py

Tests (1) +200 / -1
test_tasks.pyUnit tests for z-stream consistency checks and stale-branch terminal handling +200/-1

Unit tests for z-stream consistency checks and stale-branch terminal handling

• Adds coverage for stale branches (non-ancestor and missing ref), up-to-date branches, non-z-stream skip behavior, Brew-unreachable soft-fail behavior, and older z-stream tag selection. Also tests that the stale-branch handler labels/comments correctly and respects dry-run.

ymir/agents/tests/unit/test_tasks.py

@qodo-for-packit

qodo-for-packit Bot commented Aug 4, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Remediation recommended

1. Non-critical stale label write ✗ Dismissed 🐞 Bug ☼ Reliability ⭐ New
Description
handle_zstream_branch_stale_error() swallows failures from set_jira_labels() without using its
built-in critical retry mode, so a transient Jira outage can leave the TRIAGED_* in-flight label in
place and fail to add the terminal *_ERRORED label. That can cause the JiraIssueFetcher to keep
skipping the issue as “in flight” until the stale-label safety net triggers (default 24h), delaying
maintainer visibility and recovery.
Code

ymir/agents/tasks.py[R130-133]

+            await set_jira_labels(
+                jira_issue=issue_key,
+                labels_to_add=[errored_label],
+                labels_to_remove=[triaged_label],
Relevance

●●● Strong

PR #540 established critical/retried Jira label writes for reliability and dedup; this matches that
pattern.

PR-#540

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The handler currently calls set_jira_labels() without critical=True and catches/logs any
exception, meaning a transient failure can leave the in-flight ymir_triaged_* label unchanged and
the terminal ymir_*_errored label absent. set_jira_labels()’s own docs/logic show non-critical
writes are not retried, and JiraIssueFetcher explicitly skips issues with TRIAGED_* as in-flight,
only recovering them via a time-based stale-label flip after the threshold.

ymir/agents/tasks.py[121-167]
ymir/agents/tasks.py[610-682]
ymir/jira_issue_fetcher/jira_issue_fetcher.py[64-77]
ymir/jira_issue_fetcher/jira_issue_fetcher.py[598-651]
PR-#540

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

### Issue description
`handle_zstream_branch_stale_error()` updates Jira labels via `set_jira_labels(...)` but does not request the function’s `critical=True` retry behavior, and it swallows any exception. If Jira/MCP is transiently unavailable, the issue can remain stuck with the in-flight `TRIAGED_*` label and without the terminal `*_ERRORED` dedup/outcome label.

### Issue Context
* `set_jira_labels()` explicitly supports “critical” writes with retries/backoff, while non-critical writes are single-attempt and swallowed.
* `JiraIssueFetcher` treats `TRIAGED_*` labels as in-flight and will skip those issues; the stuck-label safety net only flips stale labels after a threshold (default 24h) and only when no other Ymir outcome label is present.

### Fix Focus Areas
- ymir/agents/tasks.py[109-167]

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


2. Merge-base exit misclassified ✓ Resolved 🐞 Bug ☼ Reliability
Description
_check_zstream_branch_consistency() raises ZStreamBranchStaleError for any non-zero exit from
git merge-base --is-ancestor, even though the code comment says only exit 1/128 represent
staleness. Because the agents treat ZStreamBranchStaleError as terminal (no retries), unexpected
git failures can produce a misleading “branch maintainer must update” Jira error and stop processing
incorrectly.
Code

ymir/agents/tasks.py[R95-98]

+    # exit 1 = not ancestor; exit 128 = "not a valid commit" (ref not in repo).
+    # Both mean the branch is stale.
+    _, head_stdout, _ = await run_subprocess(["git", "rev-parse", "HEAD"], cwd=local_clone)
+    raise ZStreamBranchStaleError(package, dist_git_branch, build_source_ref, (head_stdout or "").strip())
Relevance

●●● Strong

Team often hardens terminal error handling; misclassifying git failures as stale breaks retries and
messaging.

PR-#540
PR-#611

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The consistency check explicitly documents only exit 1/128 as staleness but raises on any non-zero
exit; downstream agents catch this exception and route to a terminal handler that does not
requeue/retry, making misclassification user-visible and workflow-stopping.

ymir/agents/tasks.py[67-99]
ymir/agents/rebase_agent.py[529-560]

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

### Issue description
`_check_zstream_branch_consistency()` currently treats *every* non-zero exit code from `git merge-base --is-ancestor` as proof the branch is stale, and raises `ZStreamBranchStaleError`. This can misclassify operational git failures (bad repo state, unexpected exit code, etc.) as “stale branch”, which then triggers terminal handling (no retries) and posts a maintainer-facing message that may be incorrect.

### Issue Context
The code comment indicates only exit 1 (not ancestor) and exit 128 (invalid/missing commit) should be considered stale, but the implementation doesn’t enforce that distinction.

### Fix Focus Areas
- ymir/agents/tasks.py[67-99]
- ymir/agents/rebase_agent.py[529-560]

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


3. Modular z-streams unchecked ✗ Dismissed 🐞 Bug ≡ Correctness
Description
The new consistency check is skipped for modular stream-* branches because it only runs when
parse_zstream_branch_name(dist_git_branch) matches, but that parser only accepts plain
rhel-X.Y(.0) branch names. As a result, modular z-stream branches supported by the workflow can
still proceed on stale dist-git state without the intended early-abort protection.
Code

ymir/agents/tasks.py[R74-75]

+    if not parse_zstream_branch_name(dist_git_branch):
+        return
Relevance

●●● Strong

Likely fix; check intent is z-stream safety and modular branches should not bypass it.

PR-#429
PR-#540

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The check’s early return depends on parse_zstream_branch_name, which is implemented to match only
plain rhel-X.Y(.0) names; modular branches are explicitly recognized elsewhere as stream-* but
will not satisfy that parser, so they will skip the new check.

ymir/agents/tasks.py[67-76]
ymir/common/version_utils.py[41-58]
ymir/common/base_utils.py[315-318]
ymir/agents/tasks.py[236-239]

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

### Issue description
`_check_zstream_branch_consistency()` returns early unless `parse_zstream_branch_name(dist_git_branch)` matches. Modular dist-git branches use names like `stream-<module>-<stream>-rhel-8.10.0`, which are supported elsewhere in `fork_and_prepare_dist_git()`, but are not recognized by `parse_zstream_branch_name()`. This leaves modular branches out of the new stale-branch protection.

### Issue Context
There is already branch parsing logic (`parse_branch_name`) that can extract the embedded `rhel-X.Y` from modular branches; the consistency check (and `is_older_zstream`, if needed) should use a parser that handles modular branches or otherwise normalize modular branch names for z-stream detection.

### Fix Focus Areas
- ymir/agents/tasks.py[67-82]
- ymir/common/version_utils.py[41-94]
- ymir/common/base_utils.py[315-318]

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


Grey Divider

Context used
✅ Compliance rules (platform): 7 rules

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Previous review results

Review updated until commit 65f3dc1

Results up to commit a1426a0 ⚖️ Balanced


🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)


Remediation recommended
1. Modular z-streams unchecked ✗ Dismissed 🐞 Bug ≡ Correctness
Description
The new consistency check is skipped for modular stream-* branches because it only runs when
parse_zstream_branch_name(dist_git_branch) matches, but that parser only accepts plain
rhel-X.Y(.0) branch names. As a result, modular z-stream branches supported by the workflow can
still proceed on stale dist-git state without the intended early-abort protection.
Code

ymir/agents/tasks.py[R74-75]

+    if not parse_zstream_branch_name(dist_git_branch):
+        return
Relevance

●●● Strong

Likely fix; check intent is z-stream safety and modular branches should not bypass it.

PR-#429
PR-#540

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The check’s early return depends on parse_zstream_branch_name, which is implemented to match only
plain rhel-X.Y(.0) names; modular branches are explicitly recognized elsewhere as stream-* but
will not satisfy that parser, so they will skip the new check.

ymir/agents/tasks.py[67-76]
ymir/common/version_utils.py[41-58]
ymir/common/base_utils.py[315-318]
ymir/agents/tasks.py[236-239]

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

### Issue description
`_check_zstream_branch_consistency()` returns early unless `parse_zstream_branch_name(dist_git_branch)` matches. Modular dist-git branches use names like `stream-<module>-<stream>-rhel-8.10.0`, which are supported elsewhere in `fork_and_prepare_dist_git()`, but are not recognized by `parse_zstream_branch_name()`. This leaves modular branches out of the new stale-branch protection.

### Issue Context
There is already branch parsing logic (`parse_branch_name`) that can extract the embedded `rhel-X.Y` from modular branches; the consistency check (and `is_older_zstream`, if needed) should use a parser that handles modular branches or otherwise normalize modular branch names for z-stream detection.

### Fix Focus Areas
- ymir/agents/tasks.py[67-82]
- ymir/common/version_utils.py[41-94]
- ymir/common/base_utils.py[315-318]

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


2. Merge-base exit misclassified ✓ Resolved 🐞 Bug ☼ Reliability
Description
_check_zstream_branch_consistency() raises ZStreamBranchStaleError for any non-zero exit from
git merge-base --is-ancestor, even though the code comment says only exit 1/128 represent
staleness. Because the agents treat ZStreamBranchStaleError as terminal (no retries), unexpected
git failures can produce a misleading “branch maintainer must update” Jira error and stop processing
incorrectly.
Code

ymir/agents/tasks.py[R95-98]

+    # exit 1 = not ancestor; exit 128 = "not a valid commit" (ref not in repo).
+    # Both mean the branch is stale.
+    _, head_stdout, _ = await run_subprocess(["git", "rev-parse", "HEAD"], cwd=local_clone)
+    raise ZStreamBranchStaleError(package, dist_git_branch, build_source_ref, (head_stdout or "").strip())
Relevance

●●● Strong

Team often hardens terminal error handling; misclassifying git failures as stale breaks retries and
messaging.

PR-#540
PR-#611

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The consistency check explicitly documents only exit 1/128 as staleness but raises on any non-zero
exit; downstream agents catch this exception and route to a terminal handler that does not
requeue/retry, making misclassification user-visible and workflow-stopping.

ymir/agents/tasks.py[67-99]
ymir/agents/rebase_agent.py[529-560]

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

### Issue description
`_check_zstream_branch_consistency()` currently treats *every* non-zero exit code from `git merge-base --is-ancestor` as proof the branch is stale, and raises `ZStreamBranchStaleError`. This can misclassify operational git failures (bad repo state, unexpected exit code, etc.) as “stale branch”, which then triggers terminal handling (no retries) and posts a maintainer-facing message that may be incorrect.

### Issue Context
The code comment indicates only exit 1 (not ancestor) and exit 128 (invalid/missing commit) should be considered stale, but the implementation doesn’t enforce that distinction.

### Fix Focus Areas
- ymir/agents/tasks.py[67-99]
- ymir/agents/rebase_agent.py[529-560]

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


Qodo Logo

Comment thread ymir/agents/tasks.py
Comment thread ymir/agents/tasks.py
@opohorel
opohorel force-pushed the branch_consistency branch from a1426a0 to a17fff2 Compare August 4, 2026 12:33
@opohorel

opohorel commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator Author

/agentic_review

Comment thread ymir/agents/tasks.py
@qodo-for-packit

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit a17fff2

@lbarcziova lbarcziova left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you, this looks good!

Comment thread ymir/agents/tasks.py
Comment on lines +62 to +63
f"Please fix the branch and re-trigger by removing all ymir_ labels "
f"and adding ymir_todo."

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could in the future Ymir "fix" the branch? Or this would be likely not desired by maintainers?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe yes, but I would like to avoid it, as there could be some corner cases which I don't see right now. in the end it is not our mess to clean 😅

Verify branch HEAD contains the latest candidate/z-pending source ref
after clone so rebase, backport, and rebuild do not open MRs on stale
z-stream state. Fail with ymir_*_errored, always comment (unless dry-run),
and skip retries. Shared terminal handling lives in
tasks.handle_zstream_branch_stale_error. Only treat git merge-base exit
1/128 as stale; unexpected git failures soft-fail.

Assisted-by: Composer (Cursor)
@opohorel
opohorel force-pushed the branch_consistency branch from a17fff2 to 65f3dc1 Compare August 6, 2026 10:36
@opohorel
opohorel merged commit 4bec440 into packit:main Aug 6, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants