feat(signals): Notify suggested reviewers in Slack#58774
Merged
Conversation
Adds a per-user Slack notification config for the signals product. When a report transitions to READY (a new inbox item lands), the summary workflow now dispatches a Slack message to every suggested reviewer that has wired up an integration + channel in their user autonomy config. The optional `slack_notification_min_priority` filter mirrors the autostart-priority threshold: when set, only reports at or above that priority notify; reports without a priority judgment always notify so we never silently swallow them. The dispatch reuses the existing SlackIntegration channel-posting path used by Insight Alerts and evaluation reports, including the `channel_id|#channel-name` target convention. Generated-By: PostHog Code Task-Id: 046d7425-f0b4-4ab4-97f8-cce0627927f0
Contributor
Migration SQL ChangesHey 👋, we've detected some migrations on this PR. Here's the SQL output for each migration, make sure they make sense:
|
Contributor
🔍 Migration Risk AnalysisWe've analyzed your migrations for potential risks. Summary: 1 Safe | 0 Needs Review | 0 Blocked ✅ SafeBrief or no lock, backwards compatible 📚 How to Deploy These Changes SafelyAddField: This operation acquires a brief lock but doesn't rewrite the table. Deployment uses lock timeouts with automatic retries, so lock contention will cause retries rather than connection pile-up. Last updated: 2026-05-22 08:49 UTC (2c6b2cb) |
Contributor
Query snapshots: Backend query snapshots updatedChanges: 1 snapshots (1 modified, 0 added, 0 deleted) What this means:
Next steps:
|
Contributor
|
🎭 Playwright didn't run on this PR — your changes touch code that could affect E2E behavior, but Playwright is opt-in via label now to keep CI cost down. Add the Most PRs don't need this. Real regressions still get caught on master and fix-forward. |
- mypy: switch `slack_notification_integration_id` to a typed `IntegerField` instead of an inferred `PrimaryKeyRelatedField` so the read-only field has a clear annotation. - semgrep idor-lookup-without-team: replace the `team_id__in=` lookup on `Integration` with a fetch + per-team-id membership check, which the rule recognises as a proper team-scoping pattern. - migration risk: drop `atomic=False` from migration 0019. The migration only adds nullable fields, so a brief lock is fine and atomic=True restores transaction rollback safety. - temporal module integrity test: add the new `dispatch_inbox_slack_notifications_activity` to the expected activities list so the signals worker count assertion passes. Generated-By: PostHog Code Task-Id: 046d7425-f0b4-4ab4-97f8-cce0627927f0
…urn page Add `slack-integration` to the `/account-connected/<kind>` landing-page kinds so PostHog Cloud can route Slack OAuth completions to a `posthog-code://slack-integration?...` deep link. Pairs with the new in-app Slack connect flow in PostHog/code: users can connect a Slack workspace to PostHog without leaving the PostHog Code app for any settings page. `integration_id` is forwarded alongside the existing `installation_id` so the desktop handler can act on whichever its provider uses. Generated-By: PostHog Code Task-Id: 046d7425-f0b4-4ab4-97f8-cce0627927f0
Twixes
added a commit
to PostHog/code
that referenced
this pull request
May 18, 2026
Adds an in-app Slack connect flow so users no longer have to leave PostHog Code for the PostHog web settings page just to wire up notifications. Flow: clicking "Connect Slack workspace" in the inbox source settings opens the browser to PostHog Cloud's Slack OAuth authorize endpoint with `next=/account-connected/slack-integration?...`. PostHog Cloud completes OAuth, creates the team-level Slack `Integration`, and redirects to a landing page that fires a `posthog-code://slack-integration?...` deep link back into the app. The new `SlackIntegrationService` (main process) handles the deep link, emits a callback event, and the renderer hook refreshes the integrations list. Mirrors the existing `GitHubIntegrationService` so each provider's deep link handler stays isolated. Pairs with backend changes in PostHog/posthog#58774 that teach the `/account-connected/<kind>` page about the new `slack-integration` kind. Generated-By: PostHog Code Task-Id: 046d7425-f0b4-4ab4-97f8-cce0627927f0
Contributor
|
Size Change: -2.85 MB (-2.39%) Total Size: 116 MB 📦 View Changed
ℹ️ View Unchanged
|
Filter the cached channel list server-side so clients can search large workspaces without loading every channel at once.
Align the autonomy config integration guard with the standard team visibility helper instead of a raw organization membership query.
…mail lookup Frame channel posts with reviewer @mentions via lookup_slack_user_id_by_email, PostHog Code deep links, and first-line summary excerpts. Add GitHub integration callback route and Slack channel search API follow-ups for notification settings.
Contributor
Query snapshots: Backend query snapshots updatedChanges: 1 snapshots (1 modified, 0 added, 0 deleted) What this means:
Next steps:
|
Contributor
|
⏭️ Skipped snapshot commit because branch advanced to The new commit will trigger its own snapshot update workflow. If you expected this workflow to succeed: This can happen due to concurrent commits. To get a fresh workflow run, either:
|
Contributor
MCP UI Apps size report
|
Twixes
changed the title
GitHub integration callback and finish_setup work moved to fix/github-integration-finish-setup.
Contributor
|
⏭️ Skipped snapshot commit because branch advanced to The new commit will trigger its own snapshot update workflow. If you expected this workflow to succeed: This can happen due to concurrent commits. To get a fresh workflow run, either:
|
Contributor
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
products/signals/frontend/generated/api.zod.ts:101-111
The generated `UsersSignalAutonomyCreateBody` Zod schema is missing `slack_notification_integration_id`, even though `SignalUserAutonomyConfigCreateSerializer` defines it as an `IntegerField`. Without this field in the schema, any frontend client using Zod validation to build the POST body will be unable to type-check or validate the integration ID — the key field that actually enables Slack notifications.
```suggestion
export const UsersSignalAutonomyCreateBody = /* @__PURE__ */ zod.object({
autostart_priority: zod
.union([
zod
.enum(['P0', 'P1', 'P2', 'P3', 'P4'])
.describe('\* `P0` - P0\n\* `P1` - P1\n\* `P2` - P2\n\* `P3` - P3\n\* `P4` - P4'),
zod.enum(['']),
zod.null(),
])
.optional(),
slack_notification_integration_id: zod
.number()
.int()
.nullish()
.describe(
'Primary key of a Slack Integration row in one of the caller\'s teams. Pair with `slack_notification_channel` to enable notifications; pass null on either to disable them.'
),
slack_notification_channel: zod
```
### Issue 2 of 2
posthog/api/test/__snapshots__/test_api_docs.ambr:2-6
**OpenAPI enum naming warning accepted rather than fixed**
The `ENUM_NAME_OVERRIDES` addition maps `"AutonomyPriorityEnum"` to `products.signals.backend.models.AutonomyPriority`, but the warning surfacing is `SlackNotificationMinPriorityEnum` — a different auto-generated name for the same choices set derived from the `slack_notification_min_priority` field name. The override didn't suppress the warning, so the snapshot was updated to expect it instead of resolving it. Adding `"SlackNotificationMinPriorityEnum": "products.signals.backend.models.AutonomyPriority"` to `ENUM_NAME_OVERRIDES` in `posthog/settings/web.py` should eliminate the warning without needing to carry it in the snapshot.
Reviews (1): Last reviewed commit: "chore: update OpenAPI generated types" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
101
to
+111
| @@ -106,4 +108,23 @@ export const UsersSignalAutonomyCreateBody = /* @__PURE__ */ zod.object({ | |||
| zod.null(), | |||
| ]) | |||
| .optional(), | |||
| slack_notification_channel: zod | |||
Contributor
There was a problem hiding this comment.
UsersSignalAutonomyCreateBody Zod schema is missing slack_notification_integration_id, even though SignalUserAutonomyConfigCreateSerializer defines it as an IntegerField. Without this field in the schema, any frontend client using Zod validation to build the POST body will be unable to type-check or validate the integration ID — the key field that actually enables Slack notifications.
Suggested change
| export const UsersSignalAutonomyCreateBody = /* @__PURE__ */ zod.object({ | |
| autostart_priority: zod | |
| .union([ | |
| zod | |
| .enum(['P0', 'P1', 'P2', 'P3', 'P4']) | |
| .describe('\* `P0` - P0\n\* `P1` - P1\n\* `P2` - P2\n\* `P3` - P3\n\* `P4` - P4'), | |
| zod.enum(['']), | |
| zod.null(), | |
| ]) | |
| .optional(), | |
| slack_notification_integration_id: zod | |
| .number() | |
| .int() | |
| .nullish() | |
| .describe( | |
| 'Primary key of a Slack Integration row in one of the caller\'s teams. Pair with `slack_notification_channel` to enable notifications; pass null on either to disable them.' | |
| ), | |
| slack_notification_channel: zod |
Prompt To Fix With AI
This is a comment left during a code review.
Path: products/signals/frontend/generated/api.zod.ts
Line: 101-111
Comment:
The generated `UsersSignalAutonomyCreateBody` Zod schema is missing `slack_notification_integration_id`, even though `SignalUserAutonomyConfigCreateSerializer` defines it as an `IntegerField`. Without this field in the schema, any frontend client using Zod validation to build the POST body will be unable to type-check or validate the integration ID — the key field that actually enables Slack notifications.
```suggestion
export const UsersSignalAutonomyCreateBody = /* @__PURE__ */ zod.object({
autostart_priority: zod
.union([
zod
.enum(['P0', 'P1', 'P2', 'P3', 'P4'])
.describe('\* `P0` - P0\n\* `P1` - P1\n\* `P2` - P2\n\* `P3` - P3\n\* `P4` - P4'),
zod.enum(['']),
zod.null(),
])
.optional(),
slack_notification_integration_id: zod
.number()
.int()
.nullish()
.describe(
'Primary key of a Slack Integration row in one of the caller\'s teams. Pair with `slack_notification_channel` to enable notifications; pass null on either to disable them.'
),
slack_notification_channel: zod
```
How can I resolve this? If you propose a fix, please make it concise.
Co-authored-by: Michael Matloka <dev@twixes.com>
Co-authored-by: Michael Matloka <dev@twixes.com>
PR overviewSlack inbox notifications for suggested reviewersThis PR adds Signals Slack notification configuration and dispatch, plus Slack channel search/pagination and generated client updates. I checked the new autonomy config authorization, team scoping on selected integrations, notification dispatch path, Slack channel filtering/cache behavior, and the generated/frontend changes, and did not find a new security issue in the diff. Security review
Risk: 2/10 |
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
| return [] | ||
|
|
||
| return list( | ||
| SignalUserAutonomyConfig.objects.filter(user_id__in=user_ids) |
There was a problem hiding this comment.
Medium: Missing project access check for notification recipients
A user who configured Slack notifications while they had project access can keep receiving future report titles and summaries after being removed from that project, as long as their GitHub login is still resolved in the organization. Filter targets by current project membership before posting.
Suggested change
| SignalUserAutonomyConfig.objects.filter(user_id__in=user_ids) | |
| SignalUserAutonomyConfig.objects.filter(user_id__in=user_ids, user__teams__id=report.team_id) |
Co-authored-by: Michael Matloka <dev@twixes.com>
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
Comment on lines
+1000
to
+1001
| current_team_id = request.user.current_team_id | ||
| if current_team_id is None or current_team_id not in UserPermissions(user).team_ids_visible_for_user: |
Contributor
There was a problem hiding this comment.
Security/Logic bug: Using request.user.current_team_id to validate against the target user's permissions is incorrect. When a staff member updates another user's autonomy config, this checks if request.user.current_team_id is in the target user's visible teams, which will fail if the staff member's current team differs from the target user's teams.
Impact: Staff members cannot update other users' Slack notification settings even though they should be able to.
Fix:
if integration_id is not None:
user_team_ids = UserPermissions(user).team_ids_visible_for_user
candidate = Integration.objects.filter(
pk=integration_id,
kind="slack",
team_id__in=user_team_ids,
).first()
if candidate is None:
raise serializers.ValidationError(
{"slack_notification_integration_id": "Unknown Slack integration for this user."}
)
integration = candidateThis ensures the integration's team is one that the target user has access to, regardless of who is making the request.
Suggested change
| current_team_id = request.user.current_team_id | |
| if current_team_id is None or current_team_id not in UserPermissions(user).team_ids_visible_for_user: | |
| if integration_id is not None: | |
| user_team_ids = UserPermissions(user).team_ids_visible_for_user | |
| candidate = Integration.objects.filter( | |
| pk=integration_id, | |
| kind="slack", | |
| team_id__in=user_team_ids, | |
| ).first() | |
| if candidate is None: | |
| raise serializers.ValidationError( | |
| {"slack_notification_integration_id": "Unknown Slack integration for this user."} | |
| ) | |
| integration = candidate | |
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
Co-authored-by: Michael Matloka <dev@twixes.com>
andrewm4894
reviewed
May 21, 2026
| return email or "Unknown user" | ||
|
|
||
|
|
||
| def lookup_slack_user_id_by_email(slack: SlackIntegration, email: str) -> str | None: |
Member
There was a problem hiding this comment.
👋 just double checkign this one around caching:
This is called once per recipient per report with no caching, and users.lookupByEmail is Slack Tier 3 (~50/min per workspace) — a burst of reports with several configured reviewers each can throttle the workspace's lookups, and we'll repeat the same lookup for the same person across every report.
An earlier commit (f3cdfc2f9f3) imported lookup_slack_user_id_by_email from products/slack_app/backend/api.py, which already layers a Django cache on top of the SlackUserProfileCache DB table — so steady-state Slack traffic from this feature scales with new reviewer emails, not report volume × reviewer count. Was there a reason for inlining a copy here? Re-using that helper looks like it would address this with no extra plumbing.
andrewm4894
approved these changes
May 21, 2026
Renumber slack notification migration to 0020 after master 0019.
Contributor
ClickHouse migration SQL per cloud environmentNo ClickHouse migrations changed in this PR. |
Member
Author
Twixes
added a commit
to PostHog/code
that referenced
this pull request
May 22, 2026
## Problem Signals inbox items previously surfaced only in-app. Users wanting a heads-up when a new item lands and they're a suggested reviewer had no way to opt in. Even after the initial Slack notifications work landed, picking a channel still required a detour through PostHog Web — users had to leave PostHog Code to install the Slack integration, then come back. That's too much friction for what should be a one-time setup. ## Changes  Adding Slack notifications under **Inbox** source settings. What it includes: - Slack workspace picker (shown only when more than one Slack `Integration` is connected; otherwise the single workspace is implied). - Notification channel picker, populated from `/api/environments/{teamId}/integrations/{id}/channels/` - same endpoint as Insight Alerts. Includes an explicit "Off" option. - Minimum priority filter (also includes an "All priorities" option). Also, in-app Slack connect flow so users no longer have to leave PostHog Code for PostHog Web. - `SlackIntegrationService` in the main process. Mirrors the existing `GitHubIntegrationService` but registers a separate `slack-integration` deep-link key so each provider's handler stays isolated. - `slackIntegration` tRPC router. - The empty state in `SignalSlackNotificationsSettings` shows a "Connect Slack workspace" button that runs connection without redirecting to the Integrations page of Cloud settings. Pairs with the backend changes in PostHog/posthog#58774. ## How did you test this? Ran this locally. ## Publish to changelog? no --- _Created with_ [_PostHog Code_](https://posthog.com/code?ref=pr)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Signals inbox items only surfaced in-app. When a new report landed and a user was a suggested reviewer, they had no way to find out until they opened the inbox. We want to pull them in.
Changes
Adding Slack notifications for new inbox items as part of the existing per-user signals autonomy config:
SignalUserAutonomyConfig:slack_notification_integration(FK to team-scoped SlackIntegration),slack_notification_channel(the samechannel_id|#channel-nametarget string Insight Alerts uses), andslack_notification_min_priority(P0–P4, nullable).slack_inbox_notifications.pydispatcher resolves thesuggested_reviewersartefact to PostHog user and sends a message in Slack when applicable. Runsdispatch_inbox_slack_notifications_activity.Also switched POST
/api/users/:id/signal_autonomy/switched to partial updates so editing autostart priority and Slack settings from different surfaces doesn't wipe each other.UI changes for the channel/priority picker and the in-app Slack connect flow live in PostHog/code#2174.
How did you test this code?
Added
products/signals/backend/test/test_slack_inbox_notifications.py. And ran locally.Publish to changelog?
no
Docs update
skip-inkeep-docs
Created with PostHog Code