Fix Z-stream fix approach logic bugs

[majamassarini]

Summary

Fixes Low/Moderate Y-stream postponement that ignores shipped Z-stream clones.

Problem

_check_zstream_fix_approach() only checked the "Fixed in Build" custom field to decide whether a Z-stream clone was ready. A clone that had already reached Closed/Done-Errata (shipped) but had an empty "Fixed in Build" field was incorrectly treated as still pending, causing the Y-stream issue to be postponed indefinitely.

Solution

The function now requests status and resolution from Jira and checks them before falling back to the "Fixed in Build" field:

• Closed + Done-Errata/Done with NVR → existing Koji lookup (CS vs RHEL)
• Closed + Done-Errata/Done without NVR → RHEL_FIRST (proceed)
• Closed + rejected resolution (WONTFIX, etc.) → skip the clone

Additional improvements (latest commit)

Also fixed 6 related bugs discovered during code review:

1. Case-sensitive resolution comparison (now uses .upper() consistently)
2. Closed issues with non-shipping resolutions (e.g., "Cannot Reproduce") are now skipped
3. Shipped clone wins over pending (RHEL_FIRST takes precedence)
4. Added guards to prevent overwriting the first RHEL_FIRST diagnostic context
5. Extracted _build_rhel_first_result() helper to eliminate code duplication
6. Simplified resolution extraction by removing intermediate variable

Test plan

• Added comprehensive unit tests covering all resolution scenarios
• Existing unit tests pass
• Pre-commit hooks pass

🤖 Generated with Claude Code

[qodo-for-packit]

PR Summary by Qodo

Fix Z-stream fix approach misclassification for shipped and rejected clones

🐞 Bug fix 🧪 Tests 🕐 40+ Minutes

AI Description

• Fetch Jira status/resolution to avoid treating shipped Z-stream clones as pending.
• Scan all relevant clones to prioritize CS_FIRST while keeping a safe RHEL_FIRST fallback.
• Add unit tests for shipped/no-NVR, rejected, pending, and mixed-clone scenarios.

Diagram

graph TD
  A["CVE eligibility flow"] --> B["_check_zstream_fix_approach"] --> C{{"Jira search"}} --> D["Evaluate clones"] --> E{{"CS Koji lookup"}} --> F["FixApproach result"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Explicit priority scoring across clones

• ➕ Makes precedence rules (CS_FIRST > RHEL_FIRST > PENDING) explicit and easier to audit
• ➕ Can reduce control-flow complexity (fewer continues/early returns)
• ➖ Requires refactoring to a scoring/aggregation model and may be less straightforward than current imperative flow

2. Split per-clone evaluation into a pure helper returning a tri-state

• ➕ Simplifies unit testing of edge cases without heavy mocking of Jira/Koji calls
• ➕ Encapsulates status/resolution/NVR handling cleanly
• ➖ Adds additional abstraction; may be unnecessary given current scope and adequate tests

Recommendation: The PR’s approach (scan all clones, return CS_FIRST immediately, otherwise preserve the first RHEL_FIRST evidence and only return PENDING if no shipped/RHEL evidence exists) is a good fit for the domain and fixes the main misclassification risk. If this logic grows further, consider moving to an explicit priority/aggregation model to make precedence rules more self-documenting.

Files changed (2) +285 / -12

Bug fix (1) +55 / -12

jira.pyUse Jira status/resolution and RHEL_FIRST fallback when determining fix approach +55/-12

Use Jira status/resolution and RHEL_FIRST fallback when determining fix approach

• Extends the Jira search to request status and resolution fields and uses them to avoid misclassifying shipped clones as pending. Introduces an RHEL_FIRST fallback that is preserved while continuing to scan remaining clones for a CS_FIRST Koji match, and skips closed clones with rejected or non-shipping resolutions.

ymir/tools/privileged/jira.py

Tests (1) +230 / -0

test_jira.pyAdd unit tests for Z-stream fix approach classification +230/-0

Add unit tests for Z-stream fix approach classification

• Adds a dedicated test suite covering shipped clones with/without Fixed-in-Build NVRs, rejected resolutions, pending clones, mixed shipped+pending scenarios, and CS_FIRST precedence when later clones have a CS Koji build.

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

[qodo-for-packit]

Code Review by Qodo

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

Context used

✅ Compliance rules (platform): 7 rules

1. Status None crashes check ✓ Resolved 🐞 Bug ☼ Reliability

Description

In _check_zstream_fix_approach, status is read via chained .get() calls that will raise
AttributeError when SearchJiraIssuesTool returns status=None for a result. This can fail the
fix-approach check and cause the low/moderate triage path to incorrectly fall into the surrounding
exception handling instead of producing a decision.

Code

ymir/tools/privileged/jira.py[R629-632]

+        status_name = issue.get("fields", {}).get("status", {}).get("name", "")
+        resolution_raw = issue.get("fields", {}).get("resolution", {})
+        resolution_name = resolution_raw.get("name", "") if resolution_raw else ""
+

Relevance

●●● Strong

Team previously fixed Jira AttributeError by guarding non-dict/None field shapes (PR #528); similar
defensive access accepted.

PR-#528
PR-#642

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The Jira search tool can populate requested fields with None, and the new code assumes status is
always a dict. The module already demonstrates the correct defensive access pattern elsewhere.

ymir/tools/privileged/jira.py[622-635]
ymir/tools/privileged/jira.py[1430-1435]
ymir/tools/privileged/jira.py[547-555]

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_fix_approach()` accesses `issue.fields.status.name` using chained `.get()` calls. If `status` is present but `None`, this raises `AttributeError`.

### Issue Context
`SearchJiraIssuesTool` builds `fields` by copying requested keys from Jira’s response; if a requested field is missing, the value will be `None`. Other code in this module already uses the defensive `(… or {})` pattern for status access.

### Fix Focus Areas
- ymir/tools/privileged/jira.py[629-632]
- ymir/tools/privileged/jira.py[1430-1435]
- ymir/tools/privileged/tests/unit/test_jira.py[781-1009]

### Proposed fix
- Change:
 - `status_name = issue.get("fields", {}).get("status", {}).get("name", "")`
- To:
 - `status_name = (issue.get("fields", {}).get("status") or {}).get("name", "")`

### Test addition (recommended)
Add a unit test where a search result includes `"status": None` to ensure `_check_zstream_fix_approach` does not crash and returns a sensible result (likely `PENDING` when `Fixed in Build` is missing).

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

---

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

[majamassarini]

/agentic_review

[qodo-for-packit]

Code review by qodo was updated up to the latest commit 8e1ec74

[nforro]

3. PENDING now takes precedence over RHEL_FIRST when pending clones exist

Does it? The code literally says shipped clone wins over pending.

[nforro]

Apart from the comments, LGTM.