From c23671476c5748c4fe89853cdaad1591dbf470b1 Mon Sep 17 00:00:00 2001 From: Spencer Murray Date: Wed, 21 Oct 2026 17:09:14 -0400 Subject: [PATCH 1/8] Notify creators once about action items filed in the incident team --- src/firetower/incidents/tasks/action_items.py | 58 ++++++++ .../tests/test_action_item_team_reminders.py | 135 ++++++++++++++++++ src/firetower/incidents/tests/test_tasks.py | 7 + src/firetower/integrations/services/linear.py | 61 +++++++- .../tests/test_linear_team_reminders.py | 102 +++++++++++++ 5 files changed, 357 insertions(+), 6 deletions(-) create mode 100644 src/firetower/incidents/tests/test_action_item_team_reminders.py create mode 100644 src/firetower/integrations/tests/test_linear_team_reminders.py diff --git a/src/firetower/incidents/tasks/action_items.py b/src/firetower/incidents/tasks/action_items.py index 2eebad10..c43ef7b2 100644 --- a/src/firetower/incidents/tasks/action_items.py +++ b/src/firetower/incidents/tasks/action_items.py @@ -1,6 +1,7 @@ import logging from dataclasses import dataclass from datetime import timedelta +from uuid import NAMESPACE_URL, UUID, uuid5 from django.conf import settings from django.utils import timezone @@ -71,10 +72,67 @@ def _build_nag_tiers(linear: dict) -> dict[int, NagTier]: _NAG_TEMPLATE_ENV = Environment(autoescape=False) +MISFILED_ACTION_ITEM_REMINDER_MARKER = "Firetower action-item team reminder" + + +def _remind_misfiled_action_item_creators(linear: dict) -> None: + team_id = linear.get("TEAM_ID") + if not team_id: + return + + service = LinearService() + try: + issues = service.get_misfiled_action_items( + team_id, MISFILED_ACTION_ITEM_REMINDER_MARKER + ) + except Exception: + logger.exception("Failed to find misfiled action items") + return + if issues is None: + logger.error("Failed to find misfiled action items") + return + + for issue in issues: + creator = issue.get("creator") + greeting = ( + f"Hi @[{creator['displayName']}]({creator['id']}), " if creator else "Hi, " + ) + comment = ( + f"**{MISFILED_ACTION_ITEM_REMINDER_MARKER}**\n\n" + f"{greeting}this action item was created in the " + f"{settings.PROJECT_KEY} team. Please move it to the relevant " + "actioning team, keeping it as a child of the incident ticket. " + "The incident team is reserved for incident tickets, not action items." + ) + # A stable UUID in Linear's required v4 format also prevents duplicate + # comments from overlapping runs or a successful but timed-out request. + comment_id = str( + UUID( + bytes=uuid5( + NAMESPACE_URL, f"firetower:inc-team-reminder:{issue['id']}" + ).bytes, + version=4, + ) + ) + try: + success = service.create_comment( + issue["id"], comment, comment_id=comment_id + ) + except Exception: + logger.exception( + "Failed to post team reminder for action item %s", issue["identifier"] + ) + continue + if not success: + logger.error( + "Failed to post team reminder for action item %s", issue["identifier"] + ) + @datadog_log def send_action_item_reminder() -> None: linear = settings.LINEAR or {} + _remind_misfiled_action_item_creators(linear) nag_tiers = _build_nag_tiers(linear) comments_by_priority: dict[int, str] = { priority: linear.get(tier.comment_setting_key, "") diff --git a/src/firetower/incidents/tests/test_action_item_team_reminders.py b/src/firetower/incidents/tests/test_action_item_team_reminders.py new file mode 100644 index 00000000..a32958ce --- /dev/null +++ b/src/firetower/incidents/tests/test_action_item_team_reminders.py @@ -0,0 +1,135 @@ +from unittest.mock import MagicMock, patch +from uuid import UUID + +import pytest + +from firetower.incidents.tasks.action_items import ( + MISFILED_ACTION_ITEM_REMINDER_MARKER, + _remind_misfiled_action_item_creators, + send_action_item_reminder, +) + +LINEAR = {"TEAM_ID": "incident-team"} +ISSUE = { + "id": "issue-1", + "identifier": "INC-2508", + "creator": {"id": "creator-id", "displayName": "Leander Rodrigues"}, +} + + +@pytest.fixture +def service(settings): + settings.PROJECT_KEY = "INC" + settings.LINEAR = LINEAR + service = MagicMock() + service.get_misfiled_action_items.return_value = [ISSUE] + service.create_comment.return_value = True + with patch( + "firetower.incidents.tasks.action_items.LinearService", return_value=service + ): + yield service + + +def test_mentions_creator_and_explains_move(service): + _remind_misfiled_action_item_creators(LINEAR) + + service.get_misfiled_action_items.assert_called_once_with( + "incident-team", MISFILED_ACTION_ITEM_REMINDER_MARKER + ) + args, kwargs = service.create_comment.call_args + assert args[0] == ISSUE["id"] + assert "@[Leander Rodrigues](creator-id)" in args[1] + assert MISFILED_ACTION_ITEM_REMINDER_MARKER in args[1] + assert "created in the INC team" in args[1] + assert "move it to the relevant actioning team" in args[1] + assert "keeping it as a child" in args[1] + assert UUID(kwargs["comment_id"]).version == 4 + + +def test_skips_issues_with_existing_reminder_on_later_runs(service): + service.get_misfiled_action_items.side_effect = [[ISSUE], []] + + _remind_misfiled_action_item_creators(LINEAR) + _remind_misfiled_action_item_creators(LINEAR) + + service.create_comment.assert_called_once() + + +def test_comment_id_is_stable_across_retries(service): + service.create_comment.side_effect = [TimeoutError("response lost"), True] + + _remind_misfiled_action_item_creators(LINEAR) + _remind_misfiled_action_item_creators(LINEAR) + + calls = service.create_comment.call_args_list + assert len(calls) == 2 + assert calls[0].kwargs["comment_id"] == calls[1].kwargs["comment_id"] + + +def test_comment_ids_differ_between_issues(service): + service.get_misfiled_action_items.return_value = [ + ISSUE, + {**ISSUE, "id": "issue-2", "identifier": "INC-2509"}, + ] + + _remind_misfiled_action_item_creators(LINEAR) + + calls = service.create_comment.call_args_list + assert calls[0].kwargs["comment_id"] != calls[1].kwargs["comment_id"] + + +def test_handles_missing_creator(service): + service.get_misfiled_action_items.return_value = [{**ISSUE, "creator": None}] + + _remind_misfiled_action_item_creators(LINEAR) + + body = service.create_comment.call_args.args[1] + assert "Hi, this action item" in body + assert "@[" not in body + + +@pytest.mark.parametrize("config", [{}, {"TEAM_ID": ""}]) +def test_skips_when_team_is_not_configured(service, config): + _remind_misfiled_action_item_creators(config) + + service.get_misfiled_action_items.assert_not_called() + service.create_comment.assert_not_called() + + +@pytest.mark.parametrize("failure", [None, RuntimeError("Linear unavailable")]) +def test_does_not_post_when_lookup_fails(service, failure): + if isinstance(failure, Exception): + service.get_misfiled_action_items.side_effect = failure + else: + service.get_misfiled_action_items.return_value = failure + + _remind_misfiled_action_item_creators(LINEAR) + + service.create_comment.assert_not_called() + + +@pytest.mark.parametrize("failure", [False, RuntimeError("Linear unavailable")]) +def test_continues_after_a_comment_failure(service, failure): + service.get_misfiled_action_items.return_value = [ + ISSUE, + {**ISSUE, "id": "issue-2", "identifier": "INC-2509"}, + ] + service.create_comment.side_effect = [failure, True] + + _remind_misfiled_action_item_creators(LINEAR) + + assert service.create_comment.call_count == 2 + + +def test_uses_configured_incident_team_key(service, settings): + settings.PROJECT_KEY = "TESTINC" + + _remind_misfiled_action_item_creators(LINEAR) + + assert "created in the TESTINC team" in service.create_comment.call_args.args[1] + + +def test_cron_sweeps_even_without_slo_nag_configuration(service): + send_action_item_reminder() + + service.create_comment.assert_called_once() diff --git a/src/firetower/incidents/tests/test_tasks.py b/src/firetower/incidents/tests/test_tasks.py index 13c2140b..6230db56 100644 --- a/src/firetower/incidents/tests/test_tasks.py +++ b/src/firetower/incidents/tests/test_tasks.py @@ -838,6 +838,13 @@ def _configure_linear(self): with patch.object(settings, "LINEAR", linear_settings): yield + @pytest.fixture(autouse=True) + def mock_team_reminders(self): + with patch( + "firetower.incidents.tasks.action_items._remind_misfiled_action_item_creators" + ) as mock: + yield mock + @pytest.fixture(autouse=True) def mock_sync(self): with patch( diff --git a/src/firetower/integrations/services/linear.py b/src/firetower/integrations/services/linear.py index d980a10d..0cd3f13c 100644 --- a/src/firetower/integrations/services/linear.py +++ b/src/firetower/integrations/services/linear.py @@ -545,7 +545,57 @@ def create_attachment(self, issue_id: str, url: str, title: str) -> bool: return data.get("attachmentCreate", {}).get("success", False) - def create_comment(self, issue_id: str, body: str) -> bool: + def get_misfiled_action_items( + self, team_id: str, reminder_marker: str + ) -> list[dict[str, Any]] | None: + """Find unnotified child issues in the incident team, in any status.""" + query = """ + query($filter: IssueFilter!, $after: String) { + issues( + filter: $filter, includeArchived: true, first: 50, after: $after + ) { + nodes { + id + identifier + creator { + id + displayName + } + } + pageInfo { + hasNextPage + endCursor + } + } + } + """ + issue_filter = { + "team": {"id": {"eq": team_id}}, + "number": {"gte": 2000}, + "parent": {"team": {"id": {"eq": team_id}}}, + "comments": {"none": {"body": {"contains": reminder_marker}}}, + } + issues: list[dict[str, Any]] = [] + cursor: str | None = None + for _ in range(25): + data = self._graphql(query, {"filter": issue_filter, "after": cursor}) + if not data or not data.get("issues"): + return None + page = data["issues"] + issues.extend(page["nodes"]) + if not page["pageInfo"]["hasNextPage"]: + return issues + next_cursor = page["pageInfo"]["endCursor"] + if not next_cursor or next_cursor == cursor: + break + cursor = next_cursor + + logger.error("Incomplete Linear misfiled action item pagination") + return None + + def create_comment( + self, issue_id: str, body: str, *, comment_id: str | None = None + ) -> bool: mutation = """ mutation($input: CommentCreateInput!) { commentCreate(input: $input) { @@ -553,11 +603,10 @@ def create_comment(self, issue_id: str, body: str) -> bool: } } """ - data = self._graphql( - mutation, - {"input": {"issueId": issue_id, "body": body}}, - retryable=False, - ) + input_data = {"issueId": issue_id, "body": body} + if comment_id is not None: + input_data["id"] = comment_id + data = self._graphql(mutation, {"input": input_data}, retryable=False) if not data: return False diff --git a/src/firetower/integrations/tests/test_linear_team_reminders.py b/src/firetower/integrations/tests/test_linear_team_reminders.py new file mode 100644 index 00000000..aa1569c3 --- /dev/null +++ b/src/firetower/integrations/tests/test_linear_team_reminders.py @@ -0,0 +1,102 @@ +from unittest.mock import patch + +import pytest + +from firetower.integrations.services.linear import LinearService + + +@pytest.fixture +def service(settings): + settings.LINEAR = {"API_KEY": "test"} + return LinearService() + + +def page(issues, has_next=False, cursor=None): + return { + "issues": { + "nodes": issues, + "pageInfo": {"hasNextPage": has_next, "endCursor": cursor}, + } + } + + +def test_filters_by_child_team_parent_team_number_and_existing_comments(service): + with patch.object(service, "_graphql", return_value=page([])) as graphql: + assert service.get_misfiled_action_items("team-id", "reminder-marker") == [] + + query, variables = graphql.call_args.args + assert variables == { + "filter": { + "team": {"id": {"eq": "team-id"}}, + "number": {"gte": 2000}, + "parent": {"team": {"id": {"eq": "team-id"}}}, + "comments": {"none": {"body": {"contains": "reminder-marker"}}}, + }, + "after": None, + } + assert "includeArchived: true" in query + assert "creator {" in query + assert "displayName" in query + + +def test_paginates_all_candidates_before_posting(service): + first = {"id": "issue-1", "creator": None} + second = {"id": "issue-2", "creator": {"id": "user-1", "displayName": "Name"}} + with patch.object( + service, + "_graphql", + side_effect=[page([first], True, "cursor-1"), page([second])], + ) as graphql: + assert service.get_misfiled_action_items("team-id", "marker") == [first, second] + + assert graphql.call_args_list[1].args[1]["after"] == "cursor-1" + + +@pytest.mark.parametrize("response", [None, {}, {"issues": None}]) +def test_returns_none_when_query_fails(service, response): + with patch.object(service, "_graphql", return_value=response): + assert service.get_misfiled_action_items("team-id", "marker") is None + + +def test_does_not_return_partial_results_on_failure(service): + with patch.object( + service, + "_graphql", + side_effect=[page([{"id": "issue-1"}], True, "cursor-1"), None], + ): + assert service.get_misfiled_action_items("team-id", "marker") is None + + +@pytest.mark.parametrize("cursor", [None, "cursor-1"]) +def test_fails_closed_when_pagination_cannot_advance(service, cursor): + with patch.object( + service, + "_graphql", + side_effect=[page([], True, "cursor-1"), page([], True, cursor)], + ): + assert service.get_misfiled_action_items("team-id", "marker") is None + + +def test_fails_closed_at_page_limit(service): + with patch.object( + service, + "_graphql", + side_effect=[page([], True, f"cursor-{i}") for i in range(25)], + ) as graphql: + assert service.get_misfiled_action_items("team-id", "marker") is None + + assert graphql.call_count == 25 + + +@pytest.mark.parametrize("comment_id", [None, "stable-comment-id"]) +def test_passes_optional_comment_id_without_retrying_mutation(service, comment_id): + with patch.object( + service, "_graphql", return_value={"commentCreate": {"success": True}} + ) as graphql: + assert service.create_comment("issue-1", "body", comment_id=comment_id) + + expected_input = {"issueId": "issue-1", "body": "body"} + if comment_id is not None: + expected_input["id"] = comment_id + assert graphql.call_args.args[1] == {"input": expected_input} + assert graphql.call_args.kwargs == {"retryable": False} From 6cc37bd1da413f570d2d5d85e72c1e9a60986d5d Mon Sep 17 00:00:00 2001 From: Spencer Murray Date: Thu, 22 Oct 2026 12:33:23 -0400 Subject: [PATCH 2/8] Use supported Linear filters to exclude existing reminders --- src/firetower/integrations/services/linear.py | 2 +- src/firetower/integrations/tests/test_linear_team_reminders.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/firetower/integrations/services/linear.py b/src/firetower/integrations/services/linear.py index 0cd3f13c..40df08c2 100644 --- a/src/firetower/integrations/services/linear.py +++ b/src/firetower/integrations/services/linear.py @@ -573,7 +573,7 @@ def get_misfiled_action_items( "team": {"id": {"eq": team_id}}, "number": {"gte": 2000}, "parent": {"team": {"id": {"eq": team_id}}}, - "comments": {"none": {"body": {"contains": reminder_marker}}}, + "comments": {"every": {"body": {"notContains": reminder_marker}}}, } issues: list[dict[str, Any]] = [] cursor: str | None = None diff --git a/src/firetower/integrations/tests/test_linear_team_reminders.py b/src/firetower/integrations/tests/test_linear_team_reminders.py index aa1569c3..d577755c 100644 --- a/src/firetower/integrations/tests/test_linear_team_reminders.py +++ b/src/firetower/integrations/tests/test_linear_team_reminders.py @@ -30,7 +30,7 @@ def test_filters_by_child_team_parent_team_number_and_existing_comments(service) "team": {"id": {"eq": "team-id"}}, "number": {"gte": 2000}, "parent": {"team": {"id": {"eq": "team-id"}}}, - "comments": {"none": {"body": {"contains": "reminder-marker"}}}, + "comments": {"every": {"body": {"notContains": "reminder-marker"}}}, }, "after": None, } From bf7d322811de0802e0deec2dcc53e75b5016b48a Mon Sep 17 00:00:00 2001 From: Spencer Murray Date: Thu, 22 Oct 2026 12:33:25 -0400 Subject: [PATCH 3/8] Isolate malformed action items from the reminder sweep --- src/firetower/incidents/tasks/action_items.py | 56 +++++++++---------- .../tests/test_action_item_team_reminders.py | 49 ++++++++++++++++ 2 files changed, 77 insertions(+), 28 deletions(-) diff --git a/src/firetower/incidents/tasks/action_items.py b/src/firetower/incidents/tasks/action_items.py index c43ef7b2..964e2fdd 100644 --- a/src/firetower/incidents/tasks/action_items.py +++ b/src/firetower/incidents/tasks/action_items.py @@ -80,8 +80,8 @@ def _remind_misfiled_action_item_creators(linear: dict) -> None: if not team_id: return - service = LinearService() try: + service = LinearService() issues = service.get_misfiled_action_items( team_id, MISFILED_ACTION_ITEM_REMINDER_MARKER ) @@ -93,40 +93,40 @@ def _remind_misfiled_action_item_creators(linear: dict) -> None: return for issue in issues: - creator = issue.get("creator") - greeting = ( - f"Hi @[{creator['displayName']}]({creator['id']}), " if creator else "Hi, " - ) - comment = ( - f"**{MISFILED_ACTION_ITEM_REMINDER_MARKER}**\n\n" - f"{greeting}this action item was created in the " - f"{settings.PROJECT_KEY} team. Please move it to the relevant " - "actioning team, keeping it as a child of the incident ticket. " - "The incident team is reserved for incident tickets, not action items." - ) - # A stable UUID in Linear's required v4 format also prevents duplicate - # comments from overlapping runs or a successful but timed-out request. - comment_id = str( - UUID( - bytes=uuid5( - NAMESPACE_URL, f"firetower:inc-team-reminder:{issue['id']}" - ).bytes, - version=4, - ) - ) + issue_id = None try: - success = service.create_comment( - issue["id"], comment, comment_id=comment_id + issue_id = issue["id"] + creator = issue.get("creator") + greeting = ( + f"Hi @[{creator['displayName']}]({creator['id']}), " + if creator + else "Hi, " ) + comment = ( + f"**{MISFILED_ACTION_ITEM_REMINDER_MARKER}**\n\n" + f"{greeting}this action item was created in the " + f"{settings.PROJECT_KEY} team. Please move it to the relevant " + "actioning team, keeping it as a child of the incident ticket. " + "The incident team is reserved for incident tickets, not action items." + ) + # A stable UUID in Linear's required v4 format also prevents duplicate + # comments from overlapping runs or a successful but timed-out request. + comment_id = str( + UUID( + bytes=uuid5( + NAMESPACE_URL, f"firetower:inc-team-reminder:{issue_id}" + ).bytes, + version=4, + ) + ) + success = service.create_comment(issue_id, comment, comment_id=comment_id) except Exception: logger.exception( - "Failed to post team reminder for action item %s", issue["identifier"] + "Failed to post team reminder for action item %s", issue_id ) continue if not success: - logger.error( - "Failed to post team reminder for action item %s", issue["identifier"] - ) + logger.error("Failed to post team reminder for action item %s", issue_id) @datadog_log diff --git a/src/firetower/incidents/tests/test_action_item_team_reminders.py b/src/firetower/incidents/tests/test_action_item_team_reminders.py index a32958ce..c0228906 100644 --- a/src/firetower/incidents/tests/test_action_item_team_reminders.py +++ b/src/firetower/incidents/tests/test_action_item_team_reminders.py @@ -121,6 +121,55 @@ def test_continues_after_a_comment_failure(service, failure): assert service.create_comment.call_count == 2 +@pytest.mark.parametrize( + "malformed", + [ + None, + {}, + {**ISSUE, "creator": {"id": "creator-id"}}, + {**ISSUE, "creator": {"displayName": "Leander Rodrigues"}}, + ], +) +def test_continues_after_malformed_issue_data(service, malformed): + service.get_misfiled_action_items.return_value = [malformed, ISSUE] + + _remind_misfiled_action_item_creators(LINEAR) + + service.create_comment.assert_called_once() + assert service.create_comment.call_args.args[0] == ISSUE["id"] + + +def test_initialization_failure_does_not_escape(service): + with patch( + "firetower.incidents.tasks.action_items.LinearService", + side_effect=RuntimeError("initialization failed"), + ): + _remind_misfiled_action_item_creators(LINEAR) + + service.create_comment.assert_not_called() + + +def test_malformed_issue_does_not_block_slo_sweep(service, settings): + settings.LINEAR = {**LINEAR, "ACTION_ITEM_NAG_COMMENT_HIGH_PRIORITY": "SLO nag"} + service.get_misfiled_action_items.return_value = [ + {**ISSUE, "creator": {"id": "creator-id"}} + ] + incident = MagicMock() + incident.action_items.filter.return_value = [] + with ( + patch("firetower.incidents.tasks.action_items.Incident") as incident_model, + patch( + "firetower.incidents.tasks.action_items.sync_action_items_from_linear" + ) as sync, + ): + incident_model.objects.filter.return_value.exclude.return_value = [incident] + + send_action_item_reminder() + + sync.assert_called_once_with(incident) + service.create_comment.assert_not_called() + + def test_uses_configured_incident_team_key(service, settings): settings.PROJECT_KEY = "TESTINC" From 656c6c9393aeb046c81909bc820bf166c3a821a3 Mon Sep 17 00:00:00 2001 From: Spencer Murray Date: Thu, 22 Oct 2026 17:02:29 -0400 Subject: [PATCH 4/8] Include comment-free action items in the team reminder sweep --- src/firetower/incidents/tasks/action_items.py | 7 ++++++ .../tests/test_action_item_team_reminders.py | 23 +++++++++++++++++++ src/firetower/integrations/services/linear.py | 5 +++- .../tests/test_linear_team_reminders.py | 5 +++- 4 files changed, 38 insertions(+), 2 deletions(-) diff --git a/src/firetower/incidents/tasks/action_items.py b/src/firetower/incidents/tasks/action_items.py index 964e2fdd..151f1ff0 100644 --- a/src/firetower/incidents/tasks/action_items.py +++ b/src/firetower/incidents/tasks/action_items.py @@ -92,6 +92,11 @@ def _remind_misfiled_action_item_creators(linear: dict) -> None: logger.error("Failed to find misfiled action items") return + logger.info( + "Found %d misfiled action items without a team reminder in Linear team %s", + len(issues), + team_id, + ) for issue in issues: issue_id = None try: @@ -127,6 +132,8 @@ def _remind_misfiled_action_item_creators(linear: dict) -> None: continue if not success: logger.error("Failed to post team reminder for action item %s", issue_id) + else: + logger.info("Posted team reminder for action item %s", issue_id) @datadog_log diff --git a/src/firetower/incidents/tests/test_action_item_team_reminders.py b/src/firetower/incidents/tests/test_action_item_team_reminders.py index c0228906..fb9fd2d9 100644 --- a/src/firetower/incidents/tests/test_action_item_team_reminders.py +++ b/src/firetower/incidents/tests/test_action_item_team_reminders.py @@ -46,6 +46,29 @@ def test_mentions_creator_and_explains_move(service): assert UUID(kwargs["comment_id"]).version == 4 +@pytest.mark.parametrize("issues", [[], [ISSUE]]) +def test_logs_candidate_count(service, caplog, issues): + service.get_misfiled_action_items.return_value = issues + + with caplog.at_level("INFO", logger="firetower.incidents.tasks.action_items"): + _remind_misfiled_action_item_creators(LINEAR) + + assert ( + f"Found {len(issues)} misfiled action items without a team reminder " + "in Linear team incident-team" + ) in caplog.text + + +@pytest.mark.parametrize("success", [True, False]) +def test_logs_posted_reminder_only_on_success(service, caplog, success): + service.create_comment.return_value = success + + with caplog.at_level("INFO", logger="firetower.incidents.tasks.action_items"): + _remind_misfiled_action_item_creators(LINEAR) + + assert ("Posted team reminder for action item issue-1" in caplog.text) is success + + def test_skips_issues_with_existing_reminder_on_later_runs(service): service.get_misfiled_action_items.side_effect = [[ISSUE], []] diff --git a/src/firetower/integrations/services/linear.py b/src/firetower/integrations/services/linear.py index 40df08c2..183f98dd 100644 --- a/src/firetower/integrations/services/linear.py +++ b/src/firetower/integrations/services/linear.py @@ -573,7 +573,10 @@ def get_misfiled_action_items( "team": {"id": {"eq": team_id}}, "number": {"gte": 2000}, "parent": {"team": {"id": {"eq": team_id}}}, - "comments": {"every": {"body": {"notContains": reminder_marker}}}, + "or": [ + {"comments": {"length": {"eq": 0}}}, + {"comments": {"every": {"body": {"notContains": reminder_marker}}}}, + ], } issues: list[dict[str, Any]] = [] cursor: str | None = None diff --git a/src/firetower/integrations/tests/test_linear_team_reminders.py b/src/firetower/integrations/tests/test_linear_team_reminders.py index d577755c..fc63302a 100644 --- a/src/firetower/integrations/tests/test_linear_team_reminders.py +++ b/src/firetower/integrations/tests/test_linear_team_reminders.py @@ -30,7 +30,10 @@ def test_filters_by_child_team_parent_team_number_and_existing_comments(service) "team": {"id": {"eq": "team-id"}}, "number": {"gte": 2000}, "parent": {"team": {"id": {"eq": "team-id"}}}, - "comments": {"every": {"body": {"notContains": "reminder-marker"}}}, + "or": [ + {"comments": {"length": {"eq": 0}}}, + {"comments": {"every": {"body": {"notContains": "reminder-marker"}}}}, + ], }, "after": None, } From 885de9805a619ebb4aefcdc8c5cd14d3536b9619 Mon Sep 17 00:00:00 2001 From: Spencer Murray Date: Fri, 23 Oct 2026 11:26:21 -0400 Subject: [PATCH 5/8] Process capped reminder batches so large backlogs can drain --- src/firetower/integrations/services/linear.py | 11 +++++-- .../tests/test_linear_team_reminders.py | 29 +++++++++++++++++-- 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/src/firetower/integrations/services/linear.py b/src/firetower/integrations/services/linear.py index 183f98dd..2d44506b 100644 --- a/src/firetower/integrations/services/linear.py +++ b/src/firetower/integrations/services/linear.py @@ -590,11 +590,16 @@ def get_misfiled_action_items( return issues next_cursor = page["pageInfo"]["endCursor"] if not next_cursor or next_cursor == cursor: - break + logger.error("Incomplete Linear misfiled action item pagination") + return None cursor = next_cursor - logger.error("Incomplete Linear misfiled action item pagination") - return None + logger.info( + "Reached Linear misfiled action item page limit; returning %d issues " + "and leaving the rest for the next run", + len(issues), + ) + return issues def create_comment( self, issue_id: str, body: str, *, comment_id: str | None = None diff --git a/src/firetower/integrations/tests/test_linear_team_reminders.py b/src/firetower/integrations/tests/test_linear_team_reminders.py index fc63302a..8bbd0a9f 100644 --- a/src/firetower/integrations/tests/test_linear_team_reminders.py +++ b/src/firetower/integrations/tests/test_linear_team_reminders.py @@ -80,17 +80,40 @@ def test_fails_closed_when_pagination_cannot_advance(service, cursor): assert service.get_misfiled_action_items("team-id", "marker") is None -def test_fails_closed_at_page_limit(service): +def test_returns_fetched_batch_at_page_limit(service): + issues = [{"id": f"issue-{i}"} for i in range(1250)] with patch.object( service, "_graphql", - side_effect=[page([], True, f"cursor-{i}") for i in range(25)], + side_effect=[ + page(issues[i * 50 : (i + 1) * 50], True, f"cursor-{i}") for i in range(25) + ], ) as graphql: - assert service.get_misfiled_action_items("team-id", "marker") is None + assert service.get_misfiled_action_items("team-id", "marker") == issues assert graphql.call_count == 25 +def test_starts_a_fresh_sweep_on_next_call(service): + first_batch = [{"id": f"issue-{i}"} for i in range(25)] + remaining = [{"id": "remaining-issue"}] + with patch.object( + service, + "_graphql", + side_effect=[ + *[ + page([issue], True, f"cursor-{i}") + for i, issue in enumerate(first_batch) + ], + page(remaining), + ], + ) as graphql: + assert service.get_misfiled_action_items("team-id", "marker") == first_batch + assert service.get_misfiled_action_items("team-id", "marker") == remaining + + assert graphql.call_args_list[25].args[1]["after"] is None + + @pytest.mark.parametrize("comment_id", [None, "stable-comment-id"]) def test_passes_optional_comment_id_without_retrying_mutation(service, comment_id): with patch.object( From b61f2402605be156d5fd46091e5c4370c864bf86 Mon Sep 17 00:00:00 2001 From: Spencer Murray Date: Fri, 23 Oct 2026 11:53:48 -0400 Subject: [PATCH 6/8] Skip team reminders when Linear identifier sync is disabled --- src/firetower/incidents/tasks/action_items.py | 3 +++ .../tests/test_action_item_team_reminders.py | 14 ++++++++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/firetower/incidents/tasks/action_items.py b/src/firetower/incidents/tasks/action_items.py index 151f1ff0..a28db87a 100644 --- a/src/firetower/incidents/tasks/action_items.py +++ b/src/firetower/incidents/tasks/action_items.py @@ -76,6 +76,9 @@ def _build_nag_tiers(linear: dict) -> dict[int, NagTier]: def _remind_misfiled_action_item_creators(linear: dict) -> None: + if not linear.get("SYNC_IDENTIFIERS", False): + return + team_id = linear.get("TEAM_ID") if not team_id: return diff --git a/src/firetower/incidents/tests/test_action_item_team_reminders.py b/src/firetower/incidents/tests/test_action_item_team_reminders.py index fb9fd2d9..00f59c28 100644 --- a/src/firetower/incidents/tests/test_action_item_team_reminders.py +++ b/src/firetower/incidents/tests/test_action_item_team_reminders.py @@ -9,7 +9,7 @@ send_action_item_reminder, ) -LINEAR = {"TEAM_ID": "incident-team"} +LINEAR = {"TEAM_ID": "incident-team", "SYNC_IDENTIFIERS": True} ISSUE = { "id": "issue-1", "identifier": "INC-2508", @@ -113,12 +113,22 @@ def test_handles_missing_creator(service): @pytest.mark.parametrize("config", [{}, {"TEAM_ID": ""}]) def test_skips_when_team_is_not_configured(service, config): - _remind_misfiled_action_item_creators(config) + _remind_misfiled_action_item_creators({**config, "SYNC_IDENTIFIERS": True}) service.get_misfiled_action_items.assert_not_called() service.create_comment.assert_not_called() +@pytest.mark.parametrize("config", [{}, {"SYNC_IDENTIFIERS": False}]) +def test_skips_when_identifier_sync_is_disabled(service, config): + with patch("firetower.incidents.tasks.action_items.LinearService") as client: + _remind_misfiled_action_item_creators({"TEAM_ID": "incident-team", **config}) + + client.assert_not_called() + service.get_misfiled_action_items.assert_not_called() + service.create_comment.assert_not_called() + + @pytest.mark.parametrize("failure", [None, RuntimeError("Linear unavailable")]) def test_does_not_post_when_lookup_fails(service, failure): if isinstance(failure, Exception): From cb8cd024b3dd919aed8fbd6420afe85303744acb Mon Sep 17 00:00:00 2001 From: Spencer Murray Date: Fri, 23 Oct 2026 14:45:02 -0400 Subject: [PATCH 7/8] Keep team reminder failures informational and clarify fetch failures --- src/firetower/incidents/tasks/action_items.py | 17 ++++++++++++----- .../tests/test_action_item_team_reminders.py | 14 ++++++++++---- src/firetower/integrations/services/linear.py | 2 +- .../tests/test_linear_team_reminders.py | 16 +++++++++++----- 4 files changed, 34 insertions(+), 15 deletions(-) diff --git a/src/firetower/incidents/tasks/action_items.py b/src/firetower/incidents/tasks/action_items.py index a28db87a..d0f6f19e 100644 --- a/src/firetower/incidents/tasks/action_items.py +++ b/src/firetower/incidents/tasks/action_items.py @@ -89,10 +89,15 @@ def _remind_misfiled_action_item_creators(linear: dict) -> None: team_id, MISFILED_ACTION_ITEM_REMINDER_MARKER ) except Exception: - logger.exception("Failed to find misfiled action items") + logger.info( + "Failed to fetch misfiled action items from Linear; skipping team reminders", + exc_info=True, + ) return if issues is None: - logger.error("Failed to find misfiled action items") + logger.info( + "Failed to fetch misfiled action items from Linear; skipping team reminders" + ) return logger.info( @@ -129,12 +134,14 @@ def _remind_misfiled_action_item_creators(linear: dict) -> None: ) success = service.create_comment(issue_id, comment, comment_id=comment_id) except Exception: - logger.exception( - "Failed to post team reminder for action item %s", issue_id + logger.info( + "Failed to post team reminder for action item %s", + issue_id, + exc_info=True, ) continue if not success: - logger.error("Failed to post team reminder for action item %s", issue_id) + logger.info("Failed to post team reminder for action item %s", issue_id) else: logger.info("Posted team reminder for action item %s", issue_id) diff --git a/src/firetower/incidents/tests/test_action_item_team_reminders.py b/src/firetower/incidents/tests/test_action_item_team_reminders.py index 00f59c28..262b5b00 100644 --- a/src/firetower/incidents/tests/test_action_item_team_reminders.py +++ b/src/firetower/incidents/tests/test_action_item_team_reminders.py @@ -130,28 +130,34 @@ def test_skips_when_identifier_sync_is_disabled(service, config): @pytest.mark.parametrize("failure", [None, RuntimeError("Linear unavailable")]) -def test_does_not_post_when_lookup_fails(service, failure): +def test_does_not_post_when_lookup_fails(service, failure, caplog): if isinstance(failure, Exception): service.get_misfiled_action_items.side_effect = failure else: service.get_misfiled_action_items.return_value = failure - _remind_misfiled_action_item_creators(LINEAR) + with caplog.at_level("INFO", logger="firetower.incidents.tasks.action_items"): + _remind_misfiled_action_item_creators(LINEAR) service.create_comment.assert_not_called() + assert "Failed to fetch misfiled action items from Linear" in caplog.text + assert all(record.levelname == "INFO" for record in caplog.records) @pytest.mark.parametrize("failure", [False, RuntimeError("Linear unavailable")]) -def test_continues_after_a_comment_failure(service, failure): +def test_continues_after_a_comment_failure(service, failure, caplog): service.get_misfiled_action_items.return_value = [ ISSUE, {**ISSUE, "id": "issue-2", "identifier": "INC-2509"}, ] service.create_comment.side_effect = [failure, True] - _remind_misfiled_action_item_creators(LINEAR) + with caplog.at_level("INFO", logger="firetower.incidents.tasks.action_items"): + _remind_misfiled_action_item_creators(LINEAR) assert service.create_comment.call_count == 2 + assert "Failed to post team reminder for action item issue-1" in caplog.text + assert all(record.levelname == "INFO" for record in caplog.records) @pytest.mark.parametrize( diff --git a/src/firetower/integrations/services/linear.py b/src/firetower/integrations/services/linear.py index 2d44506b..74ccf0f5 100644 --- a/src/firetower/integrations/services/linear.py +++ b/src/firetower/integrations/services/linear.py @@ -590,7 +590,7 @@ def get_misfiled_action_items( return issues next_cursor = page["pageInfo"]["endCursor"] if not next_cursor or next_cursor == cursor: - logger.error("Incomplete Linear misfiled action item pagination") + logger.info("Incomplete Linear misfiled action item pagination") return None cursor = next_cursor diff --git a/src/firetower/integrations/tests/test_linear_team_reminders.py b/src/firetower/integrations/tests/test_linear_team_reminders.py index 8bbd0a9f..be922e33 100644 --- a/src/firetower/integrations/tests/test_linear_team_reminders.py +++ b/src/firetower/integrations/tests/test_linear_team_reminders.py @@ -71,14 +71,20 @@ def test_does_not_return_partial_results_on_failure(service): @pytest.mark.parametrize("cursor", [None, "cursor-1"]) -def test_fails_closed_when_pagination_cannot_advance(service, cursor): - with patch.object( - service, - "_graphql", - side_effect=[page([], True, "cursor-1"), page([], True, cursor)], +def test_fails_closed_when_pagination_cannot_advance(service, cursor, caplog): + with ( + patch.object( + service, + "_graphql", + side_effect=[page([], True, "cursor-1"), page([], True, cursor)], + ), + caplog.at_level("INFO", logger="firetower.integrations.services.linear"), ): assert service.get_misfiled_action_items("team-id", "marker") is None + assert "Incomplete Linear misfiled action item pagination" in caplog.text + assert all(record.levelname == "INFO" for record in caplog.records) + def test_returns_fetched_batch_at_page_limit(service): issues = [{"id": f"issue-{i}"} for i in range(1250)] From 429c482d42e565faabd1c5ee8c35b59263f7e1ee Mon Sep 17 00:00:00 2001 From: Spencer Murray Date: Fri, 23 Oct 2026 15:09:23 -0400 Subject: [PATCH 8/8] Run SLO reminders before the misfiled action item sweep --- src/firetower/incidents/tasks/action_items.py | 6 ++++-- src/firetower/incidents/tests/test_tasks.py | 13 +++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/firetower/incidents/tasks/action_items.py b/src/firetower/incidents/tasks/action_items.py index d0f6f19e..08c24fcf 100644 --- a/src/firetower/incidents/tasks/action_items.py +++ b/src/firetower/incidents/tasks/action_items.py @@ -149,14 +149,14 @@ def _remind_misfiled_action_item_creators(linear: dict) -> None: @datadog_log def send_action_item_reminder() -> None: linear = settings.LINEAR or {} - _remind_misfiled_action_item_creators(linear) nag_tiers = _build_nag_tiers(linear) comments_by_priority: dict[int, str] = { priority: linear.get(tier.comment_setting_key, "") for priority, tier in nag_tiers.items() } if not any(comments_by_priority.values()): - logger.warning("No Linear nag comments configured, skipping job") + logger.warning("No Linear nag comments configured, skipping SLO reminders") + _remind_misfiled_action_item_creators(linear) return now = timezone.now() @@ -284,3 +284,5 @@ def _nag(action_item: ActionItem, incident: Incident) -> None: for action_item in eligible: _nag(action_item, incident) + + _remind_misfiled_action_item_creators(linear) diff --git a/src/firetower/incidents/tests/test_tasks.py b/src/firetower/incidents/tests/test_tasks.py index 6230db56..f9f53ed2 100644 --- a/src/firetower/incidents/tests/test_tasks.py +++ b/src/firetower/incidents/tests/test_tasks.py @@ -909,6 +909,19 @@ def _make_action_item(self, incident: Incident, **kwargs) -> ActionItem: defaults.update(kwargs) return ActionItem.objects.create(incident=incident, **defaults) + def test_sends_slo_nags_before_team_reminders( + self, mock_linear, mock_team_reminders + ): + incident = self._make_incident() + self._make_action_item(incident) + calls = MagicMock() + calls.attach_mock(mock_linear.create_comment, "slo_comment") + calls.attach_mock(mock_team_reminders, "team_reminders") + + send_action_item_reminder() + + assert [c[0] for c in calls.mock_calls] == ["slo_comment", "team_reminders"] + def test_processes_incident_in_age_window(self, mock_sync): incident = self._make_incident(days_old=45) send_action_item_reminder()