feat(integrations): connect slack in a new tab from every slack picker - #106427
Conversation
|
😎 This pull request was merged. |
🤖 CI report✅ Trunk lane — non-backend laneThis PR is assigned to the non-backend lane. It does not run backend Python tests and may merge in parallel with PRs in other lanes.
|
| Function | Location | Complexity | Limit |
|---|---|---|---|
AlertNotificationDestinationEditor |
products/alerts/frontend/components/AlertNotificationDestinationEditor.tsx:224 |
19 | 10 |
✅ Duplication (Python) — clean
New Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.
✅ Duplication (TypeScript) — clean
New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.
⚠️ Comment density — 6% of added code lines are comments (17 of 304)
This section warns when comments are more than 3% of the code lines a PR adds, and alerts above 6%. Before agent-assisted PRs, the typical share was about 2%. Only full-line comments count. Docstrings, generated files, snapshots, migrations, and workflow files are left out.
Comments that restate the code, record how the change came about, or narrate the next line add noise for the next reader. Keep the comments that explain a reason the code cannot show, and remove the rest. See .agents/skills/writing-code-comments/SKILL.md for the house rules.
Files with the most added comment lines:
| File | Comment lines | Added lines |
|---|---|---|
frontend/src/lib/integrations/slackConnectLogic.ts |
13 | 133 |
frontend/src/lib/integrations/SlackIntegrationHelpers.tsx |
3 | 69 |
frontend/src/lib/integrations/slackChannel.ts |
1 | 4 |
This check does not block merging. It updates on every push and clears when the share drops.
⚠️ Bundle size — 🔺 +1.6 KiB (+0.0%)
Uncompressed size of every built .js bundle, compared against the base branch.
Total: 68.82 MiB · 🔺 +1.6 KiB (+0.0%)
| File | Size | Δ vs base |
|---|---|---|
render-query/src/render-query/render-query.js |
20.23 MiB | 🔺 +1.5 KiB (+0.0%) |
Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report
✅ Eager graph — within budget
How much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy import() / React.lazy chunks are not counted.
| Root | Eager (shipped) | Δ vs base | Budget |
|---|---|---|---|
entry (logged-out pages, app bootstrap)src/index.tsx |
1.57 MiB · 22 files | no change | █████████░ 85.1% of 1.84 MiB |
logged-out boot: index + App + bootApp (preloaded by every page, including /login)src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts |
3.57 MiB · 628 files | no change | █████████░ 88.6% of 4.03 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
7.27 MiB · 2,301 files | no change | █████████░ 87.2% of 8.34 MiB |
🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/layout/navigation-3000/navigationLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/scenes/dashboard/dashboardLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/lemon-ui/LemonMarkdown/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/RichContentEditor/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/CodeSnippet/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/taxonomy/core-filter-definitions-by-group.json stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/scenes/session-recordings/player/sessionRecordingPlayerLogic.ts stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
Largest files eagerly shipped from src/index.tsx
| Size | File |
|---|---|
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 24.6 KiB | ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js |
| 6.3 KiB | ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js |
| 4.5 KiB | ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js |
| 3.9 KiB | ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js |
| 1.4 KiB | ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js |
| 1.3 KiB | src/index.tsx |
| 1.3 KiB | src/RootErrorBoundary.tsx |
| 912 B | ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js |
| 854 B | src/scenes/ChunkLoadErrorBoundary.tsx |
Largest files eagerly shipped from src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
| Size | File |
|---|---|
| 301.8 KiB | ../node_modules/.pnpm/posthog-js@1.434.13_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 267.6 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 100.4 KiB | src/lib/api.ts |
| 84.9 KiB | src/products.tsx |
| 69.1 KiB | src/lib/lemon-ui/icons/icons.tsx |
| 63.9 KiB | src/lib/utils/eventUsageLogic.ts |
| 38.7 KiB | ../node_modules/.pnpm/@dnd-kit+core@6.0.8_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@dnd-kit/core/dist/core.esm.js |
| 33.9 KiB | ../node_modules/.pnpm/kea@4.0.0-pre.6_patch_hash=139b8d1f1304f9d9da452a9a1244c94ea679dbcb85687d8999563146879fb6f5_react@18.3.1/node_modules/kea/lib/index.cjs.js |
| 28.3 KiB | src/scenes/scenes.ts |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 301.8 KiB | ../node_modules/.pnpm/posthog-js@1.434.13_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 271.7 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 267.6 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 153.7 KiB | ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 100.4 KiB | src/lib/api.ts |
| 98.5 KiB | ../packages/quill/packages/quill/dist/index.js |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 90.6 KiB | ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js |
| 84.9 KiB | src/products.tsx |
Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479
✅ Toolbar bundle — eager 2.37 MiB within budget
What the toolbar ships to customer pages, measured from the esbuild output (minified, post-tree-shake). The eager set is the entry plus everything statically imported from it — fetched before any feature runs; deferred chunks load lazily. The eager guardrail is 5.72 MiB. Each output file must also stay below 10 MB, where CloudFront stops compressing it. The module boundary is enforced separately by check-toolbar-graph.
| Metric | Size | Δ vs base | Budget |
|---|---|---|---|
| Eager (shipped) entry + static imports |
2.37 MiB · 19 files | no change | ████░░░░░░ 41.4% of 5.72 MiB |
| Deferred (lazy) | 2.10 MiB · 44 files | no change | n/a — loads on demand |
Loader dist/toolbar.js |
1.2 KiB | no change | █░░░░░░░░░ 6.0% of 19.5 KiB |
Largest eagerly-shipped chunks
| Size | File |
|---|---|
| 790.2 KiB | dist/toolbar/toolbar-app-ZNXS4H6W.css |
| 650.4 KiB | dist/toolbar/chunk-chunk-DPTOFHTZ.js |
| 483.6 KiB | dist/toolbar/chunk-chunk-DPLA2GAY.js |
| 138.3 KiB | dist/toolbar/chunk-chunk-2EHPTPPA.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-FDH2IBXT.js |
| 75.2 KiB | dist/toolbar/toolbar-app-FOEGQ3V3.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-TSAL54PB.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-U5CQIMDB.js |
| 21.0 KiB | dist/toolbar/chunk-chunk-OYNHVBHM.js |
| 6.8 KiB | dist/toolbar/chunk-chunk-DV7IWQNF.js |
Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile
✅ Dist folder size — 🔺 +53.3 KiB (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 943.52 MiB · 🔺 +53.3 KiB (+0.0%)
⚠️ Playwright — 1 flaky
🎭 Playwright report · View test results →
- Save view (chromium)
These issues are not necessarily caused by your changes.
Annoyed by this section? Help fix flakies and failures and it will go green!
|
[Medium risk] Adds Slack connection flow from integration picker UI. This PR should not merge until workspace detection cannot select an unrelated or pre-existing Slack integration, and the repository null-result requirement is met. Reviews (1) · Last reviewed commit: "feat(integrations): connect Slack in a n..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
🦔 Hogbox preview · ✅ ready▶ Open the preview
commit |
👀 Auto-assigned reviewersThese soft owners were skipped because they only have minor changes here. Nothing blocks merge, so self-assign if you'd like a look:
Soft owners come from each directory's |
![parameterai[bot]](https://avatars.githubusercontent.com/in/2665233?s=60&v=4){kind=small}
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Not approved — this change needs a human reviewer.
Re-add the stamphog label to request another review once you have addressed this.
Greptile's review flagged two unresolved P1 correctness issues (a race can attribute a newly-appeared Slack workspace to the wrong connection attempt, and a failed initial load can cause an existing workspace to be misidentified as the new one) plus a P2 null/undefined convention issue, and its discussion comment explicitly says the PR should not merge until these are fixed — none of that has been addressed.
- 👍 on the PR from greptile-apps[bot].
- Unresolved Greptile P1: unrelated/newest workspace can be selected during the Slack connect wait, misconfiguring comments/alerts for the wrong workspace.
- Unresolved Greptile P1: a failed initial integrations load lets a pre-existing workspace be mistaken for the new OAuth connection.
- Unresolved Greptile P2: slackChannelName returns undefined instead of the repo's null convention for an absent result.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 169L, 5F substantive, 230L/6F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1c-medium (230L, 6F, two-areas, feat) |
| stamphog 2.1.0 | .stamphog/policy.yml @ 6b0b3d6 · reviewed head 6b0b3d6 |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Important Review skippedWe couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe Slack banner now tracks connection attempts, polls for newly added Slack integrations, and reports the connected integration ID through a callback. Slack destination editors pass their integration-change callbacks to the banner. A new helper extracts a channel display name from picker values. Priority: ⬇️ Low Merge Risk: 🔵 Low · up to The Slack retry control remains usable, but screen-reader users hear an error rather than the retry action, and dashboards and tests lack a stable selector. These localized issues make the PR low risk; the earlier unbounded-polling concerns are bounded by the timeout. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The new flow checks that a detected workspace is new and was created by the current user. It does not, however, tie that workspace to the particular connection the user started, so simultaneous connections could select the wrong workspace in a form. No direct authorization bypass was established. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (1)
frontend/src/lib/integrations/slackConnectLogic.ts-84-91 (1)
84-91: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick winCap how long polling runs after "Add to Slack" is clicked.
Polling stops only when a new Slack integration appears or the banner unmounts. If the user closes the Slack tab without authorizing, the form tab keeps polling the integrations endpoint with no end. It also polls while hidden, because of
pauseOnPageHidden: false. The UI also keeps showing "Waiting for Slack to connect" with no way out.Add a timeout, for example 10 minutes, that disposes
integrationsPollingand resetswaitingForSlack. The focus refetch still brings a late workspace back into the picker.Proposed fix
actions({ connectSlackClicked: true, slackConnected: (integrationId: number) => ({ integrationId }), + stopWaitingForSlack: true, }), reducers({ waitingForSlack: [ false, { connectSlackClicked: () => true, slackConnected: () => false, + stopWaitingForSlack: () => false, }, ], }), @@ cache.disposables.add( () => { actions.startPolling() return () => actions.stopPolling() }, 'integrationsPolling', { pauseOnPageHidden: false } ) + cache.disposables.add(() => { + const timeout = setTimeout(() => actions.stopWaitingForSlack(), 10 * 60 * 1000) + return () => clearTimeout(timeout) + }, 'slackConnectTimeout') }, + stopWaitingForSlack: () => { + cache.disposables.dispose('integrationsPolling') + },
🧹 Nitpick comments (1)
frontend/src/lib/integrations/slackChannel.ts (1)
34-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the unused
slackChannelNameexport.No import or call exists outside its declaration. Move it to the layer that consumes it, or add the consumer in this PR, so this change does not add dead code.
Source: Path instructions
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: fd0bb00d-14ad-441e-b065-eeacc64e81bd
📒 Files selected for processing (6)
frontend/src/lib/components/Comments/SlackDestinationPicker.tsxfrontend/src/lib/integrations/SlackIntegrationHelpers.tsxfrontend/src/lib/integrations/slackChannel.tsfrontend/src/lib/integrations/slackConnectLogic.test.tsfrontend/src/lib/integrations/slackConnectLogic.tsproducts/alerts/frontend/components/AlertNotificationDestinationEditor.tsx
Included review availability: Your plan provides up to 12 included reviews per hour; 5 remain after this review.
|
|
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Bound the Slack connection polling attempt. · slackConnectLogic.ts:89
frontend/src/lib/integrations/slackConnectLogic.ts:89
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winBound the Slack connection polling attempt.
If the OAuth tab closes or authorization is canceled while the banner remains mounted,
slackConnectednever runs. The banner stays on “Waiting for Slack to connect” andintegrationsLogiccontinues callingloadIntegrations()every 30 seconds, including while the page is hidden. Add an attempt timeout that disposesintegrationsPollingand clearswaitingForSlackso the user can retry. This is a localized UI and request-load defect, not a major availability failure.
🟡 Other comments (1)
frontend/src/lib/integrations/slackConnectLogic.ts-82-85 (1)
82-85: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGate Slack OAuth until integrations load.
SlackNotConfiguredBannerrenders its connect link whileintegrationsLogic.slackIntegrationsis still undefined. If the user completes OAuth before the first integrations load,connectSlackClickedstores no baseline, andloadIntegrationsSuccessrecords the new workspace as that baseline. No later load detects it, so the banner can remain waiting indefinitely.Prevent the banner from accepting clicks until the initial integrations load completes, as
SlackDestinationPickeralready does, or correlate the OAuth attempt with its result.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 725a94ce-c893-40a0-9dec-2aa3ceccab95
📒 Files selected for processing (3)
frontend/src/lib/integrations/SlackIntegrationHelpers.tsxfrontend/src/lib/integrations/slackConnectLogic.test.tsfrontend/src/lib/integrations/slackConnectLogic.ts
Included review availability: Your plan provides up to 12 included reviews per hour; 1 remains after this review.
|
@coderabbitai review-body comments: |
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (1)
frontend/src/lib/integrations/SlackIntegrationHelpers.tsx-58-60 (1)
58-60: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winHandle the initial integrations failure separately from loading.
integrationsLogicstarts withintegrations === null, so a failed first load can leaveslackIntegrationsasundefined. The banner then renders only the spinner.slackConnectLogicstarts polling only afterconnectSlackClicked, but the banner hides that link while the spinner is shown. Without another polling subscriber, the user cannot open Slack OAuth or retry.Expose the load failure and render an error state with a retry action that dispatches
loadIntegrations().Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 7938ee56-3897-4848-87a4-30825bb9b061
📒 Files selected for processing (3)
frontend/src/lib/integrations/SlackIntegrationHelpers.tsxfrontend/src/lib/integrations/slackConnectLogic.test.tsfrontend/src/lib/integrations/slackConnectLogic.ts
Included review availability: Your plan provides up to 12 included reviews per hour; 4 remain after this review.
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
git show --stat --oneline b8886efa7ee -- frontend/src/lib/integrations
ast-grep outline frontend/src/lib/integrations/slackConnectLogic.ts --items all
ast-grep outline frontend/src/lib/integrations/SlackIntegrationHelpers.tsx --items all
ast-grep outline frontend/src/lib/integrations/slackChannel.ts --items all
rg -n -C3 'SLACK_CONNECT_TIMEOUT_MS|slackConnectTimedOut|waitingForSlack|slackIntegrations|startPolling|stopPolling|target=|onConnectClick|slackChannelName' frontend/src/lib/integrations/slackConnectLogic.ts frontend/src/lib/integrations/SlackIntegrationHelpers.tsx frontend/src/lib/integrations/slackChannel.ts
rg -n -C2 '\bslackChannelName\s*\(' frontend productsLength of output: 21882
You are interacting with an AI system. |
|
Stacked PR 106430 was cancelled: the stack changed after it was queued. |
|
@coderabbitai review-body comment:
|
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (1)
frontend/src/lib/integrations/SlackIntegrationHelpers.tsx-68-68 (1)
68-68: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGive the retry button an action-based accessible label.
LemonButtonuses this tooltip as the button’saria-label. A screen reader therefore announces “Couldn't load your Slack workspaces” instead of the retry action. Change the tooltip to “Retry loading Slack workspaces,” or set an explicit action-basedaria-label.
🧹 Nitpick comments (1)
frontend/src/lib/integrations/SlackIntegrationHelpers.tsx (1)
65-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a stable identifier to the retry button.
This new
LemonButtonis a meaningful interaction covered by the frontend UI convention. Withoutdata-attr, autocapture can still record the click, but dashboards and Playwright lack a stable selector for this retry action.Suggested fix
<LemonButton type="secondary" size="small" tooltip="Couldn't load your Slack workspaces" + data-attr="retry-slack-integrations" onClick={() => loadIntegrations()} >
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 58026d66-291f-4a79-87c1-acb2ba704264
📒 Files selected for processing (1)
frontend/src/lib/integrations/SlackIntegrationHelpers.tsx
Included review availability: Your plan provides up to 12 included reviews per hour; 3 remain after this review.
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
git show --stat --oneline d7fc6ea9c3a -- frontend/src/lib/integrations/SlackIntegrationHelpers.tsx frontend/src/lib/integrations/slackConnectLogic.ts
git show --format=fuller --no-ext-diff d7fc6ea9c3a -- frontend/src/lib/integrations/SlackIntegrationHelpers.tsx frontend/src/lib/integrations/slackConnectLogic.ts
ast-grep outline frontend/src/lib/integrations/SlackIntegrationHelpers.tsx --match SlackNotConfiguredBanner --view expanded
rg -n -C6 'SlackNotConfiguredBanner|loadIntegrations|isLoading|isLoadingIntegrations|Retry|retry|slackIntegrations' frontend/src/lib/integrations/SlackIntegrationHelpers.tsxLength of output: 6247
You are interacting with an AI system. |
|
👋 Visual changes detected for this PR. Review and approve in PostHog Visual Review If these changes are unexpected, they may be caused by a flaky test or a broken snapshot on master. Don't approve — rerun the job or wait for a fix. |
The not-configured banner opened Slack's OAuth in the same tab, so any form the user was filling in was lost. It now opens a new tab, polls integrations until the new workspace appears, and hands it to the caller through onConnected. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A workspace another project member adds while the banner waits is no longer picked up, and a click before the first integrations load takes the next successful load as the baseline instead of an empty list. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…e connect link Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…rkspaces Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
d7fc6ea to
6484bb7
Compare
There was a problem hiding this comment.
Approved.
Contained frontend UX change (Slack OAuth now opens in a new tab, with polling to detect the new workspace); no auth, data-model, API-contract, billing, CI, or dependency changes. All CodeRabbit findings were fixed and confirmed in later commits, and Greptile's race-condition concern is covered by the added tests and logic (only the current user's newly-created workspace is auto-selected).
- 👍 on the PR from greptile-apps[bot].
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 249L, 5F substantive, 362L/6F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (362L, 6F, two-areas, feat) |
| stamphog 2.1.0 | .stamphog/policy.yml @ 6484bb7 · reviewed head 6484bb7 |
|
This pull request was merged into |
This branch was successfully deployed
1 participant
Problem
Changes
SlackNotConfiguredBannertakes an optionaldescription,onConnectedandonConnectClick, so a caller can say what the connection is for.slackConnectLogicinlib/integrationsholds the polling.The screenshot comes from the layer 9 stories, which render the banner at full width and inside a card.
How did you test this code?
slackConnectLogic.test.tscovers the wait: it catches a regression where a new workspace never selects itself, or the wait never ends.Release status
Automatic notifications
Docs update
None.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Agent: Claude Code, Claude Fable 5.1 wrote the original #101991. Claude Opus 5.5 split it into this stack and fixed the review findings.
🤖 Generated with Claude Code