Updated activitypub Bluesky sharing enablement flow#34
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
WalkthroughVersion bump to 2.0.0 with API endpoint migration from v1 to v2. enableBluesky signature changed to return void instead of string. New confirmBlueskyHandle and disableBluesky methods added. BlueskyDetails type introduced for cache management. UI enhanced with polling/retry logic for handle confirmation. Strict TypeScript checking disabled. Changes
Sequence DiagramsequenceDiagram
actor User
participant BlueskyUI as BlueskySharing<br/>Component
participant QueryHooks as React Query<br/>Hooks
participant API as ActivityPub<br/>API
participant Cache as Query<br/>Cache
User->>BlueskyUI: Click Enable Bluesky
BlueskyUI->>QueryHooks: enableBlueskyMutation.mutate()
QueryHooks->>API: POST /v2/bluesky/enable
API-->>QueryHooks: 200 OK (void)
QueryHooks->>Cache: updateAccountBlueskyCache(BlueskyDetails)
Cache-->>BlueskyUI: Update state
Note over BlueskyUI: Start confirmation polling<br/>(interval, max retries)
loop Confirmation Poll
BlueskyUI->>QueryHooks: confirmBlueskyHandleMutation.mutate()
QueryHooks->>API: POST /v2/bluesky/confirm-handle
API-->>QueryHooks: { handle: "user.bsky" }
QueryHooks->>Cache: updateAccountBlueskyCache(BlueskyDetails<br/>{ confirmed: true, handle })
Cache-->>BlueskyUI: Update state
BlueskyUI->>BlueskyUI: showAsEnabled = true
end
BlueskyUI->>User: Show profile card & "Enabled"
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx (1)
48-55: Missing error handling for enable mutation.The
handleEnablefunction previously hadtry/finallywrapping but now callsmutateAsyncwithout error handling. If the mutation fails,loadingremainstrueindefinitely and the user has no feedback.Suggested fix
const handleEnable = async () => { if (!account?.avatarUrl) { setIsEditingProfile(true); } else { setLoading(true); - await enableBlueskyMutation.mutateAsync(); + try { + await enableBlueskyMutation.mutateAsync(); + } catch { + setLoading(false); + } } };
🤖 Fix all issues with AI agents
In `@apps/activitypub/src/hooks/use-activity-pub-queries.ts`:
- Around line 2810-2840: In useConfirmBlueskyHandleMutationForUser's onSuccess
handler, either remove the trailing TODO comment if no follow-state change
occurs, or invalidate the accountFollows cache to match the enable/disable
mutations: use the existing queryClient (from useQueryClient()) inside onSuccess
to call queryClient.invalidateQueries('accountFollows') (or the project's
canonical accountFollows query key) after updateAccountBlueskyCache so the
follow relationship with the Bridgy Fed account is refreshed.
In `@apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx`:
- Around line 103-117: The current polling uses setInterval(confirmHandle) which
can start overlapping async confirmHandle calls; change to a chainable approach
that waits for each confirmHandle to finish before scheduling the next attempt:
replace the setInterval flow (confirmHandleInterval, CONFIRMATION_INTERVAL,
retryCountRef, MAX_CONFIRMATION_RETRIES) with a sequential loop using an async
function that calls await confirmHandle(), increments retryCountRef.current,
checks MAX_CONFIRMATION_RETRIES, and uses setTimeout to schedule the next
iteration (or returns and runs disableBlueskyMutation.mutateAsync(), toast.error
and setLoading(false) when retries exhausted); alternatively add a boolean guard
(e.g., isConfirming) around confirmHandle to prevent concurrent executions while
preserving the existing retry and error handling (including
disableBlueskyMutation.mutateAsync, toast.error, setLoading).
In `@apps/activitypub/tsconfig.json`:
- Line 18: Remove the global relaxation ("strict": false) and instead fix the
underlying type problems by introducing proper TypeScript definitions for Actor
and Activity and eliminating unsafe casts; update the code that currently uses
"as any" for Actor and Activity (search for usages in files that cast to
Actor/Activity) to use the new types, replace "as unknown" casts in Search.tsx
and Notifications.tsx with correct typed conversions or guarded parsing, and
where an exception is legitimately untypable, add targeted `@ts-expect-error`
comments; ensure blueskyHandle remains typed as string | null and keep existing
optional chaining usages intact.
🧹 Nitpick comments (1)
apps/activitypub/src/api/activitypub.ts (1)
723-733: Minor: RedundantString()conversion.Line 732 wraps
json.handlewithString(), but the type check on line 728 already confirms it's a string. This is harmless but unnecessary.Suggested simplification
- return String(json.handle); + return json.handle;
📜 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 (2)
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)
apps/activitypub/src/hooks/use-activity-pub-queries.ts (1)
apps/admin-x-framework/src/index.ts (1)
useQueryClient(44-44)
🔇 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:
enableBlueskynow returnsvoidinstead of a handle, and the new confirmation flow introduces a different contract for consumers.apps/activitypub/src/api/activitypub.ts (2)
29-30: LGTM!The new
blueskyHandleConfirmedboolean and makingblueskyHandleexplicitlystring | nullcorrectly models the two-phase confirmation flow where enabling Bluesky doesn't immediately provide a handle.
711-715: LGTM!The migration to v2 endpoint and void return type aligns with the new flow where handle confirmation is a separate step.
apps/activitypub/src/api/activitypub.test.ts (3)
1600-1627: LGTM!The test correctly verifies the v2 endpoint and void return behavior for
enableBluesky.
1629-1656: LGTM!The
disableBlueskytest properly validates the POST method and v2 endpoint.
1658-1767: LGTM!Comprehensive test coverage for
confirmBlueskyHandleincluding edge cases: null response, missing handle property, and invalid handle type. This ensures the defensive checks in the implementation are properly exercised.apps/activitypub/src/hooks/use-activity-pub-queries.ts (2)
2725-2744: LGTM!The
BlueskyDetailstype and refactoredupdateAccountBlueskyCachefunction provide a clean interface for managing the three-field Bluesky state atomically.
2756-2768: LGTM!The enable mutation correctly sets
blueskyHandleConfirmed: falseinitially, reflecting that the handle confirmation is now a separate step. TheaccountFollowsinvalidation handles the Bridgy Fed follow relationship.apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx (3)
27-28: LGTM!The polling constants (5 second interval, 12 max retries = 60 seconds total timeout) provide reasonable bounds for the confirmation flow.
138-149: LGTM!The
showAsEnabledderived state cleanly gates the UI to only show the "Enabled" button when both conditions are met.
190-228: LGTM!The confirmed profile display provides good UX with avatar, Bluesky badge overlay, handle with copy button, and direct link to the Bluesky profile.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| export function useConfirmBlueskyHandleMutationForUser(handle: string) { | ||
| const queryClient = useQueryClient(); | ||
|
|
||
| return useMutation({ | ||
| async mutationFn() { | ||
| const siteUrl = await getSiteUrl(); | ||
| const api = createActivityPubAPI(handle, siteUrl); | ||
|
|
||
| return api.confirmBlueskyHandle(); | ||
| }, | ||
| onSuccess(blueskyHandle: string) { | ||
| // If the bluesky handle is empty then the handle was not confirmed | ||
| // so we don't need to update the cache | ||
| if (blueskyHandle === '') { | ||
| return; | ||
| } | ||
|
|
||
| updateAccountBlueskyCache(queryClient, { | ||
| blueskyEnabled: true, | ||
| blueskyHandleConfirmed: true, | ||
| blueskyHandle: blueskyHandle | ||
| }); | ||
|
|
||
| // Note: Missing invalidation of accountFollows query that should happen here | ||
| }, | ||
| onError(error: {message: string, statusCode: number}) { | ||
| if (error.statusCode === 429) { | ||
| renderRateLimitError(); | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
Address the missing accountFollows invalidation.
Line 2833 contains a comment noting that the accountFollows query invalidation is missing. The enable and disable mutations both invalidate this query to reflect follow relationship changes with the Bridgy Fed account. If confirming the handle can also affect follow state, this should be added for consistency.
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')
+ });
},If the invalidation is intentionally omitted (because confirmation doesn't change follow state), remove the TODO comment to avoid confusion.
🤖 Prompt for AI Agents
In `@apps/activitypub/src/hooks/use-activity-pub-queries.ts` around lines 2810 -
2840, In useConfirmBlueskyHandleMutationForUser's onSuccess handler, either
remove the trailing TODO comment if no follow-state change occurs, or invalidate
the accountFollows cache to match the enable/disable mutations: use the existing
queryClient (from useQueryClient()) inside onSuccess to call
queryClient.invalidateQueries('accountFollows') (or the project's canonical
accountFollows query key) after updateAccountBlueskyCache so the follow
relationship with the Bridgy Fed account is refreshed.
| 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 overlapping confirmation calls if mutation takes longer than interval.
The setInterval calls confirmHandle() every 5 seconds, but confirmHandle is async and doesn't await completion before the next interval fires. If the confirmation request is slow, multiple concurrent requests could be in flight.
Consider using setTimeout chained after each confirmation completes, or guard against concurrent calls.
Alternative approach using chained setTimeout
- const confirmHandleInterval = setInterval(async () => {
+ let timeoutId: NodeJS.Timeout;
+ const scheduleNextConfirmation = () => {
if (retryCountRef.current >= MAX_CONFIRMATION_RETRIES) {
- clearInterval(confirmHandleInterval);
-
toast.error('Something went wrong, please try again.');
-
- await disableBlueskyMutation.mutateAsync();
+ disableBlueskyMutation.mutateAsync();
setLoading(false);
-
return;
}
-
retryCountRef.current += 1;
- confirmHandle();
- }, CONFIRMATION_INTERVAL);
+ confirmBlueskyHandleMutation.mutateAsync().then((handle) => {
+ if (handle) {
+ setHandleConfirmed(true);
+ } else {
+ timeoutId = setTimeout(scheduleNextConfirmation, CONFIRMATION_INTERVAL);
+ }
+ });
+ };
+ timeoutId = setTimeout(scheduleNextConfirmation, CONFIRMATION_INTERVAL);
- return () => clearInterval(confirmHandleInterval);
+ return () => clearTimeout(timeoutId);🤖 Prompt for AI Agents
In `@apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx` around
lines 103 - 117, The current polling uses setInterval(confirmHandle) which can
start overlapping async confirmHandle calls; change to a chainable approach that
waits for each confirmHandle to finish before scheduling the next attempt:
replace the setInterval flow (confirmHandleInterval, CONFIRMATION_INTERVAL,
retryCountRef, MAX_CONFIRMATION_RETRIES) with a sequential loop using an async
function that calls await confirmHandle(), increments retryCountRef.current,
checks MAX_CONFIRMATION_RETRIES, and uses setTimeout to schedule the next
iteration (or returns and runs disableBlueskyMutation.mutateAsync(), toast.error
and setLoading(false) when retries exhausted); alternatively add a boolean guard
(e.g., isConfirming) around confirmHandle to prevent concurrent executions while
preserving the existing retry and error handling (including
disableBlueskyMutation.mutateAsync, toast.error, setLoading).
|
|
||
| /* Linting */ | ||
| "strict": true, | ||
| "strict": false, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the current tsconfig.json file and its history
cat -n apps/activitypub/tsconfig.jsonRepository: qodo-benchmark/Ghost-combined-coderabbit
Length of output: 1148
🏁 Script executed:
# Look for any mentions of blueskyHandle in the codebase
rg -n "blueskyHandle" --type=ts --type=jsRepository: qodo-benchmark/Ghost-combined-coderabbit
Length of output: 4104
🏁 Script executed:
# Check for TypeScript compilation errors or type-related issues
rg -n "(as any|as unknown|@ts-ignore|any\b)" apps/activitypub/src --type=ts --type=js -A 2Repository: qodo-benchmark/Ghost-combined-coderabbit
Length of output: 4720
🏁 Script executed:
# Check what the overall strict configuration affects
rg -n "strict" apps/activitypub/ --type=jsonRepository: qodo-benchmark/Ghost-combined-coderabbit
Length of output: 138
Disabling strict mode should be replaced with fixing underlying type issues.
Setting "strict": false weakens TypeScript's type safety. However, reviewing the codebase shows the specific concern about blueskyHandle: string | null is not problematic—the code consistently uses safe optional chaining (account?.blueskyHandle || '', account?.blueskyHandle?.replace(...)).
The actual issue is the codebase uses unsafe type casts like as any (for Actor and Activity types) and as unknown (in Search.tsx, Notifications.tsx). Rather than disabling strict mode globally, define proper types for Actor and Activity to replace the any types, or use @ts-expect-error comments for legitimate exceptions. This preserves null safety on fields like blueskyHandle while addressing the real type issues.
🤖 Prompt for AI Agents
In `@apps/activitypub/tsconfig.json` at line 18, Remove the global relaxation
("strict": false) and instead fix the underlying type problems by introducing
proper TypeScript definitions for Actor and Activity and eliminating unsafe
casts; update the code that currently uses "as any" for Actor and Activity
(search for usages in files that cast to Actor/Activity) to use the new types,
replace "as unknown" casts in Search.tsx and Notifications.tsx with correct
typed conversions or guarded parsing, and where an exception is legitimately
untypable, add targeted `@ts-expect-error` comments; ensure blueskyHandle remains
typed as string | null and keep existing optional chaining usages intact.
3 participants
Benchmark PR from qodo-benchmark#244
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.