feat(jira): post final status comments with cost, turns, and duration (#573) (#603)

* feat(jira): post final status comments with cost, turns, and duration (#573)

Mirror the Linear final-status fan-out for Jira. Jira-origin terminal
tasks now get a deterministic platform-side comment with outcome, cost,
turns, duration, task id, and the PR link — posted by the fan-out plane,
not the agent, so it fires even when the agent crashes before completing
(max-turns, OOM).

- fanout: add `jira` NotificationChannel + CHANNEL_DEFAULTS (terminal
  events + task_timed_out) + `dispatchToJira`, structurally mirroring
  `dispatchToLinear` (env guard -> load task -> channel_source gate ->
  metadata read -> post-once marker -> post). New renderer
  `renderJiraFinalStatusComment` emits ADF paragraphs with the same
  three-outcome framing (completed / shipped-but-stopped / failed); the
  PR link renders on the success path too (Jira has no other surviving
  PR-link surface once the agent terminal comment is demoted).
- jira-feedback: add `buildAdfDocument` (multi-paragraph ADF with
  strong/em marks) + `postIssueCommentAdf` returning a classified
  `JiraPostResult` (retryable vs terminal), reusing the 401 forced-
  refresh-and-retry-once path. `postIssueComment` keeps its boolean API.
- TaskRecord: add `jira_final_comment_event_id` post-once marker;
  TaskNotificationsConfig gains a `jira` channel.
- construct/stack: wire JiraWorkspaceRegistryTable +
  `bgagent-jira-oauth-*` Get/Put grant into FanOutConsumer (guarded).
- agent: demote the Jira terminal comment — remove
  `comment_task_finished` and its two pipeline call sites so fanout owns
  the terminal comment (no double-post); keep the start comment.
- docs: update JIRA_SETUP_GUIDE for the platform-side final comment.

Tests: fanout Jira success / failure / shipped-but-stopped / timed_out /
missing-metadata / non-Jira skip / idempotent retry / retryable escalation
/ marker persistence / missing-env; ADF builder + classified post result;
construct grant guards; agent terminal-comment-demoted guard.

* feat(jira): render the PR link in the final-status comment as a clickable hyperlink

ADF — unlike Linear's Markdown — does not auto-linkify a bare URL in a
plain text node, so the PR link in the Jira final-status comment rendered
as unclickable text the requester had to copy-paste.

Add an optional `href` to `AdfTextRun`; `buildAdfDocument` maps it to an
ADF `link` mark (composes with strong/em). The Jira final-status renderer
splits the PR line into a "PR: " label run + a URL run carrying `href`, so
it renders as a real hyperlink on both the ✅ and ⚠️ paths.

Tests: buildAdfDocument link-mark emission + href/strong composition;
renderer asserts the URL run carries the href.

* fix(jira): address #603 review — reframe ✅ PR-link rationale, humanize task_timed_out header, match Linear bold scope

Review feedback on #603 (all from isadeks' approve-with-notes):

1. Drop the "diverges from Linear" framing for rendering the PR link on
   the ✅ success path. The real reason is general (ABCA-584): the agent's
   own "PR opened" comment isn't guaranteed to fire, so the platform
   comment must always carry the link. #601 lands the same fix for Linear,
   so this is no longer a Jira-specific divergence — reframed the docstring
   + renamed the test accordingly to avoid a stale-comment / semantic
   merge conflict with #601.

2. Humanize the ❌ header subtype: strip `task_` AND turn underscores into
   spaces so `task_timed_out` renders "Task timed out", not the raw
   "Task timed_out". Jira is the only channel routing task_timed_out
   through this renderer, so it's a case the copied-from-Linear code never
   hit. Added a test.

4. Match Linear's ⚠️ bold scope: bold only through the reason and leave
   the trailing "— review and decide…" advice unbolded (two runs), instead
   of bolding the whole header. Added a test.

(#3 — adding task_timed_out to CHANNEL_DEFAULTS.linear for parity — is an
optional follow-up the reviewer flagged as out of scope; tracked separately.)

---------

Co-authored-by: Sphia Sadek <isadeks@gmail.com>

Author: ayushtr-aws

agent/src/jira_reactions.py

@@ -1,9 +1,15 @@
 """Jira issue-comment helper for Jira-origin tasks.
 
-Posts a "starting" comment on the originating Jira issue at task start and a
-terminal "succeeded / failed (+ PR link)" comment at the end — the Jira
-analogue of ``linear_reactions`` (Linear uses emoji reactions; Jira's REST
-API has no lightweight reaction primitive, so comments are the right tool).
+Posts a "starting" comment on the originating Jira issue at task start — the
+Jira analogue of ``linear_reactions`` (Linear uses emoji reactions; Jira's
+REST API has no lightweight reaction primitive, so comments are the right
+tool).
+
+The *terminal* status comment is NOT posted from here: since issue #573 the
+deterministic fan-out plane (``cdk/src/handlers/fanout-task-events.ts``
+``dispatchToJira``) owns it, so it carries cost/turns/duration and fires even
+when this agent crashes before completing. This module only owns the start
+comment.
 
 Why a direct REST call instead of MCP: Atlassian's Remote MCP
 (``mcp.atlassian.com``) requires an interactive, browser-based OAuth 2.1
@@ -182,32 +188,13 @@ def comment_task_started(
     log("TASK", f"jira_reactions: comment_task_started issue={issue_key} ok={ok}")
 
 
-def comment_task_finished(
-    channel_source: str,
-    channel_metadata: dict[str, str] | None,
-    success: bool,
-    pr_url: str | None = None,
-) -> None:
-    """Post a terminal status comment on the Jira issue. No-op for non-Jira tasks."""
-    target = _enabled(channel_source, channel_metadata)
-    if not target:
-        return
-    cloud_id, issue_key = target
-    if success:
-        text = "✅ ABCA finished this task."
-        if pr_url:
-            text += f" Pull request: {pr_url}"
-        else:
-            text += " No pull request was opened."
-    else:
-        text = "❌ ABCA could not complete this task. Check the agent logs for details."
-        if pr_url:
-            text += f" A pull request was opened anyway: {pr_url}"
-    ok = _post_comment(cloud_id, issue_key, text)
-    log(
-        "TASK",
-        f"jira_reactions: comment_task_finished issue={issue_key} success={success} ok={ok}",
-    )
+# NOTE: there is deliberately no ``comment_task_finished`` here. Since issue
+# #573 the deterministic fan-out plane
+# (``cdk/src/handlers/fanout-task-events.ts`` ``dispatchToJira``) owns the Jira
+# terminal comment — it carries cost/turns/duration and, crucially, fires even
+# when the agent crashes before completing (max-turns, OOM). The agent only
+# posts the *start* comment (``comment_task_started`` above); posting a terminal
+# comment here too would double-comment on the issue.
 
 
 def _reset_state_for_testing() -> None:

agent/src/pipeline.py

@@ -23,7 +23,7 @@
     resolve_linear_api_token,
 )
 from context import assemble_prompt, fetch_github_issue
-from jira_reactions import comment_task_finished, comment_task_started
+from jira_reactions import comment_task_started
 from linear_reactions import react_task_finished, react_task_started
 from models import AgentResult, HydratedContext, RepoSetup, TaskConfig, TaskResult
 from observability import current_otel_trace_id, task_span
@@ -1106,14 +1106,14 @@ def _on_trace_truncated(max_bytes: int, first_dropped: int) -> None:
                 started_reaction_id=linear_eyes_reaction_id,
             )
 
-            # Terminal status comment on the Jira issue (REST shim, with the
-            # PR link when one was opened). No-op for non-Jira tasks.
-            comment_task_finished(
-                config.channel_source,
-                config.channel_metadata,
-                success=(overall_status == "success"),
-                pr_url=pr_url,
-            )
+            # NOTE: the terminal status comment on the Jira issue is NOT posted
+            # here. Since issue #573 the deterministic fan-out plane
+            # (``cdk/src/handlers/fanout-task-events.ts`` ``dispatchToJira``)
+            # owns the Jira final-status comment — it carries cost/turns/
+            # duration and, crucially, fires even if this agent crashes before
+            # reaching this point (max-turns, OOM). Posting here too would
+            # double-comment. The agent still posts the *start* comment
+            # (``comment_task_started`` above) for in-flight progress.
 
             # --trace trajectory S3 upload (design §10.1). Runs AFTER
             # post-hooks but BEFORE ``write_terminal`` so the resulting
@@ -1244,14 +1244,11 @@ def _on_trace_truncated(max_bytes: int, first_dropped: int) -> None:
                 success=False,
                 started_reaction_id=linear_eyes_reaction_id,
             )
-            # Best-effort failure comment on the Jira issue. No-op for
-            # non-Jira tasks; network failures are swallowed.
-            comment_task_finished(
-                config.channel_source,
-                config.channel_metadata,
-                success=False,
-                pr_url=None,
-            )
+            # NOTE: no Jira failure comment here — the fan-out plane's
+            # ``dispatchToJira`` (issue #573) owns the Jira terminal comment
+            # and fires on the platform side even when this crash path runs,
+            # so posting here would double-comment. (Contrast the Linear ❌
+            # reaction above, which the fan-out plane does not replicate.)
             raise
 
 

agent/tests/test_jira_reactions.py

@@ -7,7 +7,7 @@
 import pytest
 
 import jira_reactions
-from jira_reactions import comment_task_finished, comment_task_started
+from jira_reactions import comment_task_started
 
 JIRA_META = {"jira_cloud_id": "cloud-1", "jira_issue_key": "KAN-1"}
 
@@ -36,7 +36,6 @@ def test_non_jira_source_is_noop(self, monkeypatch):
         monkeypatch.setenv("JIRA_API_TOKEN", "jira_at")
         with patch("jira_reactions.requests.post") as post:
             comment_task_started("linear", JIRA_META)
-            comment_task_finished("linear", JIRA_META, success=True)
             post.assert_not_called()
 
     def test_empty_metadata_is_noop(self, monkeypatch):
@@ -74,31 +73,14 @@ def test_skips_when_token_missing(self, monkeypatch):
             post.assert_not_called()
 
 
-class TestFinishComment:
-    def test_success_with_pr_includes_pr_url(self, monkeypatch):
-        monkeypatch.setenv("JIRA_API_TOKEN", "jira_at")
-        with patch("jira_reactions.requests.post", return_value=_resp(201)) as post:
-            comment_task_finished(
-                "jira", JIRA_META, success=True, pr_url="https://github.com/o/r/pull/7"
-            )
-        text = post.call_args[1]["json"]["body"]["content"][0]["content"][0]["text"]
-        assert "✅" in text
-        assert "https://github.com/o/r/pull/7" in text
-
-    def test_success_without_pr_notes_no_pr(self, monkeypatch):
-        monkeypatch.setenv("JIRA_API_TOKEN", "jira_at")
-        with patch("jira_reactions.requests.post", return_value=_resp(201)) as post:
-            comment_task_finished("jira", JIRA_META, success=True, pr_url=None)
-        text = post.call_args[1]["json"]["body"]["content"][0]["content"][0]["text"]
-        assert "✅" in text
-        assert "No pull request" in text
+class TestTerminalCommentDemoted:
+    """Since issue #573 the fan-out plane (``dispatchToJira``) owns the Jira
+    terminal comment, so the agent no longer exposes ``comment_task_finished``.
+    This pins the demotion so a future refactor can't silently re-introduce a
+    duplicate terminal comment on the agent side."""
 
-    def test_failure_comment(self, monkeypatch):
-        monkeypatch.setenv("JIRA_API_TOKEN", "jira_at")
-        with patch("jira_reactions.requests.post", return_value=_resp(201)) as post:
-            comment_task_finished("jira", JIRA_META, success=False, pr_url=None)
-        text = post.call_args[1]["json"]["body"]["content"][0]["content"][0]["text"]
-        assert "❌" in text
+    def test_comment_task_finished_is_gone(self):
+        assert not hasattr(jira_reactions, "comment_task_finished")
 
 
 class TestFailureIsSwallowed:
@@ -107,7 +89,6 @@ def test_http_500_does_not_raise(self, monkeypatch):
         with patch("jira_reactions.requests.post", return_value=_resp(500, "boom")):
             # Must not raise.
             comment_task_started("jira", JIRA_META)
-            comment_task_finished("jira", JIRA_META, success=True)
 
     def test_request_exception_does_not_raise(self, monkeypatch):
         import requests
@@ -130,7 +111,7 @@ def test_opens_after_threshold_consecutive_401s(self, monkeypatch):
             calls_after_open = post.call_count
             # Further calls short-circuit without hitting the network.
             comment_task_started("jira", JIRA_META)
-            comment_task_finished("jira", JIRA_META, success=True)
+            comment_task_started("jira", JIRA_META)
             assert post.call_count == calls_after_open
 
     def test_2xx_resets_failure_counter(self, monkeypatch):

cdk/src/constructs/fanout-consumer.ts

@@ -106,6 +106,27 @@ export interface FanOutConsumerProps {
    */
   readonly linearOauthSecretArnPattern?: string;
 
+  /**
+   * JiraWorkspaceRegistryTable — the Jira dispatcher reads this to resolve
+   * per-tenant OAuth tokens at comment-post time (issue #573). Optional:
+   * when omitted, the dispatcher logs and skips so a deployment without
+   * Jira onboarding doesn't accumulate dangling IAM grants. Mirrors
+   * ``linearWorkspaceRegistryTable``.
+   */
+  readonly jiraWorkspaceRegistryTable?: dynamodb.ITable;
+
+  /**
+   * Secrets Manager ARN-prefix pattern for per-tenant Jira OAuth bundles.
+   * Mirrors ``linearOauthSecretArnPattern`` — typically
+   * ``bgagent-jira-oauth-*``. Required when ``jiraWorkspaceRegistryTable``
+   * is set; without it the dispatcher would resolve the registry row but
+   * fail at the SM GetSecretValue call. GetSecretValue + PutSecretValue
+   * because the resolver rotates an expiring token in place (the fan-out
+   * Lambda is trusted stack code, same as the orchestrator + webhook
+   * processor).
+   */
+  readonly jiraOauthSecretArnPattern?: string;
+
   /**
    * Maximum batch size delivered to the Lambda per invocation.
    *
@@ -231,6 +252,29 @@ export class FanOutConsumer extends Construct {
       }));
     }
 
+    // Jira dispatcher plumbing (issue #573). Same guarded shape as Linear:
+    // a deployment without Jira onboarding gets no IAM grants and the
+    // dispatcher logs-and-skips on missing env. The registry table resolves
+    // the per-tenant OAuth-secret ARN; the secret holds the access token
+    // ``postIssueCommentAdf`` uses to POST the final-status comment via the
+    // Jira REST v3 API.
+    if (props.jiraWorkspaceRegistryTable) {
+      props.jiraWorkspaceRegistryTable.grantReadData(this.fn);
+      this.fn.addEnvironment(
+        'JIRA_WORKSPACE_REGISTRY_TABLE_NAME',
+        props.jiraWorkspaceRegistryTable.tableName,
+      );
+    }
+    if (props.jiraOauthSecretArnPattern) {
+      this.fn.addToRolePolicy(new iam.PolicyStatement({
+        // GetSecretValue + PutSecretValue: resolving an expiring token
+        // refreshes it in place — same grants the orchestrator + Jira
+        // webhook-processor Lambdas hold.
+        actions: ['secretsmanager:GetSecretValue', 'secretsmanager:PutSecretValue'],
+        resources: [props.jiraOauthSecretArnPattern],
+      }));
+    }
+
     // Alarm on any record landing in the DLQ. Notifications are
     // best-effort by design, so individual failures don't fail the
     // batch — which means the DLQ is the ONLY persistent signal of a
@@ -244,7 +288,7 @@ export class FanOutConsumer extends Construct {
       threshold: 1,
       evaluationPeriods: 1,
       alarmDescription:
-        'Fan-out DLQ has undelivered task-event records — Slack/GitHub/Linear notifications are failing',
+        'Fan-out DLQ has undelivered task-event records — Slack/GitHub/Linear/Jira notifications are failing',
       treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING,
     });