From 07799a007576b23f8b28156f9ca38aa21a8553ac Mon Sep 17 00:00:00 2001 From: Maja Massarini Date: Wed, 26 Aug 2026 16:18:11 +0200 Subject: [PATCH 1/7] Fix: Include all terminal labels when checking sibling completion status Previously, when checking if all siblings had finished triaging in check_and_queue_primary_if_ready(), we only excluded issues with "triaged" labels (ymir_triaged_*), but not those with completion labels (ymir_backported, ymir_rebased, ymir_rebuilt) or error labels (ymir_backport_errored, ymir_rebase_errored, etc.). This caused primary issues to remain stuck waiting for siblings that had already completed their work, as seen in RHEL-248139 where siblings finished with ymir_backported and ymir_backport_errored labels. The fix adds all terminal state labels to both: 1. check_and_queue_primary_if_ready() - so primary can proceed when all siblings are done 2. queue_siblings_for_triage() - for consistency and to avoid re-queueing completed siblings Terminal states now include: - Triage decisions: ymir_triaged_{rebase,backport,rebuild,not_affected,postponed} - Completions: ymir_{backported,rebased,rebuilt} - Errors: ymir_{backport,rebase,rebuild}_errored - Failures: ymir_{backport,rebase,rebuild}_failed Also adds test documentation for expected terminal label behavior. Fixes: https://redhat.atlassian.net/browse/RHEL-248139 Assisted-by: Claude Sonnet 4.5 --- ymir/agents/rebase_consolidation.py | 21 ++++++++ .../tests/unit/test_rebase_consolidation.py | 48 +++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/ymir/agents/rebase_consolidation.py b/ymir/agents/rebase_consolidation.py index 344538448..da77b614a 100644 --- a/ymir/agents/rebase_consolidation.py +++ b/ymir/agents/rebase_consolidation.py @@ -202,9 +202,18 @@ async def queue_siblings_for_triage( JiraLabels.TRIAGED_REBUILD.value, JiraLabels.TRIAGED_NOT_AFFECTED.value, JiraLabels.TRIAGED_POSTPONED.value, + # Completion labels JiraLabels.BACKPORTED.value, JiraLabels.REBASED.value, JiraLabels.REBUILT.value, + # Error labels + JiraLabels.BACKPORT_ERRORED.value, + JiraLabels.REBASE_ERRORED.value, + JiraLabels.REBUILD_ERRORED.value, + # Failed labels + JiraLabels.BACKPORT_FAILED.value, + JiraLabels.REBASE_FAILED.value, + JiraLabels.REBUILD_FAILED.value, ] if any(label in candidate_labels for label in terminal_labels): found_labels = [label for label in terminal_labels if label in candidate_labels] @@ -411,6 +420,18 @@ async def check_and_queue_primary_if_ready( JiraLabels.TRIAGED_REBUILD.value, JiraLabels.TRIAGED_NOT_AFFECTED.value, JiraLabels.TRIAGED_POSTPONED.value, + # Completion labels - work finished successfully + JiraLabels.BACKPORTED.value, + JiraLabels.REBASED.value, + JiraLabels.REBUILT.value, + # Error labels - work finished with error + JiraLabels.BACKPORT_ERRORED.value, + JiraLabels.REBASE_ERRORED.value, + JiraLabels.REBUILD_ERRORED.value, + # Failed labels - work finished with failure + JiraLabels.BACKPORT_FAILED.value, + JiraLabels.REBASE_FAILED.value, + JiraLabels.REBUILD_FAILED.value, ] excluded = ", ".join(f'"{label}"' for label in terminal_labels) jql_pending = ( diff --git a/ymir/agents/tests/unit/test_rebase_consolidation.py b/ymir/agents/tests/unit/test_rebase_consolidation.py index 6c390ba87..d92377cc1 100644 --- a/ymir/agents/tests/unit/test_rebase_consolidation.py +++ b/ymir/agents/tests/unit/test_rebase_consolidation.py @@ -180,3 +180,51 @@ def test_extract_multiple_inline_cards(self): assert "RHEL-200" in result assert "See" in result assert "and" in result + + +class TestTerminalLabels: + """Tests for terminal label handling in sibling consolidation.""" + + def test_all_terminal_states_are_recognized(self): + """ + Verify that all terminal states (triaged, completed, errored, failed) + are properly recognized to prevent stuck primary issues. + + Regression test for: https://redhat.atlassian.net/browse/RHEL-248139 + where siblings with ymir_backported or ymir_backport_errored were not + recognized as terminal, causing the primary to wait indefinitely. + """ + from ymir.common.constants import JiraLabels + + # These are all the labels that indicate a sibling has finished + # processing and should not block the primary from proceeding + expected_terminal_labels = { + # Triage decisions + JiraLabels.TRIAGED_REBASE.value, + JiraLabels.TRIAGED_BACKPORT.value, + JiraLabels.TRIAGED_REBUILD.value, + JiraLabels.TRIAGED_NOT_AFFECTED.value, + JiraLabels.TRIAGED_POSTPONED.value, + # Successful completions + JiraLabels.BACKPORTED.value, + JiraLabels.REBASED.value, + JiraLabels.REBUILT.value, + # Errors (transient failures that may be retried) + JiraLabels.BACKPORT_ERRORED.value, + JiraLabels.REBASE_ERRORED.value, + JiraLabels.REBUILD_ERRORED.value, + # Failures (permanent failures) + JiraLabels.BACKPORT_FAILED.value, + JiraLabels.REBASE_FAILED.value, + JiraLabels.REBUILD_FAILED.value, + } + + # Note: We cannot directly access the terminal_labels lists from + # check_and_queue_primary_if_ready or queue_siblings_for_triage + # since they are defined inline. This test documents the expected + # behavior and will fail if the constants change but the functions + # are not updated accordingly. + + # Verify all expected labels exist in JiraLabels enum + for label in expected_terminal_labels: + assert label in JiraLabels.all_labels(), f"Expected terminal label {label} not in JiraLabels enum" From 4bb7a80e72e82ca878cf83a6f9f1ab9141615e15 Mon Sep 17 00:00:00 2001 From: Maja Massarini Date: Thu, 27 Aug 2026 08:53:15 +0200 Subject: [PATCH 2/7] Also treat TRIAGE_ERRORED as terminal to prevent blocking primaries After triage retry exhaustion, issues get TRIAGE_ERRORED label which is treated as terminal by triage deduplication. However, sibling discovery did not exclude it, causing siblings that exhausted triage retries to be counted as "pending", leaving primaries permanently blocked. Add TRIAGE_ERRORED to terminal_labels in both: - queue_siblings_for_triage() - avoid re-queueing exhausted siblings - check_and_queue_primary_if_ready() - unblock primary when siblings exhaust retries Assisted-by: Claude Sonnet 4.5 --- ymir/agents/rebase_consolidation.py | 2 ++ ymir/agents/tests/unit/test_rebase_consolidation.py | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/ymir/agents/rebase_consolidation.py b/ymir/agents/rebase_consolidation.py index da77b614a..604ab464a 100644 --- a/ymir/agents/rebase_consolidation.py +++ b/ymir/agents/rebase_consolidation.py @@ -207,6 +207,7 @@ async def queue_siblings_for_triage( JiraLabels.REBASED.value, JiraLabels.REBUILT.value, # Error labels + JiraLabels.TRIAGE_ERRORED.value, JiraLabels.BACKPORT_ERRORED.value, JiraLabels.REBASE_ERRORED.value, JiraLabels.REBUILD_ERRORED.value, @@ -425,6 +426,7 @@ async def check_and_queue_primary_if_ready( JiraLabels.REBASED.value, JiraLabels.REBUILT.value, # Error labels - work finished with error + JiraLabels.TRIAGE_ERRORED.value, JiraLabels.BACKPORT_ERRORED.value, JiraLabels.REBASE_ERRORED.value, JiraLabels.REBUILD_ERRORED.value, diff --git a/ymir/agents/tests/unit/test_rebase_consolidation.py b/ymir/agents/tests/unit/test_rebase_consolidation.py index d92377cc1..05543f098 100644 --- a/ymir/agents/tests/unit/test_rebase_consolidation.py +++ b/ymir/agents/tests/unit/test_rebase_consolidation.py @@ -209,7 +209,8 @@ def test_all_terminal_states_are_recognized(self): JiraLabels.BACKPORTED.value, JiraLabels.REBASED.value, JiraLabels.REBUILT.value, - # Errors (transient failures that may be retried) + # Errors (transient failures that may be retried, or exhausted retries) + JiraLabels.TRIAGE_ERRORED.value, JiraLabels.BACKPORT_ERRORED.value, JiraLabels.REBASE_ERRORED.value, JiraLabels.REBUILD_ERRORED.value, From 48e525a521e6055e0e60e88470d1cfce56af3677 Mon Sep 17 00:00:00 2001 From: Maja Massarini Date: Thu, 27 Aug 2026 09:00:40 +0200 Subject: [PATCH 3/7] Move terminal label exclusions into JQL query to fix 50-result limit issue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, terminal label filtering happened in Python after fetching up to 50 sibling candidates from Jira. This created a critical bug: if there were more than 50 total candidates and many of them had terminal labels, the JQL would return 50 results that included already-processed siblings, causing us to miss actual pending siblings and potentially fail to queue them for triage. For example, if there were 60 sibling candidates where 40 had terminal labels and 20 were still pending: - Old behavior: JQL returns first 50 (say 35 terminal + 15 pending), then Python filters to 15 pending → we missed 5 real pending siblings - New behavior: JQL excludes the 40 terminal ones and returns the 20 pending The fix moves ALL terminal label exclusions from post-query Python filtering into the JQL query itself via build_rebase_siblings_jql(): Terminal labels now excluded in JQL: - Triage decisions: ymir_triaged_{rebase,backport,rebuild,not_affected,postponed} - Completions: ymir_{backported,rebased,rebuilt} - Errors: ymir_{triage,backport,rebase,rebuild}_errored - Failures: ymir_{backport,rebase,rebuild}_failed - Sibling marker: ymir_rebase_sibling Kept defensive post-query check for race conditions (Jira indexing delays). Updated test to verify all terminal labels appear in generated JQL. Assisted-by: Claude Sonnet 4.5 --- ymir/agents/rebase_consolidation.py | 38 +++++++++++++------ .../tests/unit/test_rebase_consolidation.py | 29 ++++++++++++-- 2 files changed, 53 insertions(+), 14 deletions(-) diff --git a/ymir/agents/rebase_consolidation.py b/ymir/agents/rebase_consolidation.py index 604ab464a..04e4b3e83 100644 --- a/ymir/agents/rebase_consolidation.py +++ b/ymir/agents/rebase_consolidation.py @@ -78,17 +78,36 @@ def build_rebase_siblings_jql( issue_key: Primary issue to exclude component: Package component fix_version: Target fix version - exclude_triaged: If True, exclude already-triaged issues (for queueing new siblings). + exclude_triaged: If True, exclude all terminal states (for queueing new siblings). If False, include all siblings (for consolidating in rebase MR). """ excluded = [] if exclude_triaged: + # Exclude ALL terminal states to ensure JQL filtering happens before the 50-result limit. + # Post-query filtering is not equivalent because we might miss real pending siblings + # if there are >50 total candidates including many already-processed ones. excluded = [ + # Triage decisions JiraLabels.TRIAGED_NOT_AFFECTED.value, JiraLabels.TRIAGED_BACKPORT.value, JiraLabels.TRIAGED_REBUILD.value, JiraLabels.TRIAGED_REBASE.value, JiraLabels.TRIAGED_POSTPONED.value, + # Completion labels + JiraLabels.BACKPORTED.value, + JiraLabels.REBASED.value, + JiraLabels.REBUILT.value, + # Error labels + JiraLabels.TRIAGE_ERRORED.value, + JiraLabels.BACKPORT_ERRORED.value, + JiraLabels.REBASE_ERRORED.value, + JiraLabels.REBUILD_ERRORED.value, + # Failed labels + JiraLabels.BACKPORT_FAILED.value, + JiraLabels.REBASE_FAILED.value, + JiraLabels.REBUILD_FAILED.value, + # Sibling marker (already queued as sibling for a different primary) + JiraLabels.REBASE_SIBLING.value, ] return build_siblings_jql( issue_key=issue_key, @@ -192,35 +211,32 @@ async def queue_siblings_for_triage( logger.info(f"Sibling {candidate_key} not eligible: {eligibility_result.reason}") continue - # Check if already queued as sibling or already triaged (any resolution) - # Skip if already processed to avoid re-triaging completed issues + # Defensive check for terminal labels (should already be filtered by JQL, + # but check again in case of Jira indexing delays or race conditions) candidate_labels, _ = await tasks.get_jira_issue_metadata(candidate_key) - terminal_labels = [ + terminal_labels = { JiraLabels.REBASE_SIBLING.value, JiraLabels.TRIAGED_REBASE.value, JiraLabels.TRIAGED_BACKPORT.value, JiraLabels.TRIAGED_REBUILD.value, JiraLabels.TRIAGED_NOT_AFFECTED.value, JiraLabels.TRIAGED_POSTPONED.value, - # Completion labels JiraLabels.BACKPORTED.value, JiraLabels.REBASED.value, JiraLabels.REBUILT.value, - # Error labels JiraLabels.TRIAGE_ERRORED.value, JiraLabels.BACKPORT_ERRORED.value, JiraLabels.REBASE_ERRORED.value, JiraLabels.REBUILD_ERRORED.value, - # Failed labels JiraLabels.BACKPORT_FAILED.value, JiraLabels.REBASE_FAILED.value, JiraLabels.REBUILD_FAILED.value, - ] - if any(label in candidate_labels for label in terminal_labels): - found_labels = [label for label in terminal_labels if label in candidate_labels] + } + found_terminal = terminal_labels.intersection(candidate_labels) + if found_terminal: logger.info( f"Sibling {candidate_key} already processed " - f"(has terminal label: {found_labels}), skipping" + f"(has terminal label: {found_terminal}), skipping" ) continue diff --git a/ymir/agents/tests/unit/test_rebase_consolidation.py b/ymir/agents/tests/unit/test_rebase_consolidation.py index 05543f098..f67ca6d87 100644 --- a/ymir/agents/tests/unit/test_rebase_consolidation.py +++ b/ymir/agents/tests/unit/test_rebase_consolidation.py @@ -24,15 +24,38 @@ def test_build_rebase_siblings_jql_escapes_component_quotes(): def test_build_rebase_siblings_jql_excludes_correct_labels(): - """Verify that rebase consolidation excludes terminal triage labels to prevent circular consolidation.""" + """Verify that rebase consolidation excludes ALL terminal labels in JQL. + + This prevents missing pending siblings when there are >50 total candidates. + """ jql = build_rebase_siblings_jql("RHEL-100", "python3.12", "rhel-9.8") - # Should exclude issues already triaged (prevents circular consolidation) + + # Triage decisions assert '"ymir_triaged_not_affected"' in jql assert '"ymir_triaged_backport"' in jql assert '"ymir_triaged_rebuild"' in jql - assert '"ymir_triaged_rebase"' in jql # Prevents circular consolidation + assert '"ymir_triaged_rebase"' in jql assert '"ymir_triaged_postponed"' in jql + # Completion labels (must be in JQL, not just post-query filtering) + assert '"ymir_backported"' in jql + assert '"ymir_rebased"' in jql + assert '"ymir_rebuilt"' in jql + + # Error labels (must be in JQL to avoid missing pending siblings) + assert '"ymir_triage_errored"' in jql + assert '"ymir_backport_errored"' in jql + assert '"ymir_rebase_errored"' in jql + assert '"ymir_rebuild_errored"' in jql + + # Failed labels + assert '"ymir_backport_failed"' in jql + assert '"ymir_rebase_failed"' in jql + assert '"ymir_rebuild_failed"' in jql + + # Sibling marker + assert '"ymir_rebase_sibling"' in jql + class TestSiblingCommentExtraction: """Tests for extracting and matching sibling references from Jira comments.""" From ae6a3c7a108047716f1587455ce6305867218d80 Mon Sep 17 00:00:00 2001 From: Maja Massarini Date: Thu, 27 Aug 2026 09:25:05 +0200 Subject: [PATCH 4/7] Replace enum assertion with behavioral tests for terminal label exclusions Previous test checked if labels existed in JiraLabels.all_labels(), which is circular - it doesn't verify the actual production code in build_rebase_siblings_jql() or the inline terminal_labels lists in queue_siblings_for_triage() and check_and_queue_primary_if_ready(). New behavioral tests verify: 1. JQL query actually excludes each category of terminal labels: - Triage decisions (triaged_*) - Completion labels (backported, rebased, rebuilt) - Error labels (triage_errored, backport_errored, etc.) - Failed labels (backport_failed, etc.) - Sibling marker (rebase_sibling) 2. Exclusions are in the JQL query itself (server-side), not post-query filtering, which is critical to avoid missing pending siblings when there are >50 total candidates Each test method verifies a specific category and will fail if someone removes a terminal label from the production code, preventing regressions like RHEL-248139. Assisted-by: Claude Sonnet 4.5 --- .../tests/unit/test_rebase_consolidation.py | 111 ++++++++++++++---- 1 file changed, 86 insertions(+), 25 deletions(-) diff --git a/ymir/agents/tests/unit/test_rebase_consolidation.py b/ymir/agents/tests/unit/test_rebase_consolidation.py index f67ca6d87..edc4b576b 100644 --- a/ymir/agents/tests/unit/test_rebase_consolidation.py +++ b/ymir/agents/tests/unit/test_rebase_consolidation.py @@ -206,49 +206,110 @@ def test_extract_multiple_inline_cards(self): class TestTerminalLabels: - """Tests for terminal label handling in sibling consolidation.""" + """Behavioral tests for terminal label handling in sibling consolidation. - def test_all_terminal_states_are_recognized(self): - """ - Verify that all terminal states (triaged, completed, errored, failed) - are properly recognized to prevent stuck primary issues. + These tests verify that the production code actually excludes all terminal states, + preventing bugs like RHEL-248139 where primaries got stuck waiting for siblings + that had already finished with ymir_backported or ymir_backport_errored. + """ - Regression test for: https://redhat.atlassian.net/browse/RHEL-248139 - where siblings with ymir_backported or ymir_backport_errored were not - recognized as terminal, causing the primary to wait indefinitely. - """ + def test_jql_excludes_all_triage_decision_labels(self): + """JQL must exclude all triage decision labels to avoid re-queueing decided siblings.""" from ymir.common.constants import JiraLabels - # These are all the labels that indicate a sibling has finished - # processing and should not block the primary from proceeding - expected_terminal_labels = { - # Triage decisions + jql = build_rebase_siblings_jql("RHEL-100", "postgresql", "rhel-9.8") + + # Verify each triage decision label appears in the JQL exclusion + for label in [ JiraLabels.TRIAGED_REBASE.value, JiraLabels.TRIAGED_BACKPORT.value, JiraLabels.TRIAGED_REBUILD.value, JiraLabels.TRIAGED_NOT_AFFECTED.value, JiraLabels.TRIAGED_POSTPONED.value, - # Successful completions + ]: + assert f'"{label}"' in jql, f"JQL must exclude {label} but it's missing from: {jql}" + + def test_jql_excludes_all_completion_labels(self): + """JQL must exclude completion labels or primaries wait forever for completed siblings. + + Regression test for RHEL-248139 where ymir_backported was not excluded. + """ + from ymir.common.constants import JiraLabels + + jql = build_rebase_siblings_jql("RHEL-100", "postgresql", "rhel-9.8") + + # These were the missing labels that caused RHEL-248139 + for label in [ JiraLabels.BACKPORTED.value, JiraLabels.REBASED.value, JiraLabels.REBUILT.value, - # Errors (transient failures that may be retried, or exhausted retries) + ]: + assert f'"{label}"' in jql, f"JQL must exclude {label} but it's missing from: {jql}" + + def test_jql_excludes_all_error_labels(self): + """JQL must exclude error labels or primaries wait forever for errored siblings. + + Regression test for RHEL-248139 where ymir_backport_errored was not excluded. + """ + from ymir.common.constants import JiraLabels + + jql = build_rebase_siblings_jql("RHEL-100", "postgresql", "rhel-9.8") + + # These were the missing labels that caused RHEL-248139 + for label in [ JiraLabels.TRIAGE_ERRORED.value, JiraLabels.BACKPORT_ERRORED.value, JiraLabels.REBASE_ERRORED.value, JiraLabels.REBUILD_ERRORED.value, - # Failures (permanent failures) + ]: + assert f'"{label}"' in jql, f"JQL must exclude {label} but it's missing from: {jql}" + + def test_jql_excludes_all_failed_labels(self): + """JQL must exclude failed labels or primaries wait forever for failed siblings.""" + from ymir.common.constants import JiraLabels + + jql = build_rebase_siblings_jql("RHEL-100", "postgresql", "rhel-9.8") + + for label in [ JiraLabels.BACKPORT_FAILED.value, JiraLabels.REBASE_FAILED.value, JiraLabels.REBUILD_FAILED.value, - } + ]: + assert f'"{label}"' in jql, f"JQL must exclude {label} but it's missing from: {jql}" + + def test_jql_excludes_sibling_marker(self): + """JQL must exclude ymir_rebase_sibling to avoid queueing siblings of other primaries.""" + from ymir.common.constants import JiraLabels - # Note: We cannot directly access the terminal_labels lists from - # check_and_queue_primary_if_ready or queue_siblings_for_triage - # since they are defined inline. This test documents the expected - # behavior and will fail if the constants change but the functions - # are not updated accordingly. + jql = build_rebase_siblings_jql("RHEL-100", "postgresql", "rhel-9.8") + + assert f'"{JiraLabels.REBASE_SIBLING.value}"' in jql, ( + f"JQL must exclude {JiraLabels.REBASE_SIBLING.value}" + ) + + def test_jql_exclusion_applies_before_50_result_limit(self): + """Terminal labels must be excluded in JQL, not post-query, to avoid missing pending siblings. + + If there are 60 siblings where 40 have terminal labels and 20 are pending: + - Correct: JQL excludes 40 terminal, returns 20 pending + - Bug: JQL returns first 50 (35 terminal + 15 pending), post-filter → miss 5 pending + + This test verifies the exclusion is in the JQL string (server-side filtering). + """ + from ymir.common.constants import JiraLabels - # Verify all expected labels exist in JiraLabels enum - for label in expected_terminal_labels: - assert label in JiraLabels.all_labels(), f"Expected terminal label {label} not in JiraLabels enum" + jql = build_rebase_siblings_jql("RHEL-100", "postgresql", "rhel-9.8") + + # Critical: the exclusion MUST be in the JQL query string itself + assert "labels not in" in jql, "JQL must have 'labels not in' clause for server-side filtering" + + # Spot-check a few terminal labels to ensure they're in the JQL, not filtered post-query + critical_labels = [ + JiraLabels.BACKPORTED.value, # Caused RHEL-248139 + JiraLabels.BACKPORT_ERRORED.value, # Caused RHEL-248139 + JiraLabels.TRIAGE_ERRORED.value, # Prevents exhausted retries from blocking + ] + for label in critical_labels: + assert f'"{label}"' in jql, ( + f"Critical terminal label {label} must be in JQL for server-side filtering" + ) From 7aeef342458fede619faa48db4907da6ab2a27d3 Mon Sep 17 00:00:00 2001 From: Maja Massarini Date: Thu, 27 Aug 2026 10:38:51 +0200 Subject: [PATCH 5/7] Fix code review findings: Correct ERRORED vs FAILED label handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code review identified critical bugs: 1. **Contradictory JQL in check_and_queue_primary_if_ready**: - Was calling build_rebase_siblings_jql() which excludes ymir_rebase_sibling, then adding AND labels = "ymir_rebase_sibling" → zero results - Fix: Call with exclude_triaged=False, filter pending vs terminal separately 2. **Broke auto-retry for FAILED labels**: - FAILED labels (backport/rebase_failed) are documented as "May auto-retry" - Excluding them from sibling search broke the retry mechanism - Fix: Only exclude ERRORED labels (which block retry), not FAILED Per jira_label_workflow_routing.md: - ERRORED labels (triage/backport/rebase_errored): Block retry, need human → terminal - FAILED labels (backport/rebase_failed): May auto-retry → retriable Changes: - build_rebase_siblings_jql: Exclude ERRORED, include FAILED - queue_siblings_for_triage: Defensive check excludes ERRORED, includes FAILED - check_and_queue_primary_if_ready: Call with exclude_triaged=False, both ERRORED and FAILED are terminal (won't proceed/block primary) - Tests: Verify ERRORED excluded, FAILED included Assisted-by: Claude Sonnet 4.5 --- ymir/agents/rebase_consolidation.py | 55 +++++++++-------- .../tests/unit/test_rebase_consolidation.py | 61 +++++++++++++------ 2 files changed, 73 insertions(+), 43 deletions(-) diff --git a/ymir/agents/rebase_consolidation.py b/ymir/agents/rebase_consolidation.py index 04e4b3e83..9bdd9854d 100644 --- a/ymir/agents/rebase_consolidation.py +++ b/ymir/agents/rebase_consolidation.py @@ -83,30 +83,30 @@ def build_rebase_siblings_jql( """ excluded = [] if exclude_triaged: - # Exclude ALL terminal states to ensure JQL filtering happens before the 50-result limit. + # Exclude non-retriable terminal states to ensure JQL filtering before the 50-result limit. # Post-query filtering is not equivalent because we might miss real pending siblings # if there are >50 total candidates including many already-processed ones. + # + # Per jira_label_workflow_routing.md: + # - ERRORED labels (triage/backport/rebase_errored) block retry → exclude (terminal) + # - FAILED labels (backport/rebase_failed) may auto-retry → DO NOT exclude excluded = [ - # Triage decisions + # Triage decisions (non-retriable - sibling has been triaged and decided) JiraLabels.TRIAGED_NOT_AFFECTED.value, JiraLabels.TRIAGED_BACKPORT.value, JiraLabels.TRIAGED_REBUILD.value, JiraLabels.TRIAGED_REBASE.value, JiraLabels.TRIAGED_POSTPONED.value, - # Completion labels + # Completion labels (non-retriable - work successfully finished) JiraLabels.BACKPORTED.value, JiraLabels.REBASED.value, JiraLabels.REBUILT.value, - # Error labels + # ERRORED labels (block retry, need human attention) JiraLabels.TRIAGE_ERRORED.value, JiraLabels.BACKPORT_ERRORED.value, JiraLabels.REBASE_ERRORED.value, JiraLabels.REBUILD_ERRORED.value, - # Failed labels - JiraLabels.BACKPORT_FAILED.value, - JiraLabels.REBASE_FAILED.value, - JiraLabels.REBUILD_FAILED.value, - # Sibling marker (already queued as sibling for a different primary) + # Sibling marker (already queued as sibling, don't re-queue) JiraLabels.REBASE_SIBLING.value, ] return build_siblings_jql( @@ -211,8 +211,9 @@ async def queue_siblings_for_triage( logger.info(f"Sibling {candidate_key} not eligible: {eligibility_result.reason}") continue - # Defensive check for terminal labels (should already be filtered by JQL, - # but check again in case of Jira indexing delays or race conditions) + # Defensive check for non-retriable terminal labels (should already be filtered by JQL, + # but check again in case of Jira indexing delays or race conditions). + # Per jira_label_workflow_routing.md: FAILED labels are retriable, ERRORED block retry. candidate_labels, _ = await tasks.get_jira_issue_metadata(candidate_key) terminal_labels = { JiraLabels.REBASE_SIBLING.value, @@ -228,15 +229,12 @@ async def queue_siblings_for_triage( JiraLabels.BACKPORT_ERRORED.value, JiraLabels.REBASE_ERRORED.value, JiraLabels.REBUILD_ERRORED.value, - JiraLabels.BACKPORT_FAILED.value, - JiraLabels.REBASE_FAILED.value, - JiraLabels.REBUILD_FAILED.value, } found_terminal = terminal_labels.intersection(candidate_labels) if found_terminal: logger.info( f"Sibling {candidate_key} already processed " - f"(has terminal label: {found_terminal}), skipping" + f"(has non-retriable terminal label: {found_terminal}), skipping" ) continue @@ -418,40 +416,49 @@ async def check_and_queue_primary_if_ready( ) return - # Use the same JQL builder to ensure we only get siblings of THIS primary + # Find siblings that are still pending (not finished processing). + # Use exclude_triaged=False to get ALL siblings, then filter to pending ones. jql = build_rebase_siblings_jql( issue_key=primary_issue, component=component, fix_version=fix_version, + exclude_triaged=False, # Don't exclude anything yet, we'll filter below ) - # Check for siblings that are still processing: either still labeled as sibling - # (not started triage yet) OR in-progress (triage removes ymir_rebase_sibling - # at start, so we need to check both). - # Exclude siblings with terminal triage labels - they're done, even if they - # triaged to a different resolution (e.g. BACKPORT instead of REBASE). + + # A sibling is "pending" (blocks the primary) if it has NOT finished processing. + # Pending states: + # - ymir_rebase_sibling (queued but not started) + # - ymir_triage_in_progress (currently being triaged) + # + # Terminal states (sibling is done, won't block primary): + # - Any ymir_triaged_* label (triage complete, decision made) + # - Any completion label (backported/rebased/rebuilt) + # - Any error/failed label (won't proceed, even if retriable later) sibling_label = JiraLabels.REBASE_SIBLING.value in_progress_label = JiraLabels.TRIAGE_IN_PROGRESS.value terminal_labels = [ + # Triage decisions JiraLabels.TRIAGED_REBASE.value, JiraLabels.TRIAGED_BACKPORT.value, JiraLabels.TRIAGED_REBUILD.value, JiraLabels.TRIAGED_NOT_AFFECTED.value, JiraLabels.TRIAGED_POSTPONED.value, - # Completion labels - work finished successfully + # Completions JiraLabels.BACKPORTED.value, JiraLabels.REBASED.value, JiraLabels.REBUILT.value, - # Error labels - work finished with error + # Errors (won't proceed even if retriable) JiraLabels.TRIAGE_ERRORED.value, JiraLabels.BACKPORT_ERRORED.value, JiraLabels.REBASE_ERRORED.value, JiraLabels.REBUILD_ERRORED.value, - # Failed labels - work finished with failure + # Failures (won't proceed) JiraLabels.BACKPORT_FAILED.value, JiraLabels.REBASE_FAILED.value, JiraLabels.REBUILD_FAILED.value, ] excluded = ", ".join(f'"{label}"' for label in terminal_labels) + # Find siblings that are pending (have sibling/in-progress label) AND not terminal jql_pending = ( f'{jql} AND (labels = "{sibling_label}" OR labels = "{in_progress_label}") ' f"AND labels not in ({excluded})" diff --git a/ymir/agents/tests/unit/test_rebase_consolidation.py b/ymir/agents/tests/unit/test_rebase_consolidation.py index edc4b576b..93b63fe98 100644 --- a/ymir/agents/tests/unit/test_rebase_consolidation.py +++ b/ymir/agents/tests/unit/test_rebase_consolidation.py @@ -24,36 +24,40 @@ def test_build_rebase_siblings_jql_escapes_component_quotes(): def test_build_rebase_siblings_jql_excludes_correct_labels(): - """Verify that rebase consolidation excludes ALL terminal labels in JQL. + """Verify that JQL excludes non-retriable states but includes retriable FAILED labels. + + Per jira_label_workflow_routing.md: + - ERRORED labels (triage/backport/rebase_errored) block retry → exclude + - FAILED labels (backport/rebase_failed) may auto-retry → include (don't exclude) This prevents missing pending siblings when there are >50 total candidates. """ jql = build_rebase_siblings_jql("RHEL-100", "python3.12", "rhel-9.8") - # Triage decisions + # Triage decisions (non-retriable) assert '"ymir_triaged_not_affected"' in jql assert '"ymir_triaged_backport"' in jql assert '"ymir_triaged_rebuild"' in jql assert '"ymir_triaged_rebase"' in jql assert '"ymir_triaged_postponed"' in jql - # Completion labels (must be in JQL, not just post-query filtering) + # Completion labels (non-retriable) assert '"ymir_backported"' in jql assert '"ymir_rebased"' in jql assert '"ymir_rebuilt"' in jql - # Error labels (must be in JQL to avoid missing pending siblings) + # ERRORED labels (block retry, must exclude) assert '"ymir_triage_errored"' in jql assert '"ymir_backport_errored"' in jql assert '"ymir_rebase_errored"' in jql assert '"ymir_rebuild_errored"' in jql - # Failed labels - assert '"ymir_backport_failed"' in jql - assert '"ymir_rebase_failed"' in jql - assert '"ymir_rebuild_failed"' in jql + # FAILED labels (may auto-retry, must NOT exclude) + assert '"ymir_backport_failed"' not in jql + assert '"ymir_rebase_failed"' not in jql + assert '"ymir_rebuild_failed"' not in jql - # Sibling marker + # Sibling marker (don't re-queue) assert '"ymir_rebase_sibling"' in jql @@ -246,36 +250,47 @@ def test_jql_excludes_all_completion_labels(self): ]: assert f'"{label}"' in jql, f"JQL must exclude {label} but it's missing from: {jql}" - def test_jql_excludes_all_error_labels(self): - """JQL must exclude error labels or primaries wait forever for errored siblings. + def test_jql_excludes_errored_labels(self): + """JQL must exclude ERRORED labels which block retry. - Regression test for RHEL-248139 where ymir_backport_errored was not excluded. + Per jira_label_workflow_routing.md: ERRORED labels (triage/backport/rebase_errored) + block retry and need human attention, so they're terminal for sibling queueing. """ from ymir.common.constants import JiraLabels jql = build_rebase_siblings_jql("RHEL-100", "postgresql", "rhel-9.8") - # These were the missing labels that caused RHEL-248139 + # ERRORED labels block retry → must exclude for label in [ JiraLabels.TRIAGE_ERRORED.value, JiraLabels.BACKPORT_ERRORED.value, JiraLabels.REBASE_ERRORED.value, JiraLabels.REBUILD_ERRORED.value, ]: - assert f'"{label}"' in jql, f"JQL must exclude {label} but it's missing from: {jql}" + assert f'"{label}"' in jql, ( + f"JQL must exclude {label} (blocks retry) but it's missing from: {jql}" + ) + + def test_jql_includes_failed_labels(self): + """JQL must NOT exclude FAILED labels which may auto-retry. - def test_jql_excludes_all_failed_labels(self): - """JQL must exclude failed labels or primaries wait forever for failed siblings.""" + Per jira_label_workflow_routing.md: FAILED labels (backport/rebase_failed) + "May auto-retry", so excluding them breaks the retry mechanism where a new + sibling triggers re-queueing of failed issues. + """ from ymir.common.constants import JiraLabels jql = build_rebase_siblings_jql("RHEL-100", "postgresql", "rhel-9.8") + # FAILED labels may auto-retry → must NOT exclude for label in [ JiraLabels.BACKPORT_FAILED.value, JiraLabels.REBASE_FAILED.value, JiraLabels.REBUILD_FAILED.value, ]: - assert f'"{label}"' in jql, f"JQL must exclude {label} but it's missing from: {jql}" + assert f'"{label}"' not in jql, ( + f"JQL must NOT exclude {label} (may auto-retry) but it's excluded in: {jql}" + ) def test_jql_excludes_sibling_marker(self): """JQL must exclude ymir_rebase_sibling to avoid queueing siblings of other primaries.""" @@ -306,10 +321,18 @@ def test_jql_exclusion_applies_before_50_result_limit(self): # Spot-check a few terminal labels to ensure they're in the JQL, not filtered post-query critical_labels = [ JiraLabels.BACKPORTED.value, # Caused RHEL-248139 - JiraLabels.BACKPORT_ERRORED.value, # Caused RHEL-248139 - JiraLabels.TRIAGE_ERRORED.value, # Prevents exhausted retries from blocking + JiraLabels.BACKPORT_ERRORED.value, # ERRORED blocks retry, must exclude + JiraLabels.TRIAGE_ERRORED.value, # ERRORED blocks retry, must exclude ] for label in critical_labels: assert f'"{label}"' in jql, ( f"Critical terminal label {label} must be in JQL for server-side filtering" ) + + # FAILED labels must NOT be in JQL (they're retriable) + retriable_labels = [ + JiraLabels.BACKPORT_FAILED.value, + JiraLabels.REBASE_FAILED.value, + ] + for label in retriable_labels: + assert f'"{label}"' not in jql, f"Retriable label {label} must NOT be excluded in JQL" From 52d5662ce2ed53bbf718648d762a8681a9619600 Mon Sep 17 00:00:00 2001 From: Maja Massarini Date: Thu, 27 Aug 2026 10:49:57 +0200 Subject: [PATCH 6/7] Remove ymir_rebase_sibling from JQL exclusions to fix early primary release The ymir_rebase_sibling label is a queueing marker, not a terminal triage state. Excluding it in build_rebase_siblings_jql() broke check_and_queue_primary_if_ready() because queued-but-not-started siblings were invisible to the readiness check. Before fix: - build_rebase_siblings_jql() excluded ymir_rebase_sibling in JQL - check_and_queue_primary_if_ready() added AND labels = "ymir_rebase_sibling" - Contradictory query (exclude X AND require X) returned zero results - Primary was released early while queued siblings were still pending After fix: - build_rebase_siblings_jql() does NOT exclude ymir_rebase_sibling - Queued siblings are correctly found by the pending query - Primary waits until all siblings (including queued ones) finish - queue_siblings_for_triage() still has defensive check to avoid re-queueing Changes: - Removed ymir_rebase_sibling from excluded list in build_rebase_siblings_jql() - Simplified check_and_queue_primary_if_ready() call (default params work now) - Updated test: ymir_rebase_sibling must NOT be in JQL exclusions - Added regression test proving queued siblings are found as pending Assisted-by: Claude Sonnet 4.5 --- ymir/agents/rebase_consolidation.py | 11 +++-- .../tests/unit/test_rebase_consolidation.py | 46 ++++++++++++++++--- 2 files changed, 47 insertions(+), 10 deletions(-) diff --git a/ymir/agents/rebase_consolidation.py b/ymir/agents/rebase_consolidation.py index 9bdd9854d..206c93f77 100644 --- a/ymir/agents/rebase_consolidation.py +++ b/ymir/agents/rebase_consolidation.py @@ -90,6 +90,11 @@ def build_rebase_siblings_jql( # Per jira_label_workflow_routing.md: # - ERRORED labels (triage/backport/rebase_errored) block retry → exclude (terminal) # - FAILED labels (backport/rebase_failed) may auto-retry → DO NOT exclude + # + # NOTE: Do NOT exclude ymir_rebase_sibling here - it's not a terminal triage state, + # it's a queueing marker. Excluding it here would break check_and_queue_primary_if_ready() + # which needs to find queued-but-not-started siblings to know if primary should wait. + # queue_siblings_for_triage() handles the re-queueing check in its defensive filter. excluded = [ # Triage decisions (non-retriable - sibling has been triaged and decided) JiraLabels.TRIAGED_NOT_AFFECTED.value, @@ -106,8 +111,6 @@ def build_rebase_siblings_jql( JiraLabels.BACKPORT_ERRORED.value, JiraLabels.REBASE_ERRORED.value, JiraLabels.REBUILD_ERRORED.value, - # Sibling marker (already queued as sibling, don't re-queue) - JiraLabels.REBASE_SIBLING.value, ] return build_siblings_jql( issue_key=issue_key, @@ -417,12 +420,12 @@ async def check_and_queue_primary_if_ready( return # Find siblings that are still pending (not finished processing). - # Use exclude_triaged=False to get ALL siblings, then filter to pending ones. + # build_rebase_siblings_jql() excludes terminal triage states but NOT ymir_rebase_sibling, + # so queued-but-not-started siblings will be found (critical for correct readiness check). jql = build_rebase_siblings_jql( issue_key=primary_issue, component=component, fix_version=fix_version, - exclude_triaged=False, # Don't exclude anything yet, we'll filter below ) # A sibling is "pending" (blocks the primary) if it has NOT finished processing. diff --git a/ymir/agents/tests/unit/test_rebase_consolidation.py b/ymir/agents/tests/unit/test_rebase_consolidation.py index 93b63fe98..c45e38872 100644 --- a/ymir/agents/tests/unit/test_rebase_consolidation.py +++ b/ymir/agents/tests/unit/test_rebase_consolidation.py @@ -57,8 +57,10 @@ def test_build_rebase_siblings_jql_excludes_correct_labels(): assert '"ymir_rebase_failed"' not in jql assert '"ymir_rebuild_failed"' not in jql - # Sibling marker (don't re-queue) - assert '"ymir_rebase_sibling"' in jql + # ymir_rebase_sibling must NOT be excluded - it's a queueing state, not a terminal state + # Excluding it would break check_and_queue_primary_if_ready() which needs to find + # queued-but-not-started siblings + assert '"ymir_rebase_sibling"' not in jql class TestSiblingCommentExtraction: @@ -292,14 +294,21 @@ def test_jql_includes_failed_labels(self): f"JQL must NOT exclude {label} (may auto-retry) but it's excluded in: {jql}" ) - def test_jql_excludes_sibling_marker(self): - """JQL must exclude ymir_rebase_sibling to avoid queueing siblings of other primaries.""" + def test_jql_does_not_exclude_sibling_marker(self): + """JQL must NOT exclude ymir_rebase_sibling - it's a queueing state, not terminal. + + Regression test: check_and_queue_primary_if_ready() needs to find queued siblings + that haven't started triage yet (have ymir_rebase_sibling label). If we excluded + this label, the primary would be released early while siblings are still pending. + + queue_siblings_for_triage() handles the re-queueing check in its defensive filter. + """ from ymir.common.constants import JiraLabels jql = build_rebase_siblings_jql("RHEL-100", "postgresql", "rhel-9.8") - assert f'"{JiraLabels.REBASE_SIBLING.value}"' in jql, ( - f"JQL must exclude {JiraLabels.REBASE_SIBLING.value}" + assert f'"{JiraLabels.REBASE_SIBLING.value}"' not in jql, ( + f"JQL must NOT exclude {JiraLabels.REBASE_SIBLING.value} (queueing state, not terminal)" ) def test_jql_exclusion_applies_before_50_result_limit(self): @@ -336,3 +345,28 @@ def test_jql_exclusion_applies_before_50_result_limit(self): ] for label in retriable_labels: assert f'"{label}"' not in jql, f"Retriable label {label} must NOT be excluded in JQL" + + def test_queued_sibling_blocks_primary(self): + """Regression: Queued siblings with ymir_rebase_sibling must be found as pending. + + Before fix: build_rebase_siblings_jql() excluded ymir_rebase_sibling, then + check_and_queue_primary_if_ready() added AND labels = "ymir_rebase_sibling", + resulting in zero matches. Primary was released while queued siblings were pending. + + After fix: ymir_rebase_sibling is NOT excluded in JQL, so the pending query + correctly finds queued-but-not-started siblings. + """ + + # Simulate the pending-sibling query in check_and_queue_primary_if_ready() + jql = build_rebase_siblings_jql("RHEL-100", "postgresql", "rhel-9.8") + + # The query should be able to find siblings with ymir_rebase_sibling + # This is the key fix: if ymir_rebase_sibling were excluded from JQL, + # then check_and_queue_primary_if_ready() adding: + # AND (labels = "ymir_rebase_sibling" OR labels = "ymir_triage_in_progress") + # would return zero results (contradictory query: exclude X AND require X) + + # The key assertion: ymir_rebase_sibling must NOT appear in the exclusion list + assert '"ymir_rebase_sibling"' not in jql, ( + "ymir_rebase_sibling in exclusion list would make pending query contradictory" + ) From 4f69dd704e1bae739556c8ff47b7486e463e0140 Mon Sep 17 00:00:00 2001 From: Maja Massarini Date: Thu, 27 Aug 2026 11:13:23 +0200 Subject: [PATCH 7/7] Add TRIAGED and NEEDS_ATTENTION to terminal label exclusions Per triage_agent.py: - Resolution.OPEN_ENDED_ANALYSIS maps to ymir_triaged (terminal, no automated follow-up) - Resolution.CLARIFICATION_NEEDED maps to ymir_needs_attention (blocked, needs human) These were missing from terminal label exclusions, allowing completed open-ended-analysis and clarification-needed issues to be re-queued as siblings. Changes: - Added JiraLabels.TRIAGED to JQL exclusions in build_rebase_siblings_jql() - Added JiraLabels.NEEDS_ATTENTION to JQL exclusions - Added both to defensive terminal check in queue_siblings_for_triage() - Extended test to verify both labels are excluded in JQL Assisted-by: Claude Sonnet 4.5 --- ymir/agents/rebase_consolidation.py | 4 ++++ ymir/agents/tests/unit/test_rebase_consolidation.py | 2 ++ 2 files changed, 6 insertions(+) diff --git a/ymir/agents/rebase_consolidation.py b/ymir/agents/rebase_consolidation.py index 206c93f77..977b2b8cc 100644 --- a/ymir/agents/rebase_consolidation.py +++ b/ymir/agents/rebase_consolidation.py @@ -102,6 +102,7 @@ def build_rebase_siblings_jql( JiraLabels.TRIAGED_REBUILD.value, JiraLabels.TRIAGED_REBASE.value, JiraLabels.TRIAGED_POSTPONED.value, + JiraLabels.TRIAGED.value, # Open-ended-analysis, no automated follow-up # Completion labels (non-retriable - work successfully finished) JiraLabels.BACKPORTED.value, JiraLabels.REBASED.value, @@ -111,6 +112,7 @@ def build_rebase_siblings_jql( JiraLabels.BACKPORT_ERRORED.value, JiraLabels.REBASE_ERRORED.value, JiraLabels.REBUILD_ERRORED.value, + JiraLabels.NEEDS_ATTENTION.value, # Clarification-needed, blocked ] return build_siblings_jql( issue_key=issue_key, @@ -225,6 +227,7 @@ async def queue_siblings_for_triage( JiraLabels.TRIAGED_REBUILD.value, JiraLabels.TRIAGED_NOT_AFFECTED.value, JiraLabels.TRIAGED_POSTPONED.value, + JiraLabels.TRIAGED.value, JiraLabels.BACKPORTED.value, JiraLabels.REBASED.value, JiraLabels.REBUILT.value, @@ -232,6 +235,7 @@ async def queue_siblings_for_triage( JiraLabels.BACKPORT_ERRORED.value, JiraLabels.REBASE_ERRORED.value, JiraLabels.REBUILD_ERRORED.value, + JiraLabels.NEEDS_ATTENTION.value, } found_terminal = terminal_labels.intersection(candidate_labels) if found_terminal: diff --git a/ymir/agents/tests/unit/test_rebase_consolidation.py b/ymir/agents/tests/unit/test_rebase_consolidation.py index c45e38872..6416085af 100644 --- a/ymir/agents/tests/unit/test_rebase_consolidation.py +++ b/ymir/agents/tests/unit/test_rebase_consolidation.py @@ -232,6 +232,8 @@ def test_jql_excludes_all_triage_decision_labels(self): JiraLabels.TRIAGED_REBUILD.value, JiraLabels.TRIAGED_NOT_AFFECTED.value, JiraLabels.TRIAGED_POSTPONED.value, + JiraLabels.TRIAGED.value, # Open-ended-analysis + JiraLabels.NEEDS_ATTENTION.value, # Clarification-needed ]: assert f'"{label}"' in jql, f"JQL must exclude {label} but it's missing from: {jql}"