Skip to content

Fix changelog Resolves mismatch in MR consolidation - #702

Merged
TomasKorbar merged 2 commits into
packit:mainfrom
majamassarini:fix-consolidation-changelog-resolves
Jul 22, 2026
Merged

TomasKorbar merged 2 commits into
packit:mainfrom
majamassarini:fix-consolidation-changelog-resolves

Conversation

@majamassarini

Copy link
Copy Markdown
Member

When per_commit_flow cherry-picks base-branch commits, their changelog entries carry the base MR's Jira key. If the base was backported under a multi-CVE tracker (e.g. RHEL-154707), all CVE entries get that key instead of per-CVE keys (RHEL-190609, RHEL-190617).

Add _build_cve_to_jira_map() to derive CVE→Jira mappings from MR titles and branch names, and _fix_changelog_resolves() to correct mismatched Resolves lines in the spec after cherry-picking base commits. Also fix run_log_agent (merged strategy) to pass all collected Jira keys instead of only the first one alphabetically.

Observed in mingw-glib2 MR !37 — uril flagged two changelog mismatches (CVE-2026-58014 → RHEL-154707, CVE-2026-58016 → RHEL-154707).

Assisted-by: Claude Opus 4.6 (1M context) noreply@anthropic.com

@qodo-for-packit

Copy link
Copy Markdown

PR Summary by Qodo

Fix per-commit consolidation changelog Resolves Jira mismatch

🐞 Bug fix 🧪 Tests 🕐 40+ Minutes

Grey Divider

AI Description

• Derive CVE→Jira mappings from MR titles/branches during MR consolidation.
• Post-process spec %changelog after base-branch cherry-picks to fix wrong Resolves keys.
• Pass all collected Jira keys to the log agent (merged strategy) instead of a single key.
Diagram

graph TD
A["MR consolidation agent"] --> B["Matched MRs metadata"] --> C["_build_cve_to_jira_map"] --> D["per_commit_flow"] --> E["_fix_changelog_resolves"] --> F[("Spec file (.spec)")] --> G["git commit --amend"] --> H["run_log_agent (log prompt)"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Parse spec/changelog with an RPM-aware library
  • ➕ More robust than line-scanning (handles edge cases/format variance better)
  • ➕ Potentially clearer modeling of changelog entries
  • ➖ Adds dependency and packaging complexity
  • ➖ Still must map CVE→Jira; parser alone doesn’t solve key mismatch
2. Prevent wrong Resolves at source (avoid cherry-picking base changelog)
  • ➕ Eliminates need for post-processing + amend
  • ➕ Keeps history cleaner (no rewrite step)
  • ➖ Requires larger behavioral change to per_commit_flow (different cherry-pick strategy)
  • ➖ Harder to guarantee in all backport layouts; may break existing expectations

Recommendation: Current approach is a pragmatic, low-impact fix: infer CVE→Jira from existing MR naming conventions and correct only the affected %changelog lines after base-commit cherry-picks. Keep the post-processing scoped (only when base_commits exist and a mapping is available) and rely on the added unit tests to lock in behavior.

Files changed (2) +256 / -1

Bug fix (1) +86 / -1
mr_consolidation_agent.pyMap CVEs to Jira and fix spec changelog Resolves after cherry-picks +86/-1

Map CVEs to Jira and fix spec changelog Resolves after cherry-picks

• Adds consolidation state for a CVE→Jira mapping plus helpers to build the mapping from MR titles/branches and to rewrite mismatched Resolves lines in %changelog. Hooks the fix into per_commit_flow after cherry-picking base commits, staging the spec and amending the commit when needed. Updates run_log_agent input to pass all collected Jira keys (comma-separated) instead of a single chosen key.

ymir/agents/mr_consolidation_agent.py

Tests (1) +170 / -0
test_mr_consolidation.pyUnit tests for CVE→Jira mapping and changelog Resolves fixer +170/-0

Unit tests for CVE→Jira mapping and changelog Resolves fixer

• Adds focused test coverage for _build_cve_to_jira_map, including missing-field and multi-CVE cases. Adds tmp_path-based spec file tests for _fix_changelog_resolves to validate rewrite behavior, no-op scenarios, and entry-boundary handling.

ymir/agents/tests/unit/test_mr_consolidation.py

@qodo-for-packit

qodo-for-packit Bot commented Jul 22, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

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

Context used
✅ Compliance rules (platform): 7 rules

Grey Divider


Action required

1. CVE map includes stale MRs ✓ Resolved 🐞 Bug ≡ Correctness ⭐ New
Description
fork_and_prepare_dist_git() filters out stale MRs (not based on the current target branch HEAD) for
consolidation selection, but builds cve_to_jira_map from all_open_mrs (including those stale MRs). A
stale/duplicate MR can therefore introduce or overwrite CVE→Jira mappings and cause
_fix_changelog_resolves() to rewrite changelog Resolves lines to an incorrect Jira key.
Code

ymir/agents/mr_consolidation_agent.py[R647-650]

                state.jira_issues_collected = sorted(set(all_jira))
                state.cves_collected = sorted(set(all_cves))
                state.jira_issue = state.jira_issues_collected[0] if state.jira_issues_collected else None
+                state.cve_to_jira_map = _build_cve_to_jira_map(all_mrs)
Relevance

⭐⭐ Medium

No direct prior decisions on stale MR affecting CVE→Jira maps; only loosely related “use subset, not
all” logic (PR #662).

PR-#662

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The workflow explicitly identifies stale MRs and excludes them from current_mrs, but then
constructs the CVE→Jira mapping from the full all_mrs list instead of the filtered set. Since the
mapping is later used to rewrite spec changelog Resolves lines, stale/unrelated MR metadata can
influence the rewrite outcome.

ymir/agents/mr_consolidation_agent.py[573-602]
ymir/agents/mr_consolidation_agent.py[614-651]

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

### Issue description
In `fork_and_prepare_dist_git()`, the code explicitly detects and skips stale merge requests when building `current_mrs` (those whose merge-base != current target branch HEAD). However, it sets `state.cve_to_jira_map = _build_cve_to_jira_map(all_mrs)`, where `all_mrs` is the full open-MR list (including stale ones). Because `_build_cve_to_jira_map()` is last-write-wins per CVE, stale/duplicate MRs can supply or overwrite mappings and drive an incorrect automatic changelog rewrite.

### Issue Context
`state.cve_to_jira_map` is later consumed by `_fix_changelog_resolves()` after cherry-picking base commits, and can automatically amend a git commit.

### Fix Focus Areas
- ymir/agents/mr_consolidation_agent.py[573-651]
- ymir/agents/mr_consolidation_agent.py[184-202]

### Suggested fix
- Build the mapping from the same filtered MR set used for selection (e.g. `current_mrs` or at least `selected`) instead of `all_mrs`.
- If you intentionally need non-selected MRs, apply the same staleness filter before mapping.
- Add conflict handling in `_build_cve_to_jira_map()` (if a CVE already maps to a different Jira key, log a warning and keep a deterministic precedence rule).
- Add a unit test where `all_mrs` contains a stale MR with the same CVE but a different Jira key to ensure the stale one cannot affect the map.

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



Remediation recommended

2. Rewrite fails with indentation ✓ Resolved 🐞 Bug ≡ Correctness ⭐ New
Description
_fix_changelog_resolves() matches Resolves lines using line.strip() but performs the substitution on
the original line with a start-anchored regex, so indented "- Resolves:" bullets can match yet not
be rewritten. This can still trigger a spec write + git commit --amend, leaving the mismatched Jira
key unfixed but recorded as a modification.
Code

ymir/agents/mr_consolidation_agent.py[R240-256]

+        cves_found = re.findall(r"CVE-\d{4}-\d+", stripped)
+        if cves_found:
+            cves_in_current_entry.extend(c for c in cves_found if c not in cves_in_current_entry)
+
+        resolves_match = re.match(r"^(-\s*Resolves:\s*)(RHEL-\d+)(.*)", stripped)
+        if cves_in_current_entry and resolves_match:
+            mapped_keys = {cve_to_jira[c] for c in cves_in_current_entry if c in cve_to_jira}
+            if len(mapped_keys) == 1:
+                correct_jira = mapped_keys.pop()
+                current_jira = resolves_match.group(2)
+                if current_jira != correct_jira:
+                    lines[i] = re.sub(
+                        r"^(-\s*Resolves:\s*)(RHEL-\d+)",
+                        rf"\g<1>{correct_jira}",
+                        line,
+                    )
+                    modified = True
Relevance

⭐⭐⭐ High

Team previously accepted widening Resolves/Related regexes for optional bullet/whitespace prefixes
(PR #477).

PR-#477

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The function matches Resolves lines on a stripped version of the line, but rewrites using a regex
that requires the '-' to be at the start of the *unstripped* line; this can produce a no-op rewrite
while still reporting modification. The per_commit_flow caller amends the commit whenever the
function returns True.

ymir/agents/mr_consolidation_agent.py[228-257]
ymir/agents/mr_consolidation_agent.py[861-914]
PR-#477

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

### Issue description
`_fix_changelog_resolves()` computes `resolves_match` against `stripped = line.strip()`, but then runs `re.sub()` against the original `line` using a pattern anchored to the beginning of the string (`^(-\s*Resolves:...)`). If the original line had leading whitespace (e.g. `"  - Resolves: ..."`), the match can succeed on `stripped` while the substitution fails on `line`, yet `modified=True` is still set and the caller amends the commit.

### Issue Context
This function is invoked after cherry-picking base commits and, when it returns `True`, the workflow stages the spec and runs `git commit --amend --no-edit`.

### Fix Focus Areas
- ymir/agents/mr_consolidation_agent.py[205-267]
- ymir/agents/mr_consolidation_agent.py[861-914]

### Suggested fix
- Make the substitution pattern accept leading whitespace, e.g. `r"^(\s*-\s*Resolves:\s*)(RHEL-\d+)"`.
- Alternatively, run both the match and the substitution on the same string (`line`) and use `re.match()` directly on `line` (with optional leading whitespace).
- Only set `modified=True` if the replacement actually changes the line (compare before/after), to avoid unnecessary amend commits.
- Add a unit test where the Resolves line is indented (leading spaces) to prevent regressions.

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


3. Case-sensitive branch Jira extraction ✓ Resolved 🐞 Bug ≡ Correctness
Description
_build_cve_to_jira_map() only matches Jira keys in branch names using a case-sensitive "RHEL-\d+"
regex, so branches containing lowercase keys (e.g. "rhel-123456") produce no mapping and the
changelog mismatch correction is silently skipped for those CVEs. This is inconsistent with other
parts of the repo that accept lowercase Jira keys as inputs.
Code

ymir/agents/mr_consolidation_agent.py[R194-201]

+        branch = mr.get("source_branch", "")
+        jira_match = re.search(r"(RHEL-\d+)", branch)
+        if not jira_match:
+            continue
+        jira_key = jira_match.group(1)
+        title = mr.get("title", "")
+        for cve_id in re.findall(r"CVE-\d{4}-\d+", title):
+            cve_to_jira[cve_id] = jira_key
Relevance

⭐⭐⭐ High

Team previously accepted case-insensitive Jira matching/normalization (rhel-123456) to avoid misses.

PR-#655
PR-#129

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new mapping code is explicitly case-sensitive, while branch names are derived from the raw
jira_issue string and other repo tests show lowercase Jira keys are treated as valid inputs.

ymir/agents/mr_consolidation_agent.py[184-202]
ymir/agents/tasks.py[121-151]
ymir/tools/unprivileged/tests/unit/test_wicked_git.py[198-206]
PR-#655

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

### Issue description
`_build_cve_to_jira_map()` uses `re.search(r"(RHEL-\d+)", branch)` which fails to extract Jira keys when the branch name contains a lowercase key (e.g. `...-rhel-123456`). That yields an empty mapping and prevents `_fix_changelog_resolves()` from running.

### Issue Context
Branch names are constructed from `jira_issue` without normalizing case, and tests demonstrate lowercase Jira keys are supported inputs.

### Fix Focus Areas
- ymir/agents/mr_consolidation_agent.py[184-202]
- ymir/agents/tasks.py[121-151]

### Suggested fix
- Use a case-insensitive match: `re.search(r"(RHEL-\d+)", branch, re.IGNORECASE)`.
- Normalize captured Jira keys to canonical form (e.g. `.upper()`) before storing in the map.
- (Optional) Do the same for CVE extraction (`re.IGNORECASE`) and normalize to uppercase to be robust against `cve-...` in titles.

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


4. Overwrites extra Jira keys ✓ Resolved 🐞 Bug ≡ Correctness
Description
_fix_changelog_resolves() replaces every "RHEL-\d+" on a matching Resolves line, so a line
containing multiple Jira keys can have unrelated keys overwritten. It also tracks only the first CVE
seen in a changelog entry, so entries mentioning multiple CVEs can be rewritten using the wrong
mapping or only partially corrected.
Code

ymir/agents/mr_consolidation_agent.py[R229-240]

+        cves_found = re.findall(r"CVE-\d{4}-\d+", stripped)
+        if cves_found:
+            cve_in_current_entry = cves_found[0]
+
+        if cve_in_current_entry and re.match(r"^-\s*Resolves:\s*RHEL-\d+", stripped):
+            correct_jira = cve_to_jira.get(cve_in_current_entry)
+            if correct_jira:
+                current_jira = re.search(r"RHEL-\d+", stripped)
+                if current_jira and current_jira.group(0) != correct_jira:
+                    lines[i] = re.sub(r"RHEL-\d+", correct_jira, line)
+                    modified = True
+            cve_in_current_entry = None
Relevance

⭐⭐ Medium

No prior accepted/rejected guidance on single-occurrence Jira substitution or multi-CVE entry
handling in changelog rewriting.

PR-#477
PR-#689

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The function selects only the first CVE in an entry and performs an unbounded substitution across
the entire line, which will rewrite every Jira key token present on that line.

ymir/agents/mr_consolidation_agent.py[205-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
`_fix_changelog_resolves()` currently (a) selects only the first CVE found in an entry and (b) uses a broad `re.sub(r"RHEL-\d+", ...)` which replaces **all** Jira keys on the line. This can corrupt lines that contain multiple Jira keys and can mis-handle changelog entries that mention more than one CVE.

### Issue Context
The function is intended to correct mismatched `- Resolves: RHEL-...` lines after cherry-picking base commits.

### Fix Focus Areas
- ymir/agents/mr_consolidation_agent.py[205-247]

### Suggested fix
- Replace only the Jira key associated with the `Resolves:` field (e.g., use a capturing regex like `(^-\s*Resolves:\s*)(RHEL-\d+)` and substitute only group 2, preserving any trailing text/extra keys).
- Track *all* CVEs encountered within the current changelog entry (until the next `*` header). When hitting a Resolves line:
 - If exactly one CVE is present, apply its mapping.
 - If multiple CVEs are present and mappings disagree, either skip rewriting and log a warning, or apply a deterministic rule (and add a unit test).
- Add unit tests for:
 - A Resolves line containing multiple Jira keys.
 - An entry containing multiple CVEs.

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


Grey Divider

Previous review results

Review updated until commit 37ecff5

Results up to commit 2cff3f3 ⚖️ Balanced


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


Remediation recommended
1. Case-sensitive branch Jira extraction ✓ Resolved 🐞 Bug ≡ Correctness
Description
_build_cve_to_jira_map() only matches Jira keys in branch names using a case-sensitive "RHEL-\d+"
regex, so branches containing lowercase keys (e.g. "rhel-123456") produce no mapping and the
changelog mismatch correction is silently skipped for those CVEs. This is inconsistent with other
parts of the repo that accept lowercase Jira keys as inputs.
Code

ymir/agents/mr_consolidation_agent.py[R194-201]

+        branch = mr.get("source_branch", "")
+        jira_match = re.search(r"(RHEL-\d+)", branch)
+        if not jira_match:
+            continue
+        jira_key = jira_match.group(1)
+        title = mr.get("title", "")
+        for cve_id in re.findall(r"CVE-\d{4}-\d+", title):
+            cve_to_jira[cve_id] = jira_key
Relevance

⭐⭐⭐ High

Team previously accepted case-insensitive Jira matching/normalization (rhel-123456) to avoid misses.

PR-#655
PR-#129

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new mapping code is explicitly case-sensitive, while branch names are derived from the raw
jira_issue string and other repo tests show lowercase Jira keys are treated as valid inputs.

ymir/agents/mr_consolidation_agent.py[184-202]
ymir/agents/tasks.py[121-151]
ymir/tools/unprivileged/tests/unit/test_wicked_git.py[198-206]
PR-#655

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

### Issue description
`_build_cve_to_jira_map()` uses `re.search(r"(RHEL-\d+)", branch)` which fails to extract Jira keys when the branch name contains a lowercase key (e.g. `...-rhel-123456`). That yields an empty mapping and prevents `_fix_changelog_resolves()` from running.

### Issue Context
Branch names are constructed from `jira_issue` without normalizing case, and tests demonstrate lowercase Jira keys are supported inputs.

### Fix Focus Areas
- ymir/agents/mr_consolidation_agent.py[184-202]
- ymir/agents/tasks.py[121-151]

### Suggested fix
- Use a case-insensitive match: `re.search(r"(RHEL-\d+)", branch, re.IGNORECASE)`.
- Normalize captured Jira keys to canonical form (e.g. `.upper()`) before storing in the map.
- (Optional) Do the same for CVE extraction (`re.IGNORECASE`) and normalize to uppercase to be robust against `cve-...` in titles.

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


2. Overwrites extra Jira keys ✓ Resolved 🐞 Bug ≡ Correctness
Description
_fix_changelog_resolves() replaces every "RHEL-\d+" on a matching Resolves line, so a line
containing multiple Jira keys can have unrelated keys overwritten. It also tracks only the first CVE
seen in a changelog entry, so entries mentioning multiple CVEs can be rewritten using the wrong
mapping or only partially corrected.
Code

ymir/agents/mr_consolidation_agent.py[R229-240]

+        cves_found = re.findall(r"CVE-\d{4}-\d+", stripped)
+        if cves_found:
+            cve_in_current_entry = cves_found[0]
+
+        if cve_in_current_entry and re.match(r"^-\s*Resolves:\s*RHEL-\d+", stripped):
+            correct_jira = cve_to_jira.get(cve_in_current_entry)
+            if correct_jira:
+                current_jira = re.search(r"RHEL-\d+", stripped)
+                if current_jira and current_jira.group(0) != correct_jira:
+                    lines[i] = re.sub(r"RHEL-\d+", correct_jira, line)
+                    modified = True
+            cve_in_current_entry = None
Relevance

⭐⭐ Medium

No prior accepted/rejected guidance on single-occurrence Jira substitution or multi-CVE entry
handling in changelog rewriting.

PR-#477
PR-#689

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The function selects only the first CVE in an entry and performs an unbounded substitution across
the entire line, which will rewrite every Jira key token present on that line.

ymir/agents/mr_consolidation_agent.py[205-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
`_fix_changelog_resolves()` currently (a) selects only the first CVE found in an entry and (b) uses a broad `re.sub(r"RHEL-\d+", ...)` which replaces **all** Jira keys on the line. This can corrupt lines that contain multiple Jira keys and can mis-handle changelog entries that mention more than one CVE.

### Issue Context
The function is intended to correct mismatched `- Resolves: RHEL-...` lines after cherry-picking base commits.

### Fix Focus Areas
- ymir/agents/mr_consolidation_agent.py[205-247]

### Suggested fix
- Replace only the Jira key associated with the `Resolves:` field (e.g., use a capturing regex like `(^-\s*Resolves:\s*)(RHEL-\d+)` and substitute only group 2, preserving any trailing text/extra keys).
- Track *all* CVEs encountered within the current changelog entry (until the next `*` header). When hitting a Resolves line:
 - If exactly one CVE is present, apply its mapping.
 - If multiple CVEs are present and mappings disagree, either skip rewriting and log a warning, or apply a deterministic rule (and add a unit test).
- Add unit tests for:
 - A Resolves line containing multiple Jira keys.
 - An entry containing multiple CVEs.

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


Qodo Logo

Comment thread ymir/agents/mr_consolidation_agent.py Outdated
Comment thread ymir/agents/mr_consolidation_agent.py Outdated
@majamassarini
majamassarini force-pushed the fix-consolidation-changelog-resolves branch from 2cff3f3 to a9cf03e Compare July 22, 2026 10:54
@majamassarini

Copy link
Copy Markdown
Member Author

/agentic_review

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

Copy link
Copy Markdown

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

@majamassarini
majamassarini force-pushed the fix-consolidation-changelog-resolves branch from a9cf03e to 7d35b21 Compare July 22, 2026 11:12
@TomasKorbar
TomasKorbar self-requested a review July 22, 2026 12:10
@TomasKorbar
TomasKorbar force-pushed the fix-consolidation-changelog-resolves branch 2 times, most recently from 40bd854 to 04039f6 Compare July 22, 2026 13:50
majamassarini and others added 2 commits July 22, 2026 18:39
When per_commit_flow cherry-picks base-branch commits, their changelog
entries carry the base MR's Jira key. If the base was backported under
a multi-CVE tracker (e.g. RHEL-154707), all CVE entries get that key
instead of per-CVE keys (RHEL-190609, RHEL-190617).

Add _build_cve_to_jira_map() to derive CVE→Jira mappings from MR
titles and branch names, and _fix_changelog_resolves() to correct
mismatched Resolves lines in the spec after cherry-picking base
commits. Also fix run_log_agent (merged strategy) to pass all
collected Jira keys instead of only the first one alphabetically.

Observed in mingw-glib2 MR !37 — uril flagged two changelog
mismatches (CVE-2026-58014 → RHEL-154707, CVE-2026-58016 → RHEL-154707).

Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@TomasKorbar
TomasKorbar force-pushed the fix-consolidation-changelog-resolves branch from 04039f6 to 37ecff5 Compare July 22, 2026 16:39
@TomasKorbar
TomasKorbar merged commit d80e450 into packit:main Jul 22, 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