Post error comments regardless of trigger

jira_label_workflow_routing.md

@@ -102,7 +102,7 @@ flowchart TD
 |-------|---------|--------|
 | `ymir_retry_needed` | Trigger retry | Forces reprocessing |
 | `ymir_triaged` | Triage completed, no automated follow-up | Terminal state |
-| `ymir_todo` | Maintainer-facing trigger for an e2e run | Fetcher swaps it for `ymir_triage_in_progress` on enqueue; only honored when the changelog shows the label was added by a member of the `Red Hat Employee` Jira group (verified per-issue, not via JQL). The triage run posts an ack comment and a result comment so the requester gets feedback. Default is silent — without `ymir_todo`, no comments are posted. |
+| `ymir_todo` | Maintainer-facing trigger for an e2e run | Fetcher swaps it for `ymir_triage_in_progress` on enqueue; only honored when the changelog shows the label was added by a member of the `Red Hat Employee` Jira group (verified per-issue, not via JQL). The triage run posts an ack comment and a result comment so the requester gets feedback. Automatic runs remain quiet for normal results, but error comments are posted once after the final retry regardless of trigger. |
 | `ymir_consolidate_base` | Mark a backport MR for consolidation (base) | Paired with `ymir_consolidate_next` on another issue for the same package/branch. The fetcher matches the pair, submits a targeted consolidation job, removes both labels, and posts comments. |
 | `ymir_consolidate_next` | Mark a backport MR for consolidation (next) | Must be on a different issue than `ymir_consolidate_base`, for the same package/branch. |
 | `ymir_cli_triage` | Usage-tracking marker for CLI runs | Set at the start of a user-triggered CLI triage run. Prevents the fetcher from picking up the same issue in parallel. Persists on success as a usage-tracking marker; removed on failure so the fetcher can re-process the issue. Covered by the fetcher's stale-label recovery (IN_FLIGHT_LABELS) to handle SIGKILL/OOM. |
@@ -177,7 +177,7 @@ flowchart TD
 
 ## Run Behaviour by Trigger and Flag
 
-Two env-var flags affect pipeline behaviour: `DRY_RUN` and `JIRA_ALLOW_STATUS_CHANGES`. Verbosity is no longer controlled by an env var — the system is silent by default. The only way to opt into comments is per-issue, by adding `ymir_todo` (which flows through the task as `user_triggered=True`).
+Two env-var flags affect pipeline behaviour: `DRY_RUN` and `JIRA_ALLOW_STATUS_CHANGES`. Verbosity is no longer controlled by an env var — the system is quiet by default. Adding `ymir_todo` (which flows through the task as `user_triggered=True`) opts into acknowledgement and normal result comments. Error comments are always posted regardless of trigger: workflow error resolutions are treated as no-MR results (posted immediately), and crash-based errors are posted once after the final retry.
 
 Important/Critical Y-stream CVEs may take the deterministic inheritance fast
 path before the normal backport agent. A restricted LLM adapts only the target
@@ -194,8 +194,8 @@ so clone or queue retries continue only through normal backporting.
 
 Ground rules:
 
-- **Default is silent.** No result or error comments are posted on the Jira issue, and intermediate `_failed` labels are not written. Only `not-affected`, `postponed`, `open-ended-analysis`, and `clarification-needed` triage resolutions still post a comment unbidden (those have no MR to look at, so the comment is the only visible explanation).
-- **`user_triggered=True`** (set on the task when the issue carried `ymir_todo`) **bypasses every silence filter.** The triage agent posts an immediate private ack comment, posts the result comment, and writes `_failed` labels normally.
+- **Default is quiet.** Normal result comments are suppressed on automatic runs, and intermediate `_failed` labels are not written. Only `not-affected`, `postponed`, `open-ended-analysis`, and `clarification-needed` triage resolutions still post a comment unbidden (those have no MR to look at, so the comment is the only visible explanation). Error comments are posted once after the final retry via `post_terminal_error_comment()`, regardless of trigger.
+- **`user_triggered=True`** (set on the task when the issue carried `ymir_todo`) enables an immediate private ack comment, normal result comments, and `_failed` labels. Error comments do not depend on this flag — they are posted once after retries are exhausted.
 - **Labels that are state, not notification, are always written.** `ymir_triage_in_progress` at the start of triage, terminal `ymir_*_errored` / `ymir_triaged_*` at the end. Suppressing them would break dedup against the next fetcher sweep.
 - **Jira workflow status changes are opt-in via `JIRA_ALLOW_STATUS_CHANGES`.** When the env var is unset or `false` (the default), the rebase/backport agents do NOT move the issue to "In Progress" on task pop, and the issue-verification agent does NOT transition issues to "Release Pending" / "Closed". When set to `true`, all of those transitions happen. The same flag also gates the preliminary-testing agent setting **`Preliminary Testing = Pass`** — that field admits the build into the next compose, triggers erratum creation, and moves the issue to Integration. Triage and the fetcher never touch the workflow status, regardless of the flag.
 - **`DRY_RUN` is read by both fetcher and agent.** On the fetcher, `DRY_RUN=true` skips the atomic Jira label flip (`ymir_todo` / `ymir_retry_needed` are NOT consumed; `ymir_triage_in_progress` is NOT stamped) but the task is still pushed to Redis with the correct `user_triggered` value, so the agent — also presumably in `DRY_RUN` — can exercise its full dry-mode flow. Implication: the trigger label stays on the issue, so every subsequent fetcher sweep re-picks the same issue. That is fine in a test environment; never run a production cron with `DRY_RUN=true`. `DRY_RUN=true` also implies status changes are skipped, independent of `JIRA_ALLOW_STATUS_CHANGES`.
@@ -204,9 +204,9 @@ What happens for each trigger state:
 
 | Trigger state at sweep time | Default behaviour | `DRY_RUN=true` |
 |---|---|---|
-| **No `ymir_*` labels** (fresh issue) | Fetcher pushes to `triage_queue`. Agent stamps `ymir_triage_in_progress`, runs triage, writes a terminal `ymir_*` label. Result comment is suppressed unless the resolution is `not-affected`, `postponed`, `open-ended-analysis`, or `clarification-needed`. If the run auto-chains to rebase or backport, the downstream agent moves the Jira workflow status to "In Progress" when it pops the task — **only if `JIRA_ALLOW_STATUS_CHANGES=true`**; otherwise the status is left untouched. | Agent runs triage but `set_jira_labels` / `add_jira_comment` short-circuit on `DRY_RUN`. No labels, no comment, no MR, no workflow status change. Issue untouched in Jira. |
+| **No `ymir_*` labels** (fresh issue) | Fetcher pushes to `triage_queue`. Agent stamps `ymir_triage_in_progress`, runs triage, writes a terminal `ymir_*` label. Result comment is suppressed unless the resolution is `not-affected`, `postponed`, `open-ended-analysis`, or `clarification-needed`; error comments are posted once after the final retry. If the run auto-chains to rebase or backport, the downstream agent moves the Jira workflow status to "In Progress" when it pops the task — **only if `JIRA_ALLOW_STATUS_CHANGES=true`**; otherwise the status is left untouched. | Agent runs triage but `set_jira_labels` / `add_jira_comment` short-circuit on `DRY_RUN`. No labels, no comment, no MR, no workflow status change. Issue untouched in Jira. |
 | **`ymir_todo`** (added by a member of `Red Hat Employee`, no `_in_progress`) | Fetcher verifies the latest `ymir_todo` add in the issue's changelog was performed by a Red Hat Employee; if so, atomically flips `ymir_todo` → `ymir_triage_in_progress` and pushes with `user_triggered=True`. Agent posts a private ack comment and a result comment on completion. `_failed` labels are written normally. Workflow status change is the same as the fresh-issue path (set by rebase/backport on auto-chain, gated on `JIRA_ALLOW_STATUS_CHANGES`). | Fetcher still verifies the author and skips the atomic flip (`ymir_todo` stays on the issue), but still pushes to Redis with `user_triggered=True`. Agent runs in dry mode and writes nothing; workflow status not changed. **Subsequent fetcher sweeps will re-push the same issue** because the trigger label was never consumed. |
-| **`ymir_retry_needed`** (no `_in_progress`) | Fetcher atomically flips `ymir_retry_needed` → `ymir_triage_in_progress`, pushes with `user_triggered=False`. Agent runs full triage; behaves exactly like a fresh-issue run (no ack comment, result comment only for the four "no-MR" resolutions). Workflow status change is the same as the fresh-issue path (gated on `JIRA_ALLOW_STATUS_CHANGES`). | Fetcher skips the atomic flip (`ymir_retry_needed` stays on the issue) but still pushes to Redis with `user_triggered=False`. Agent runs in dry mode and writes nothing; workflow status not changed. Subsequent fetcher sweeps will re-push the same issue. |
+| **`ymir_retry_needed`** (no `_in_progress`) | Fetcher atomically flips `ymir_retry_needed` → `ymir_triage_in_progress`, pushes with `user_triggered=False`. Agent runs full triage; behaves exactly like a fresh-issue run (no ack comment, result comment only for the four no-MR resolutions; error comments posted once after the final retry). Workflow status change is the same as the fresh-issue path (gated on `JIRA_ALLOW_STATUS_CHANGES`). | Fetcher skips the atomic flip (`ymir_retry_needed` stays on the issue) but still pushes to Redis with `user_triggered=False`. Agent runs in dry mode and writes nothing; workflow status not changed. Subsequent fetcher sweeps will re-push the same issue. |
 | **`ymir_todo`** or **`ymir_retry_needed`** **+** any `ymir_*_in_progress` label | Fetcher skips. Not enqueued. Workflow status not affected. | Fetcher skips. Not enqueued. Workflow status not affected. |
 | **Any other terminal `ymir_*` label** (e.g. `ymir_triaged_rebase`, `ymir_rebased`, `ymir_triage_errored`) | Fetcher skips. Re-run by adding `ymir_todo` (recommended — produces an ack + result comment) or `ymir_retry_needed`. Workflow status not affected. | Fetcher skips. Workflow status not affected. |
 

ymir/agents/backport_agent.py

@@ -2040,19 +2040,17 @@ async def finalize_failure(error: ErrorData, retry_queue: str, task, comment_tex
                     user_triggered=user_triggered,
                 )
                 # Crash paths have not reached the workflow's Jira-comment step.
-                if user_triggered and comment_text and not dry_run:
+                if comment_text and not dry_run:
                     try:
                         async with mcp_tools(
                             os.environ["MCP_GATEWAY_URL"],
                             call_meta={"jira_issue": backport_data.jira_issue},
                         ) as gateway_tools:
-                            await tasks.comment_in_jira(
+                            await tasks.post_terminal_error_comment(
                                 jira_issue=backport_data.jira_issue,
                                 agent_type="Backport",
                                 comment_text=comment_text,
                                 available_tools=gateway_tools,
-                                is_error=True,
-                                user_triggered=user_triggered,
                             )
                     except Exception as comment_error:
                         logger.warning(

ymir/agents/rebase_agent.py

@@ -244,7 +244,7 @@ async def post_failure_comments_to_consolidated_siblings(
         Post link comments to consolidated siblings pointing to primary issue with error details.
 
         Uses is_error=False so these informational links are always posted, even on automatic runs.
-        Only the detailed error on the primary issue is gated by user_triggered.
+        The primary terminal error is posted separately by the final retry path.
 
         Isolates errors per-sibling so a single Jira failure doesn't abort posting to other siblings.
         Deduplicates issue keys to prevent multiple identical comments.
@@ -790,32 +790,26 @@ async def retry(
                         user_triggered=user_triggered,
                     )
                     # Post failure feedback to Jira once, here on the final attempt
-                    # only — never for intermediate retries. Restricted to
-                    # user-triggered (ymir_todo) runs: a maintainer who didn't ask
-                    # for processing shouldn't be notified, so skip the gateway
-                    # connection entirely otherwise.
-                    if user_triggered and not dry_run:
+                    # only — never for intermediate retries.
+                    if comment_text and not dry_run:
                         try:
                             async with mcp_tools(
                                 os.environ["MCP_GATEWAY_URL"],
                                 call_meta={"jira_issue": rebase_data.jira_issue},
                             ) as gateway_tools:
                                 # Post detailed error to primary issue (with error handling)
-                                if comment_text:
-                                    try:
-                                        await tasks.comment_in_jira(
-                                            jira_issue=rebase_data.jira_issue,
-                                            agent_type="Rebase",
-                                            comment_text=comment_text,
-                                            available_tools=gateway_tools,
-                                            is_error=True,
-                                            user_triggered=user_triggered,
-                                        )
-                                    except Exception as e:
-                                        logger.warning(
-                                            f"Failed to post error comment to primary issue "
-                                            f"{rebase_data.jira_issue}: {e}"
-                                        )
+                                try:
+                                    await tasks.post_terminal_error_comment(
+                                        jira_issue=rebase_data.jira_issue,
+                                        agent_type="Rebase",
+                                        comment_text=comment_text,
+                                        available_tools=gateway_tools,
+                                    )
+                                except Exception as e:
+                                    logger.warning(
+                                        f"Failed to post error comment to primary issue "
+                                        f"{rebase_data.jira_issue}: {e}"
+                                    )
                                 # Link consolidated siblings to primary issue
                                 # (with per-sibling error handling)
                                 await post_failure_comments_to_consolidated_siblings(

ymir/agents/rebuild_agent.py

@@ -565,25 +565,20 @@ async def retry(
                         except Exception as e:
                             logger.warning(f"Failed to set labels on {issue_key}: {e}")
                     # Post failure feedback to Jira once, here on the final attempt
-                    # only — never for intermediate retries. Restricted to
-                    # user-triggered (ymir_todo) runs: a maintainer who didn't ask
-                    # for processing shouldn't be notified, so skip the gateway
-                    # connection entirely otherwise.
-                    if user_triggered and comment_text and not dry_run:
+                    # only — never for intermediate retries.
+                    if comment_text and not dry_run:
                         try:
                             async with mcp_tools(
                                 os.environ["MCP_GATEWAY_URL"],
                                 call_meta={"jira_issue": rebuild_data.jira_issue},
                             ) as gateway_tools:
                                 for issue_key in dict.fromkeys(rebuild_data.all_jira_issues):
                                     try:
-                                        await tasks.comment_in_jira(
+                                        await tasks.post_terminal_error_comment(
                                             jira_issue=issue_key,
                                             agent_type="Rebuild",
                                             comment_text=comment_text,
                                             available_tools=gateway_tools,
-                                            is_error=True,
-                                            user_triggered=user_triggered,
                                         )
                                     except Exception as comment_error:
                                         logger.warning(

ymir/agents/tasks.py

@@ -159,13 +159,11 @@ async def handle_zstream_branch_stale_error(
             ) as gateway_tools:
                 for issue_key in issues:
                     try:
-                        await comment_in_jira(
+                        await post_terminal_error_comment(
                             jira_issue=issue_key,
                             agent_type=agent_type,
                             comment_text=str(exc),
                             available_tools=gateway_tools,
-                            is_error=True,
-                            user_triggered=True,  # force-post regardless of actual trigger
                         )
                     except Exception as comment_error:
                         logger.warning(
@@ -669,13 +667,45 @@ async def comment_in_jira(
     is_error: bool = False,
     user_triggered: bool = False,
 ) -> None:
-    # Default is silent: error comments are only posted on user-triggered runs.
-    # A maintainer who didn't ask for processing should not be spammed with
-    # error notifications; if they want to see them, they add ymir_todo.
+    # Mid-workflow errors (e.g. consolidation failures in backport/rebase) are
+    # trigger-gated here; crash-based and resolution-based terminal errors
+    # bypass this via post_terminal_error_comment() after retries are exhausted.
     if is_error and not user_triggered:
         logger.info(f"Skipping Jira error comment for {jira_issue} (not user-triggered)")
         return
 
+    await _post_jira_comment(
+        jira_issue=jira_issue,
+        agent_type=agent_type,
+        comment_text=comment_text,
+        available_tools=available_tools,
+        is_error=is_error,
+    )
+
+
+async def post_terminal_error_comment(
+    jira_issue: str,
+    agent_type: str,
+    comment_text: str,
+    available_tools: list[Tool],
+) -> None:
+    """Post an error comment for a terminal failure, regardless of trigger."""
+    await _post_jira_comment(
+        jira_issue=jira_issue,
+        agent_type=agent_type,
+        comment_text=comment_text,
+        available_tools=available_tools,
+        is_error=True,
+    )
+
+
+async def _post_jira_comment(
+    jira_issue: str,
+    agent_type: str,
+    comment_text: str,
+    available_tools: list[Tool],
+    is_error: bool,
+) -> None:
     trace_server_url = trace_viewer_issue_url(jira_issue)
     if is_error and trace_server_url:
         comment_text = (