feat(onboarding): Revalidate restored SCM messaging destinations - #121187
Open
jaydgoss wants to merge 8 commits into
Open
feat(onboarding): Revalidate restored SCM messaging destinations#121187jaydgoss wants to merge 8 commits into
jaydgoss wants to merge 8 commits into
Conversation
github-actions
Bot
added
the
Scope: Frontend
Contributor
Sentry Snapshot Testing
|
jaydgoss
force-pushed
the
feat/vdy-140-treatment-route-state
branch
from
August 4, 2026 18:10
4a1eaad to
dd36973
Compare
jaydgoss
force-pushed
the
feat/vdy-140-messaging-revalidation
branch
from
August 4, 2026 18:10
75d419a to
6775e19
Compare
jaydgoss
marked this pull request as ready for review
August 4, 2026 20:29
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
jaydgoss
force-pushed
the
feat/vdy-140-messaging-revalidation
branch
from
August 4, 2026 21:15
6775e19 to
5a1e02a
Compare
jaydgoss
force-pushed
the
feat/vdy-140-messaging-revalidation
branch
from
August 4, 2026 21:22
5a1e02a to
1124beb
Compare
jaydgoss
force-pushed
the
feat/vdy-140-treatment-route-state
branch
from
August 4, 2026 21:24
295494f to
75c146a
Compare
jaydgoss
force-pushed
the
feat/vdy-140-messaging-revalidation
branch
from
August 4, 2026 21:24
1124beb to
3d4847c
Compare
jaydgoss
force-pushed
the
feat/vdy-140-messaging-revalidation
branch
from
August 4, 2026 21:42
3d4847c to
227da47
Compare
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
jaydgoss
marked this pull request as draft
August 4, 2026 22:29
jaydgoss
marked this pull request as ready for review
August 5, 2026 15:41
jaydgoss
force-pushed
the
feat/vdy-140-treatment-route-state
branch
from
August 5, 2026 16:05
882961a to
d9f4a41
Compare
jaydgoss
force-pushed
the
feat/vdy-140-messaging-revalidation
branch
from
August 5, 2026 16:05
4871a76 to
93b086d
Compare
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d21a47c. Configure here.
A destination restored from session storage names an organization-scoped integration and channel that may no longer exist. Revalidate both before treating it as usable, and keep it non-submittable while either query is in flight. An empty channel list is deliberately not treated as staleness: every provider helper in organization_integration_channels.py returns [] when its upstream call fails, so `results: []` cannot be told apart from a deleted channel. Resetting on it would discard a valid destination during a transient Slack or Discord outage. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Do not clear a restored destination from stale cached integration or channel data while TanStack Query is refetching. Revalidate against the completed response before updating session state.
Validate a restored integration through its direct organization-scoped endpoint and keep a channel omitted from a partial listing in session state. The destination remains non-submittable until it can be verified. Refs VDY-140
Distinguish disabled or pending-deletion integrations from missing integrations when revalidating a restored messaging destination. Refs VDY-140
Extract the integration active check and the saved-identifier match into pure module-level functions, and drop both useMemo wrappers. Neither memo was doing anything: one returns the query data object, the other an element of it, and both references are already stable across renders, so the effect dependency identities are unchanged. Name the two query-settled conditions so the effect guards and the returned flags read the same way, which also shrinks the effect dependency array from eleven entries to eight. isValid now requires the integration to have resolved rather than just checking the raw query flags, matching how isError and isPending already gate their channels half. Previously a cached channel list plus an integration that returns 200 but disabled would render the inactive warning and the selected state at the same time. Refs VDY-140
The shared query client already refuses to retry 404 responses using the same instanceof RequestError plus status check that isNotFoundError performs, so the override changed nothing for the missing-integration path it was written for. Because a query-level retry replaces the default rather than composing with it, it did drop the non-retry behavior for 400, 401, 402 and 403, which every other query treats as terminal, and it overrode the retry: false that makeTestQueryClient applies. The channels query alongside it kept the defaults, so the two halves of the hook disagreed on how to handle a failure. Refs VDY-140
Two effects touch staleReason: one clears it when a new destination is selected, the other sets it during revalidation. Their correctness rests on two assumptions that live in separate files and are easy to break silently. The first test drives the real OnboardingContextProvider and writes an unrelated field. It passes because every setter shallow-spreads prev and useSessionStorage returns the in-memory object rather than re-parsing storage, so the nested messagingSetup reference is carried through untouched. A future setter that rebuilds that nested object, or a storage-event listener that re-parses, would fail here. The second test forces a new messagingSetup reference directly. It passes because the clearing effect's dependency array is a strict subset of the validating effect's, so both run in the same commit and the two setStaleReason calls batch into one update. Dropping messagingSetup from the validating effect's deps fails this test, which is what makes it worth keeping. Refs VDY-140
jaydgoss
force-pushed
the
feat/vdy-140-messaging-revalidation
branch
from
August 5, 2026 17:43
d21a47c to
39e4610
Compare
…tests The stale-warning test mounts OnboardingContextProvider and clicks through a context setter, which persists the seeded messagingSetup under the onboarding session-storage key. useSessionStorage prefers a stored value over initialValue, so any later context-backed test in this file would silently inherit that state. Mirrors the afterEach in onboardingContext.spec.tsx.
Abdkhan14
approved these changes
Aug 5, 2026
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
2 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.
TLDR
A messaging destination restored from session storage names an organization-scoped integration and channel that may no longer exist. This revalidates both before treating the destination as usable, and keeps it non-submittable while either query is in flight.
Details
Split out of #120992, which established the treatment route and the session contract. It lands separately because nothing can write a
selecteddestination yet — VDY-143 adds the inline picker and VDY-141 addsSet up later. Until then this logic is unreachable in production, so reviewing it on its own terms is more useful than folding it into the route PR, and #120992 stays focused on the four- versus five-step split.useScmMessagingSetupValidationre-fetches the saved active integration and, if it resolves, the channel list for it. A missing integration resets state tounconfiguredwith an explanation. Cached values stay non-submittable while either query is fetching, so a restored destination is never shown as selected on stale cache.An empty channel list is deliberately not treated as staleness. Every provider helper in
organization_integration_channels.pyreturns[]when its upstream API call fails, soresults: []cannot be distinguished from a deleted channel; resetting on it would discard a valid destination during a transient Slack or Discord outage. Only a populated list that omits the saved channel counts as stale.The hook is file-local. Exporting it before it has a second consumer trips knip; VDY-143 lifts it out when the picker needs it.
Coverage: revalidation before showing selected, missing integration, missing channel, the empty-list outage case, and a stale query cache.
Refs VDY-140