Skip to content

fix(replay-vision): select Slack workspace for alerts - #106575

Open
Indira-kumar wants to merge 5 commits into
PostHog:masterfrom
Indira-kumar:codex/fix-replay-vision-slack-workspace
Open

Indira-kumar wants to merge 5 commits into
PostHog:masterfrom
Indira-kumar:codex/fix-replay-vision-slack-workspace

Conversation

@Indira-kumar

@Indira-kumar Indira-kumar commented Sep 25, 2026 •

Copy link
Copy Markdown

Problem

Changes

  • Replay Vision alerts now pass the selected Slack workspace to the channel picker and notification payload.
  • Changing workspaces clears the selected channel, preventing an invalid workspace-channel pair.
  • Adds regression coverage for staging a notification from a second workspace.

Screenshots

Currently selected workspace A:
image
Switched to workspace B:
image
Selected channels from both workspaces:
image

How did you test this code?

  • Manual local verification:
    1. Created a Replay Vision scanner.
    2. Opened Alerts and created a new alert.
    3. Selected Slack in the Notify step.
    4. Switched between two Slack workspaces.
    5. Confirmed each workspace loaded its own channel.
    6. Saved and reopened the alert.
    7. Confirmed the selected workspace and channel remained attached.
  • Automated: hogli test products/replay_vision/frontend/replay_scanners/scannerAlertNotificationLogic.test.ts passed. The test verifies that workspace 2 is staged in the Slack notification payload.
  • Automated: pnpm --filter=@posthog/frontend typescript:check passed.
  • Automated: hogli ci:preflight --strict --against upstream/master reported zero strict failures.

Release status

  • No feature flag controls this change
  • This change is behind a feature flag and is not available to users
  • This change makes a previously flagged feature available to everyone

Automatic notifications

  • Publish to changelog?

Docs update

  • No documentation update is needed for this bug fix.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Codex, GPT-5

  • The branch was rebased onto upstream/master and its focused regression test was added to cover the selected workspace payload.
  • Skills invoked: /running-ci-preflight, /writing-pr-descriptions, /qa-frontend, /run-posthog, and /reviewing-with-coderabbit.
  • CodeRabbit CLI was installed but unauthenticated, so this draft opened without a local CodeRabbit pass.
  • A public open-PR search found no duplicate Replay Vision Slack-workspace fix.
  • The committed fixture data is invented and contains no customer material.

@trunk-io

trunk-io Bot commented Sep 25, 2026

Copy link
Copy Markdown

Merging to master in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

@coderabbitai

coderabbitai Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: PostHog/posthog/.coderabbit.yaml

Review profile: QUIET

Plan: Enterprise

Run ID: f73239fa-4ca3-442d-b45d-dd5cf4bd7d3c

📥 Commits

Reviewing files that changed from the base of the PR and between d39c387 and 90f540f.

📒 Files selected for processing (3)
  • products/replay_vision/frontend/replay_scanners/components/ScannerAlertNotifications.tsx
  • products/replay_vision/frontend/replay_scanners/scannerAlertUtils.test.ts
  • products/replay_vision/frontend/replay_scanners/scannerAlertUtils.ts

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.


📝 Walkthrough

Walkthrough

Scanner alerts now track the selected Slack workspace by integration ID. The component loads channels for the selected integration and passes workspace changes to the alert logic. The logic uses the resolved integration to validate and stage Slack notifications. A test covers default selection, switching workspaces, clearing the channel value, and staging a notification.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 90f54

The reviewed workspace-selection and destination behavior has no identified issue requiring a fix before merge.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 90f54

The editor now supports alerts in a second connected Slack workspace. The reviewed flow clears the old channel when workspaces change and saves the selected workspace with its channel. No introduced security issue was established, but server-side checks for those identifiers remain unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — An alert creator can now choose a second available Slack integration as the destination workspace. Effective exposure beyond the integrations offered by the editor cannot be determined without the server-side ownership checks.

Trust Boundaries and Controls

  • observed — Changing workspaces clears the selected channel, and integration-keyed Slack logic loads channels using the newly selected integration ID. The reviewed client path therefore preserves the workspace-channel association during a normal switch.

Hardening Proposals

  • proposed — Confirm that the channel and destination endpoints enforce project ownership of the supplied integration and validate the workspace-channel association independently of the editor.
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description covers the problem, user-visible changes, screenshots, tests, release status, documentation, and agent context. The agent context does not include a session link or report patch-covera…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

@Indira-kumar
Indira-kumar marked this pull request as ready for review September 25, 2026 20:40
@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested review from a team, TueHaulund, arnohillen, fasyy612 and ksvat and removed request for a team September 25, 2026 20:41
actions.addPendingNotification({
type: VISION_ALERT_NOTIFICATION_TYPE_SLACK,
slackWorkspaceId: values.firstSlackIntegration.id,
slackWorkspaceId: values.selectedSlackIntegration.id,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Agent-drafted, reviewed by Tue.

Small follow-on: groupVisionAlertDestinations in scannerAlertUtils.ts groups Slack destinations by channel only. With a workspace picker, two workspaces can share a channel ID (Slack Connect), so they'd show up as one row. Adding the workspace to that key should fix it.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed both points:

  • Slack destinations are now grouped by workspace ID and channel ID.
  • Pending destinations now display the Slack workspace and channel.
image

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While testing, I noticed that saved destinations still display only “Slack” after reopening the alert. I’ve tracked that separately in #107955 so this PR can remain focused.

@arnohillen

Copy link
Copy Markdown
Contributor

ScannerAlertNotifications.tsx:20: the pending row shows only #channel. Show the workspace name as InlineAlertNotifications.tsx:105 does.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Can't change Slack Workspace while creating an alert

3 participants