Updated activitypub Bluesky sharing enablement flow#21
Conversation
ref TryGhost/ActivityPub#1377 We updated the backend implementation of the Bluesky sharing enablement process to include a confirmation step in which the client would prompt the backend to check what handle has been assigned to the Bluesky account generated by Bridgy Fed - This circumvents the need to for us to compute the handle manually and potentially getting it wrong if we compute it in a different way to Bridgy Fed
WalkthroughThis pull request introduces a new Bluesky handle confirmation workflow by upgrading API endpoints from v1 to v2, refactoring the Account type to include confirmation state, separating handle confirmation into a dedicated method, and enhancing the UI component with polling-based confirmation logic. The package version is bumped to 2.0.0 and TypeScript strict mode is disabled. Changes
Sequence DiagramsequenceDiagram
participant User as User (UI)
participant Component as BlueskySharing Component
participant API as ActivityPubAPI
participant QueryClient as Query Client
participant Endpoint as v2 Endpoint
User->>Component: Enable Bluesky
Component->>API: enableBluesky()
API->>Endpoint: POST /v2/actions/bluesky/enable
Endpoint-->>API: 200 OK
API-->>Component: void
Component->>QueryClient: updateAccountBlueskyCache({enabled: true, confirmed: false})
Component->>Component: Start polling effect
loop Polling (max retries)
Component->>API: confirmBlueskyHandle(handle)
API->>Endpoint: GET /v2/actions/bluesky/confirm-handle
Endpoint-->>API: {handle: "user.bsky.social"}
API-->>Component: "user.bsky.social"
Component->>QueryClient: updateAccountBlueskyCache({enabled: true, confirmed: true, handle: "user.bsky.social"})
Component->>Component: Stop polling, show success toast
Component->>User: Display confirmed Bluesky profile
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx`:
- Around line 103-117: The interval callback clears the interval and calls await
disableBlueskyMutation.mutateAsync() then setLoading(false), which can race with
unmounts or interval clears causing state updates on an unmounted component;
update the logic in the confirmHandleInterval handler (referencing
confirmHandleInterval, retryCountRef, MAX_CONFIRMATION_RETRIES,
CONFIRMATION_INTERVAL, confirmHandle) to either (a) use the mutation's
onSettled/onSuccess/onError callbacks to call setLoading(false) (and clear the
interval there), or (b) track component mounted state with a ref (e.g.,
isMountedRef) and check it before calling setLoading(false) after awaiting
disableBlueskyMutation.mutateAsync(), ensuring the mutation is awaited and you
only call setLoading when mounted and after the mutation settles.
🧹 Nitpick comments (5)
apps/activitypub/tsconfig.json (1)
18-18: Re-enable strict to avoid masking type regressions.Turning off strict mode broadly weakens type guarantees and can hide issues introduced by the new ActivityPub API changes. Prefer restoring
"strict": trueand fixing or narrowly suppressing the specific type errors (e.g., per-file//@ts-expect-error`` or selective flags) rather than disabling strict globally.♻️ Suggested change
- "strict": false, + "strict": true,[recommendation] Please confirm the concrete plan to restore strict mode (and/or which specific strict flags need relaxation) so this doesn’t become a permanent downgrade.
apps/activitypub/src/api/activitypub.ts (1)
723-733: Minor: RedundantString()conversion.On line 732, the
String(json.handle)call is redundant since the type guard on line 728 already confirmsjson.handleis a string.Suggested simplification
- return String(json.handle); + return json.handle;apps/activitypub/src/hooks/use-activity-pub-queries.ts (1)
2810-2841: Address the TODO comment or remove it.Line 2833 contains a comment noting missing invalidation of
accountFollowsquery. TheuseEnableBlueskyMutationForUseranduseDisableBlueskyMutationForUserhooks both invalidate this query, so it seems intentional to include it here as well for consistency when the handle is confirmed.Would you like me to add the missing invalidation, or should this comment be removed if the invalidation is intentionally omitted?
Suggested fix if invalidation is needed
updateAccountBlueskyCache(queryClient, { blueskyEnabled: true, blueskyHandleConfirmed: true, blueskyHandle: blueskyHandle }); - // Note: Missing invalidation of accountFollows query that should happen here + queryClient.invalidateQueries({ + queryKey: QUERY_KEYS.accountFollows('index', 'following') + }); },apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx (2)
68-75: Consider removing the eslint-disable or refactoring.The
confirmHandlecallback has an empty dependency array with an eslint-disable comment. While React Query mutations are stable in practice, this pattern can be fragile. Consider usinguseCallbackwithout disabling the lint rule:Alternative approach
- const confirmHandle = useCallback(() => { - confirmBlueskyHandleMutation.mutateAsync().then((handle) => { - if (handle) { - setHandleConfirmed(true); - } - }); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); // Empty deps - mutations are stable in practice + const confirmHandle = useCallback(() => { + confirmBlueskyHandleMutation.mutateAsync().then((handle) => { + if (handle) { + setHandleConfirmed(true); + } + }); + }, [confirmBlueskyHandleMutation]);Since
confirmBlueskyHandleMutationis indeed stable (returned fromuseMutation), including it won't cause extra re-renders but will satisfy the linter.
77-121: Verify: Initial polling call timing.The polling effect starts with
retryCountRef.current = 0on line 101, then the firstconfirmHandle()call happens after 5 seconds (CONFIRMATION_INTERVAL). This means users wait 5 seconds before the first confirmation attempt.Consider calling
confirmHandle()immediately when Bluesky is enabled but not yet confirmed, then starting the interval for subsequent retries.Suggested improvement for faster initial confirmation
setHandleConfirmed(false); setLoading(true); retryCountRef.current = 0; + // Attempt confirmation immediately + confirmHandle(); + const confirmHandleInterval = setInterval(async () => { if (retryCountRef.current >= MAX_CONFIRMATION_RETRIES) {
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/activitypub/package.jsonapps/activitypub/src/api/activitypub.test.tsapps/activitypub/src/api/activitypub.tsapps/activitypub/src/hooks/use-activity-pub-queries.tsapps/activitypub/src/views/Preferences/components/BlueskySharing.tsxapps/activitypub/tsconfig.json
🧰 Additional context used
🧬 Code graph analysis (1)
apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx (1)
apps/activitypub/src/hooks/use-activity-pub-queries.ts (4)
useAccountForUser(1616-1628)useEnableBlueskyMutationForUser(2746-2776)useDisableBlueskyMutationForUser(2778-2808)useConfirmBlueskyHandleMutationForUser(2810-2841)
🔇 Additional comments (11)
apps/activitypub/package.json (1)
3-3: LGTM!The major version bump to 2.0.0 is appropriate given the breaking API changes (v1→v2 endpoints, modified
Accounttype with nullableblueskyHandle, and the new confirmation flow).apps/activitypub/src/api/activitypub.ts (2)
29-30: LGTM!The
Accountinterface updates properly support the new Bluesky confirmation flow withblueskyHandleConfirmedflag and nullableblueskyHandle.
711-721: LGTM!The
enableBlueskyanddisableBlueskymethods are correctly updated to use v2 endpoints. The return type change fromPromise<string>toPromise<void>aligns with the new flow where the handle is obtained separately viaconfirmBlueskyHandle.apps/activitypub/src/api/activitypub.test.ts (3)
1600-1627: LGTM!Test correctly verifies the
enableBlueskymethod calls the v2 endpoint with POST and no longer expects a return value.
1629-1656: LGTM!Test correctly verifies the new
disableBlueskymethod calls the v2 endpoint with POST.
1658-1767: LGTM!Comprehensive test coverage for
confirmBlueskyHandleincluding:
- Successful handle confirmation
- Null response handling
- Missing handle property
- Invalid handle type (array instead of string)
apps/activitypub/src/hooks/use-activity-pub-queries.ts (2)
2725-2744: LGTM!The
BlueskyDetailstype and refactoredupdateAccountBlueskyCachefunction cleanly encapsulate the Bluesky state fields and provide a more maintainable cache update pattern.
2746-2776: LGTM!The
useEnableBlueskyMutationForUsercorrectly setsblueskyHandleConfirmed: falseon success, properly reflecting the intermediate state before handle confirmation completes.apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx (3)
27-28: LGTM!The polling constants (5s interval, 12 retries = 60s max) provide a reasonable timeout for handle confirmation while keeping the interval balanced between responsiveness and server load.
138-149: LGTM!The
showAsEnabledcomputed flag correctly gates the UI based on bothblueskyEnabledANDblueskyHandleConfirmed, ensuring a consistent display state.
190-228: LGTM!The confirmed-state UI section properly displays the Bluesky profile card with avatar, handle, copy functionality, and external profile link. The conditional rendering on
handleConfirmedensures this only shows after successful confirmation.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| const confirmHandleInterval = setInterval(async () => { | ||
| if (retryCountRef.current >= MAX_CONFIRMATION_RETRIES) { | ||
| clearInterval(confirmHandleInterval); | ||
|
|
||
| toast.error('Something went wrong, please try again.'); | ||
|
|
||
| await disableBlueskyMutation.mutateAsync(); | ||
| setLoading(false); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| retryCountRef.current += 1; | ||
| confirmHandle(); | ||
| }, CONFIRMATION_INTERVAL); |
There was a problem hiding this comment.
Potential race condition: mutation not awaited before state update.
On line 109, disableBlueskyMutation.mutateAsync() is awaited, but if the interval is cleared (e.g., by component unmount or state change) while the mutation is in-flight, the subsequent setLoading(false) on line 110 may not execute as expected, or may execute on an unmounted component.
Consider adding a mounted ref check or using the mutation's onSettled callback instead.
Suggested improvement
+ const mountedRef = useRef(true);
+
+ useEffect(() => {
+ return () => {
+ mountedRef.current = false;
+ };
+ }, []);
+
// In the interval callback:
if (retryCountRef.current >= MAX_CONFIRMATION_RETRIES) {
clearInterval(confirmHandleInterval);
toast.error('Something went wrong, please try again.');
- await disableBlueskyMutation.mutateAsync();
- setLoading(false);
+ disableBlueskyMutation.mutateAsync().finally(() => {
+ if (mountedRef.current) {
+ setLoading(false);
+ }
+ });
return;
}🤖 Prompt for AI Agents
In `@apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx` around
lines 103 - 117, The interval callback clears the interval and calls await
disableBlueskyMutation.mutateAsync() then setLoading(false), which can race with
unmounts or interval clears causing state updates on an unmounted component;
update the logic in the confirmHandleInterval handler (referencing
confirmHandleInterval, retryCountRef, MAX_CONFIRMATION_RETRIES,
CONFIRMATION_INTERVAL, confirmHandle) to either (a) use the mutation's
onSettled/onSuccess/onError callbacks to call setLoading(false) (and clear the
interval there), or (b) track component mounted state with a ref (e.g.,
isMountedRef) and check it before calling setLoading(false) after awaiting
disableBlueskyMutation.mutateAsync(), ensuring the mutation is awaited and you
only call setLoading when mounted and after the mutation settles.
3 participants
Benchmark PR from qodo-benchmark#244
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.