diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index 390c288..a127171 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -18158,6 +18158,41 @@ def gitea_observability_list_projects( } +def _incident_recurrence_comment_fn( + *, + apply: bool, + remote: str, + host: str | None, + org: str | None, + repo: str | None, + worktree_path: str | None = None, +): + """Sanctioned issue-comment route for AC4 recurrence comments (#607). + + Returns ``None`` on dry runs (never comment) and when the active profile + lacks ``gitea.issue.comment``. Withholding the callable degrades to "link + updated, no comment" rather than failing the durable mapping, and the + reconcile result records why the comment was withheld. + """ + if not apply: + return None + if _profile_operation_gate("gitea.issue.comment"): + return None + + def comment_fn(issue_number, body, g_org, g_repo): + return gitea_create_issue_comment( + issue_number=int(issue_number), + body=body, + remote=remote, + host=host, + org=g_org or org, + repo=g_repo or repo, + worktree_path=worktree_path, + ) + + return comment_fn + + @mcp.tool() def gitea_observability_reconcile_incident( observation_json: str, @@ -18284,12 +18319,22 @@ def gitea_observability_reconcile_incident( worktree_path=worktree_path, ) + comment_fn = _incident_recurrence_comment_fn( + apply=bool(apply), + remote=remote, + host=host, + org=org, + repo=repo, + worktree_path=worktree_path, + ) + result = incident_bridge.reconcile_incident( db, observation=observation if isinstance(observation, dict) else {}, mappings=mappings, apply=bool(apply), create_issue_fn=create_fn, + comment_issue_fn=comment_fn, force_gitea_issue_number=force_gitea_issue_number, ) result["remote"] = remote @@ -18471,6 +18516,10 @@ def gitea_sentry_reconcile_issue( observation, then delegates dedupe/link/create to the sanctioned ``gitea_observability_reconcile_incident`` path. Dry-run by default. + Dedupe basis: provider identity — provider, base URL, org, project, and + Sentry issue id. This bridge does not populate ``fingerprint``, so + fingerprint plays no part in its dedupe decisions. + Gitea remains the source of truth; the raw Sentry incident is never an assignable control-plane work item. """ @@ -18626,8 +18675,14 @@ def gitea_sentry_watchdog( Dry-run by default. ``apply=true`` additionally requires ``MCP_SENTRY_ISSUE_BRIDGE_ENABLED`` and issue-create permission. Each - incident is deduped through the #612 ``incident_links`` substrate, so a - recurring failure updates one issue instead of creating duplicates. + incident is deduped through the #612 ``incident_links`` substrate by + provider identity (provider, base URL, org, project, Sentry issue id), so + a recurring failure updates one issue instead of creating duplicates. + + When a linked issue sees continued events, a sanitized recurrence comment + is posted through ``gitea_create_issue_comment`` (AC4). That requires + ``gitea.issue.comment``; without it the link still updates and the result + records that the comment was withheld. """ read_block = _profile_operation_gate("gitea.read") if read_block: @@ -18724,6 +18779,14 @@ def gitea_sentry_watchdog( limit=limit, max_pages=max_pages, create_issue_fn=create_fn, + comment_issue_fn=_incident_recurrence_comment_fn( + apply=bool(apply), + remote=remote, + host=host, + org=org, + repo=repo, + worktree_path=worktree_path, + ), ) result["remote"] = remote return result diff --git a/incident_bridge.py b/incident_bridge.py index 1a892da..df41ad9 100644 --- a/incident_bridge.py +++ b/incident_bridge.py @@ -484,6 +484,78 @@ def build_gitea_issue_body(inc: NormalizedIncident) -> str: return "\n".join(lines) +def incident_recurred( + existing: dict[str, Any], inc: NormalizedIncident +) -> tuple[bool, str]: + """Did new provider events arrive since the existing link was last synced? + + AC4 asks for a recurrence comment when events *continue*, so a scan that + observes no new events must stay silent instead of re-posting the same + state on every pass. + """ + old_count = existing.get("event_count") + new_count = inc.event_count + if ( + isinstance(old_count, int) + and isinstance(new_count, int) + and new_count > old_count + ): + return True, f"event_count advanced {old_count} -> {new_count}" + old_seen = str(existing.get("last_seen") or "").strip() + new_seen = str(inc.last_seen or "").strip() + if new_seen and new_seen != old_seen: + return True, f"last_seen advanced '{old_seen}' -> '{new_seen}'" + return False, "no new provider events since the last sync" + + +def build_recurrence_comment_body( + inc: NormalizedIncident, existing: dict[str, Any], *, reason: str = "" +) -> str: + """Sanitized recurrence comment for an already-linked Gitea issue (AC4). + + Uses the same redaction path as :func:`build_gitea_issue_body`; never + carries tokens, raw paths, or session state. + """ + lines = [ + "## Observability incident recurrence (bridge #612)", + "", + "", + f"", + "", + f"Continued `{inc.provider}` events for this linked incident.", + "", + f"- **provider_issue_id:** `{inc.provider_issue_id}`", + ] + if inc.provider_short_id: + lines.append(f"- **provider_short_id:** `{inc.provider_short_id}`") + if inc.provider_permalink: + lines.append(f"- **provider_url:** {inc.provider_permalink}") + lines.extend( + [ + f"- **event_count:** `{existing.get('event_count')}` -> " + f"`{inc.event_count if inc.event_count is not None else ''}`", + f"- **first_seen:** `{inc.first_seen or ''}`", + f"- **last_seen:** `{inc.last_seen or ''}`", + f"- **environment:** `{inc.environment or ''}`", + f"- **severity:** `{inc.severity or ''}`", + f"- **culprit:** `{inc.culprit or ''}`", + f"- **status:** `{inc.status}`", + f"- **recurrence_basis:** `{reason}`", + "", + "### Latest summary", + "", + redact_text(inc.summary) or "(no summary)", + "", + "### Canonical next action", + "", + "Author: this incident is still firing — investigate under the " + "normal Gitea workflow. This comment records observability " + "recurrence only and changes no workflow state.", + ] + ) + return "\n".join(lines) + + def _link_conflict(existing: dict[str, Any], inc: NormalizedIncident) -> str | None: """Fail closed if existing link targets a different Gitea issue/repo.""" eg_org = str(existing.get("gitea_org") or "") @@ -514,6 +586,9 @@ def _link_conflict(existing: dict[str, Any], inc: NormalizedIncident) -> str | N CreateIssueFn = Callable[[str, str, list[str], str, str], dict[str, Any]] # create_issue_fn(title, body, labels, gitea_org, gitea_repo) -> {"number": int, ...} +CommentIssueFn = Callable[[int, str, str, str], dict[str, Any]] +# comment_issue_fn(gitea_issue_number, body, gitea_org, gitea_repo) -> {"success": bool, ...} + def reconcile_incident( db: ControlPlaneDB | None, @@ -523,6 +598,7 @@ def reconcile_incident( mapping: ProjectMapping | None = None, apply: bool = False, create_issue_fn: CreateIssueFn | None = None, + comment_issue_fn: CommentIssueFn | None = None, force_gitea_issue_number: int | None = None, ) -> dict[str, Any]: """Reconcile one observation into incident_links + optional Gitea issue. @@ -531,6 +607,11 @@ def reconcile_incident( *apply=True*: upsert link; create Gitea issue when none linked (requires ``create_issue_fn``) or use ``force_gitea_issue_number`` for explicit link. + When an existing link is reused and the provider reports *new* events, + ``comment_issue_fn`` posts a sanitized recurrence comment on the linked + Gitea issue (AC4). Dry runs never comment, and a missing + ``comment_issue_fn`` withholds the comment without failing the link. + Never creates control-plane ``work_items`` for raw incidents. """ base: dict[str, Any] = { @@ -549,6 +630,7 @@ def reconcile_incident( "gitea_issue": None, "action": None, "mapping": None, + "recurrence_comment": None, "substrate": "control_plane_db.incident_links", "durable_work_system": "gitea_issues", } @@ -652,10 +734,14 @@ def reconcile_incident( # --- apply path --- issue_number: int | None = None created = False + recurrence: tuple[bool, str] | None = None if existing: issue_number = int(existing["gitea_issue_number"]) action = "updated_existing_link" outcome = OUTCOME_UPDATED + # Compare against the pre-upsert link row: the upsert below overwrites + # event_count/last_seen, which would erase the recurrence signal. + recurrence = incident_recurred(existing, inc) elif force_gitea_issue_number is not None: issue_number = int(force_gitea_issue_number) action = "link_explicit_issue" @@ -729,6 +815,63 @@ def reconcile_incident( } return base + # AC4: continued provider events post a recurrence comment on the linked + # Gitea issue. The durable incident_links row is already written above, so + # a comment failure never rolls back or blocks the mapping — the next scan + # retries while the link stays authoritative. + if outcome == OUTCOME_UPDATED and recurrence is not None: + recurred, why = recurrence + if not recurred: + base["recurrence_comment"] = {"posted": False, "reason": why} + elif comment_issue_fn is None: + base["recurrence_comment"] = { + "posted": False, + "reason": ( + "no comment_issue_fn supplied; recurrence comment withheld " + "(link remains durable)" + ), + "recurrence_basis": why, + } + else: + try: + comment_res = comment_issue_fn( + issue_number, + build_recurrence_comment_body(inc, existing, reason=why), + inc.gitea_org, + inc.gitea_repo, + ) + except Exception as exc: # noqa: BLE001 - never break the link write + base["recurrence_comment"] = { + "posted": False, + "reason": ( + f"recurrence comment failed: {redact_text(exc)} " + "(link remains durable)" + ), + "recurrence_basis": why, + } + else: + posted = ( + bool(comment_res.get("success")) + if isinstance(comment_res, dict) + else bool(comment_res) + ) + base["recurrence_comment"] = { + "posted": posted, + "recurrence_basis": why, + "gitea_issue_number": issue_number, + "comment_id": ( + comment_res.get("comment_id") + if isinstance(comment_res, dict) + else None + ), + } + if posted: + base["gitea_mutated"] = True + elif isinstance(comment_res, dict): + base["recurrence_comment"]["reasons"] = [ + redact_text(r) for r in (comment_res.get("reasons") or []) + ] + base["success"] = True base["performed"] = True base["db_mutated"] = True diff --git a/sentry_incident_bridge.py b/sentry_incident_bridge.py index 91d1d71..56bab8e 100644 --- a/sentry_incident_bridge.py +++ b/sentry_incident_bridge.py @@ -590,6 +590,7 @@ def watchdog( max_pages: int = DEFAULT_MAX_PAGES, http_fn: HttpFn | None = None, create_issue_fn: Any = None, + comment_issue_fn: Any = None, reconcile_fn: Callable[..., dict[str, Any]] | None = None, fetch_events: bool = True, ) -> dict[str, Any]: @@ -597,6 +598,9 @@ def watchdog( Dry-run by default. ``apply=True`` additionally requires the bridge to be explicitly enabled via ``MCP_SENTRY_ISSUE_BRIDGE_ENABLED``. + + ``comment_issue_fn`` carries the sanctioned issue-comment route used for + AC4 recurrence comments on already-linked issues; dry runs never comment. """ result: dict[str, Any] = { "success": False, @@ -682,6 +686,7 @@ def watchdog( mappings=list(mappings or []), apply=bool(apply), create_issue_fn=create_issue_fn, + comment_issue_fn=comment_issue_fn, ) except Exception as exc: # noqa: BLE001 - one bad issue must not abort the scan result["failed"] += 1 @@ -702,6 +707,7 @@ def watchdog( "outcome": reconciled.get("outcome"), "gitea_issue": reconciled.get("gitea_issue"), "existing_link": reconciled.get("existing_link"), + "recurrence_comment": reconciled.get("recurrence_comment"), "reasons": reconciled.get("reasons"), } ) diff --git a/tests/test_sentry_incident_bridge.py b/tests/test_sentry_incident_bridge.py index c175888..8f19510 100644 --- a/tests/test_sentry_incident_bridge.py +++ b/tests/test_sentry_incident_bridge.py @@ -130,7 +130,9 @@ class SentryBridgeTestCase(unittest.TestCase): self.db_path = os.path.join(self._tmp.name, "cp.sqlite3") self.db = ControlPlaneDB(self.db_path) self.created: list[dict] = [] + self.comments: list[dict] = [] self._next_issue_number = 900 + self._next_comment_id = 5000 def _create_issue_fn(self): def create_fn(title, body, labels, g_org, g_repo): @@ -149,6 +151,22 @@ class SentryBridgeTestCase(unittest.TestCase): return create_fn + def _comment_issue_fn(self): + def comment_fn(issue_number, body, g_org, g_repo): + self._next_comment_id += 1 + self.comments.append( + { + "issue_number": issue_number, + "body": body, + "org": g_org, + "repo": g_repo, + "comment_id": self._next_comment_id, + } + ) + return {"success": True, "comment_id": self._next_comment_id} + + return comment_fn + def _link(self, issue_id: str = "4001"): return self.db.get_incident_link_by_provider( provider="sentry", @@ -167,6 +185,9 @@ class SentryBridgeTestCase(unittest.TestCase): mappings=[_mapping()], http_fn=http, create_issue_fn=self._create_issue_fn(), + # Always supplied, including dry runs: the bridge itself must + # withhold the comment when apply=False (AC4 + AC8). + comment_issue_fn=self._comment_issue_fn(), **kwargs, ) @@ -379,6 +400,101 @@ class TestCreateUpdateDedupe(SentryBridgeTestCase): self.assertEqual(result["results"][0]["outcome"], OUTCOME_UPDATED) self.assertEqual(self._link()["event_count"], 42) + def test_recurrence_posts_a_comment_on_the_second_scan(self): + """AC4: continued Sentry events comment on the linked Gitea issue.""" + first = self._watchdog(FakeHttp()) + self.assertEqual(first["results"][0]["outcome"], OUTCOME_CREATED) + self.assertEqual(self.comments, [], "creation must not post a recurrence comment") + + recurring = FakeHttp(routes=[(200, [_raw_issue("4001", count="42")], {})]) + second = self._watchdog(recurring) + + self.assertEqual(second["results"][0]["outcome"], OUTCOME_UPDATED) + self.assertEqual(len(self.comments), 1, "recurrence must post exactly one comment") + comment = self.comments[0] + linked_number = int(self._link()["gitea_issue_number"]) + self.assertEqual(comment["issue_number"], linked_number) + self.assertEqual(comment["org"], GITEA_ORG) + self.assertEqual(comment["repo"], GITEA_REPO) + # AC5 fields carried on the recurrence record. + self.assertIn("4001", comment["body"]) + self.assertIn("42", comment["body"]) + self.assertIn("recurrence_basis", comment["body"]) + reported = second["results"][0]["recurrence_comment"] + self.assertTrue(reported["posted"]) + self.assertEqual(reported["comment_id"], comment["comment_id"]) + self.assertEqual(len(self.created), 1, "recurrence must not create a duplicate issue") + + def test_dry_run_scan_posts_no_recurrence_comment(self): + """AC4 + AC8: dry run never comments, even on a linked recurrence.""" + self._watchdog(FakeHttp()) + recurring = FakeHttp(routes=[(200, [_raw_issue("4001", count="42")], {})]) + result = self._watchdog(recurring, apply=False) + + self.assertEqual(result["results"][0]["outcome"], OUTCOME_PREVIEW) + self.assertEqual(self.comments, [], "dry-run must not post recurrence comments") + + def test_repeat_scan_without_new_events_posts_no_comment(self): + """A scan that observes no new events must stay silent.""" + self._watchdog(FakeHttp()) + result = self._watchdog(FakeHttp()) + + self.assertEqual(result["results"][0]["outcome"], OUTCOME_UPDATED) + self.assertEqual(self.comments, [], "unchanged event state must not comment") + self.assertFalse(result["results"][0]["recurrence_comment"]["posted"]) + + def test_recurrence_comment_failure_keeps_the_link_durable(self): + """A failed comment must not roll back or block the incident_links row.""" + self._watchdog(FakeHttp()) + + def failing_comment(issue_number, body, g_org, g_repo): + raise RuntimeError("gitea comment route unavailable") + + recurring = FakeHttp(routes=[(200, [_raw_issue("4001", count="42")], {})]) + result = bridge.watchdog( + self.db, + _config(), + token=TOKEN, + apply=True, + mappings=[_mapping()], + http_fn=recurring, + create_issue_fn=self._create_issue_fn(), + comment_issue_fn=failing_comment, + ) + + entry = result["results"][0] + self.assertEqual(entry["outcome"], OUTCOME_UPDATED) + self.assertFalse(entry["recurrence_comment"]["posted"]) + self.assertEqual(self._link()["event_count"], 42, "link must still be updated") + + def test_recurrence_comment_is_redacted(self): + """AC2: recurrence comments pass through the same redaction path.""" + self._watchdog(FakeHttp()) + recurring = FakeHttp( + routes=[ + ( + 200, + [ + _raw_issue( + "4001", + count="42", + metadata={ + "type": "RuntimeError", + "value": "token=abc123supersecret", + }, + ) + ], + {}, + ) + ] + ) + self._watchdog(recurring) + + self.assertEqual(len(self.comments), 1) + body = self.comments[0]["body"] + self.assertNotIn("abc123supersecret", body) + self.assertNotIn(TOKEN, body) + def test_link_survives_a_new_db_handle(self): """AC6: bridge mapping survives process restarts.""" self._watchdog(FakeHttp())