feat: Linear action items with parent-child issue model#162
feat: Linear action items with parent-child issue model#162spalmurray wants to merge 18 commits intomainfrom
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| LINEAR_API_URL = "https://api.linear.app/graphql" | ||
| LINEAR_TOKEN_URL = "https://api.linear.app/oauth/token" | ||
|
|
||
| LINEAR_STATE_TYPE_MAP = { |
There was a problem hiding this comment.
Note that these are exhaustive. Custom statuses must map to one of these 6 under the hood.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| incident.action_items_last_synced_at = timezone.now() | ||
| incident.save(update_fields=["action_items_last_synced_at"]) | ||
| return stats | ||
|
|
||
| related = linear_service.get_related_issues(parent_id) | ||
| if related is None: | ||
| error_msg = f"Failed to fetch related issues for incident {incident.id}" | ||
| logger.error(error_msg) | ||
| stats.errors.append(error_msg) | ||
| incident.action_items_last_synced_at = timezone.now() | ||
| incident.save(update_fields=["action_items_last_synced_at"]) |
There was a problem hiding this comment.
API errors update last_synced_at, causing throttle to block retries
When get_child_issues or get_related_issues returns None (API failure), the function still sets action_items_last_synced_at = timezone.now() before returning. Subsequent non-forced sync attempts within ACTION_ITEM_SYNC_THROTTLE_SECONDS will then be skipped, even though the previous attempt completely failed. Transient Linear API outages thus lock users out of action item updates for the entire throttle window with no successful sync having occurred.
Verification
Read sync_action_items_from_linear at lines 212-310. Both error branches at lines 236-242 and 244-251 call incident.save(update_fields=['action_items_last_synced_at']) after timestamping. The throttle check at lines 221-230 compares this timestamp without distinguishing successful from failed syncs. Confirmed that get_child_issues/get_related_issues return None on failure (linear.py lines 297-401).
Also found at 1 additional location
src/firetower/incidents/services.py:200-209
Identified by Warden find-bugs · 3RP-2RQ
There was a problem hiding this comment.
Fix attempt detected (commit 57c6a1c)
The commit intentionally sets action_items_last_synced_at on API failure (lines 240, 249) to prevent unlimited retries, but this directly causes the reported bug where transient API failures lock users out of retries for the entire throttle window.
The original issue appears unresolved. Please review and try again.
Evaluated by Warden
| except Exception: | ||
| logger.exception( | ||
| f"Failed to update Linear issue title for incident {incident.id}" | ||
| ) |
There was a problem hiding this comment.
Identical Linear title-sync blocks duplicated across hooks
Low Severity
on_title_changed and on_visibility_changed contain identical code blocks that create a new LinearService, call update_issue with _linear_issue_title, and handle exceptions with the same log message. This duplicated logic could be extracted into a shared helper, reducing maintenance burden and risk of the two copies diverging.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit b9abb82. Configure here.
spalmurray
force-pushed
the
spalmurray/linear-action-items
branch
from
b9abb82 to
57c6a1c
Compare
| return issue | ||
| linear_service.create_issue("Placeholder", "", team_id, project_id) | ||
|
|
||
| return None |
There was a problem hiding this comment.
Bug: The _claim_linear_issue function leaks placeholder issues in Linear if it fails to claim a specific issue identifier, creating junk data when SYNC_IDENTIFIERS is enabled.
Severity: MEDIUM
Suggested Fix
Implement a cleanup mechanism within the _claim_linear_issue function. If the function fails to claim the desired identifier after all attempts, it should iterate through any created placeholder issues and delete them from Linear before returning.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/firetower/incidents/hooks.py#L598-L601
Potential issue: The `_claim_linear_issue` function attempts to reserve a specific
Linear issue identifier by creating placeholder issues. If this process fails to acquire
the desired identifier after `MAX_CLAIM_ATTEMPTS`, the created placeholder issues are
not cleaned up from Linear. This occurs when the Firetower incident counter is out of
sync with Linear's counter. As a result, unused "Placeholder" issues can be permanently
leaked in the Linear project for each failed attempt, requiring manual cleanup. This
behavior is active only when the `SYNC_IDENTIFIERS` feature flag is enabled.
| _linear_service: LinearService | None = None | ||
|
|
||
|
|
||
| def _get_linear_service() -> LinearService: | ||
| global _linear_service # noqa: PLW0603 | ||
| if _linear_service is None: | ||
| _linear_service = LinearService() | ||
| return _linear_service |
There was a problem hiding this comment.
Bug: The LinearService caches workflow states indefinitely. If states change in Linear, the stale cache causes API calls using those states to fail silently.
Severity: MEDIUM
Suggested Fix
Introduce a cache invalidation strategy. This could be a time-to-live (TTL) on the _workflow_states_cache to force periodic refreshes. Alternatively, remove the instance-level cache in LinearService and cache the states with a TTL in a shared cache like Redis or Django's cache framework, which is better suited for multi-process environments.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/firetower/incidents/services.py#L24-L31
Potential issue: The `_get_linear_service` function caches a single `LinearService`
instance for the lifetime of the application process. This service, in turn, caches
Linear workflow states via `get_workflow_states` upon first use and never invalidates
this cache. If workflow states are modified in Linear (e.g., a state is renamed or
deleted), the service will continue to use stale state IDs. This causes functions like
`_update_parent_issue_status` to fail silently when trying to update an issue with an
invalid state ID, preventing features like auto-completing parent issues from working
correctly.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 57c6a1c. Configure here.
| return issue | ||
| linear_service.create_issue("Placeholder", "", team_id, project_id) | ||
|
|
||
| return None |
There was a problem hiding this comment.
Off-by-one in claim loop misses last created issue
Medium Severity
_claim_linear_issue never calls get_issue after the final create_issue. If the last placeholder created in the loop is the one matching the desired identifier, the function returns None instead of returning the successfully claimed issue. This means the function handles gaps of at most MAX_CLAIM_ATTEMPTS - 1 (4) instead of MAX_CLAIM_ATTEMPTS (5). When the gap equals exactly 5, incident creation silently fails to link a Linear issue, and the ExternalLink is deleted as cleanup.
Reviewed by Cursor Bugbot for commit 57c6a1c. Configure here.
| identifier = incident.incident_number | ||
|
|
||
| for _ in range(MAX_CLAIM_ATTEMPTS): | ||
| issue = linear_service.get_issue(identifier) | ||
| if issue: | ||
| return issue | ||
| linear_service.create_issue("Placeholder", "", team_id, project_id) | ||
|
|
||
| return None |
There was a problem hiding this comment.
Bug: When SYNC_IDENTIFIERS is enabled without a matching Linear team key, the issue claiming logic creates multiple junk placeholder issues in Linear before failing.
Severity: MEDIUM
Suggested Fix
The placeholder creation loop should be removed as it's based on a flawed assumption. Instead, the function should attempt to get_issue once. If it fails, it should immediately fail with a clear error message explaining that SYNC_IDENTIFIERS requires a pre-existing Linear issue with a matching identifier or a matching team key. This prevents creating junk issues and provides better user feedback.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/firetower/incidents/hooks.py#L593-L601
Potential issue: When `SYNC_IDENTIFIERS` is enabled, the `_claim_linear_issue` function
attempts to claim a Linear issue with an identifier matching the incident number (e.g.,
`INC-2000`). If no such issue exists, it enters a loop to create placeholder issues.
However, this logic is flawed. If the Linear team's key is different (e.g., `ENG`),
creating placeholders will generate identifiers like `ENG-201`, `ENG-202`, etc., which
will never match the target. This results in creating up to `MAX_CLAIM_ATTEMPTS` (5)
junk issues in Linear before the operation fails. Additionally, the loop has an
off-by-one error: it does not re-check for the issue after creating the final
placeholder, so even if the fifth placeholder had succeeded, the function would still
fail.
1 participant
Adds Linear action item tracking to incidents using a parent-child issue model. When an incident is created, a parent Linear issue is created automatically. Action items are discovered by querying child issues and related issues of that parent.