Skip to content

[CORRUPTED] Synthetic Benchmark PR #25189 - Updated activitypub Bluesky sharing enablement flow#68

Open
tomerqodo wants to merge 6 commits intobase_pr_25189_20251204_5792from
corrupted_pr_25189_20251204_5792
Open

[CORRUPTED] Synthetic Benchmark PR #25189 - Updated activitypub Bluesky sharing enablement flow#68
tomerqodo wants to merge 6 commits intobase_pr_25189_20251204_5792from
corrupted_pr_25189_20251204_5792

Conversation

@tomerqodo
Copy link

@tomerqodo tomerqodo commented Dec 4, 2025

User description

Benchmark PR TryGhost#25189

Type: Corrupted (contains bugs)

Original PR Title: Updated activitypub Bluesky sharing enablement flow
Original PR Description: 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


Note

Switch Bluesky sharing to v2 enable/disable endpoints, add handle confirmation flow with polling, update UI/state, and bump package to 2.0.0.

  • API:
    • Migrate Bluesky endpoints to v2 (enable, disable) and add confirm-handle endpoint.
    • Change enableBluesky() to no longer return a handle; introduce confirmBlueskyHandle() returning a confirmed handle or empty string.
    • Extend Account with blueskyHandleConfirmed and make blueskyHandle nullable.
  • Hooks (apps/activitypub/src/hooks/use-activity-pub-queries.ts):
    • Add useConfirmBlueskyHandleMutationForUser and cache updater for Bluesky details.
    • Update enable/disable mutations to set {blueskyEnabled, blueskyHandleConfirmed, blueskyHandle} appropriately and invalidate follows.
  • UI (views/Preferences/components/BlueskySharing.tsx):
    • Implement enablement confirmation flow with polling, loading states, success/error toasts, and conditional rendering once handle is confirmed.
    • Update disable flow and button states; show link to Bluesky profile when confirmed.
  • Tests (src/api/activitypub.test.ts):
    • Update Bluesky tests for v2 endpoints; add tests for handle confirmation and null/invalid responses.
  • Package:
    • Bump @tryghost/activitypub version to 2.0.0.

Written by Cursor Bugbot for commit c743df5. This will update automatically on new commits. Configure here.

Original PR URL: TryGhost#25189


PR Type

Enhancement


Description

  • Migrate Bluesky API endpoints from v1 to v2 with new confirmation flow

  • Split enableBluesky() into separate enable and confirm-handle operations

  • Add handle confirmation polling mechanism with retry logic

  • Update Account model with blueskyHandleConfirmed flag and nullable handle

  • Implement UI confirmation flow with loading states and success/error handling

  • Bump @tryghost/activitypub package version to 2.0.0


Diagram Walkthrough

flowchart LR
  A["enableBluesky v1"] -->|"Migrate to v2"| B["enableBluesky v2"]
  B -->|"Returns null"| C["Poll confirmBlueskyHandle"]
  C -->|"Retry up to 12x"| D["Handle confirmed"]
  D -->|"Update cache"| E["Show Bluesky profile"]
  C -->|"Max retries exceeded"| F["Error & disable"]
  G["Account model"] -->|"Add blueskyHandleConfirmed"| H["Make blueskyHandle nullable"]
Loading

File Walkthrough

Relevant files
Enhancement
activitypub.ts
Split Bluesky enable/disable/confirm into separate v2 API methods

apps/activitypub/src/api/activitypub.ts

  • Changed enableBluesky() to use v2 endpoint and return void instead of
    handle string
  • Added new disableBluesky() method using v2 endpoint
  • Added new confirmBlueskyHandle() method using v2 confirm-handle
    endpoint with GET request
  • Updated Account interface to include blueskyHandleConfirmed boolean
    and make blueskyHandle nullable
  • Handle validation logic moved to confirmation endpoint
+17/-10 
use-activity-pub-queries.ts
Add handle confirmation mutation and update cache logic   

apps/activitypub/src/hooks/use-activity-pub-queries.ts

  • Created BlueskyDetails type to encapsulate bluesky state (enabled,
    handleConfirmed, handle)
  • Updated updateAccountBlueskyCache() to accept full BlueskyDetails
    object instead of just handle
  • Modified useEnableBlueskyMutationForUser to set enabled=true,
    handleConfirmed=false, handle=null on success
  • Modified useDisableBlueskyMutationForUser to set all bluesky fields to
    false/null on success
  • Added new useConfirmBlueskyHandleMutationForUser hook that updates
    cache with confirmed handle
+45/-7   
BlueskySharing.tsx
Implement handle confirmation polling and conditional UI rendering

apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx

  • Implemented polling mechanism with 5-second intervals and 12 retry
    limit for handle confirmation
  • Added handleConfirmed state and useEffect hook to manage confirmation
    flow lifecycle
  • Changed UI to show loading state with message during confirmation
    polling
  • Conditional rendering of Bluesky profile card only when handle is
    confirmed
  • Added "Open profile" button linking to Bluesky profile when confirmed
  • Updated error handling to show error toast instead of success on
    enable
  • Removed success toast on every visit, only shows on first confirmation
+120/-48
Tests
activitypub.test.ts
Update tests for v2 endpoints and new confirm-handle flow

apps/activitypub/src/api/activitypub.test.ts

  • Updated enableBluesky tests to use v2 endpoint and expect null
    response
  • Added new disableBluesky test suite with v2 endpoint validation
  • Added new confirmBlueskyHandle test suite with handle validation
    scenarios
  • Tests cover null responses, missing handle property, and invalid
    handle types
  • Removed old tests that expected handle to be returned from enable
    endpoint
+58/-29 
Dependencies
package.json
Bump package version to 2.0.0                                                       

apps/activitypub/package.json

  • Bumped version from 1.1.0 to 2.0.0 to reflect breaking API changes
+1/-1     

mike182uk and others added 6 commits November 20, 2025 12:07
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
@qodo-code-review
Copy link

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Missing Audits: Newly added enable/disable/confirm Bluesky flows perform critical account state changes
and polling without any added audit logging of who did what and when.

Referred Code
const handleEnable = async () => {
    if (!account?.avatarUrl) {
        setIsEditingProfile(true);
    } else {
        setLoading(true);
        try {
            await enableBlueskyMutation.mutateAsync();
        } catch (error) {
            setLoading(false);
            toast.error('Something went wrong, please try again.');
        }
    }
};

const handleDisable = async () => {
    setLoading(true);
    try {
        await disableBlueskyMutation.mutateAsync();
        setShowConfirm(false);
        toast.success('Bluesky sharing disabled');


 ... (clipped 59 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Generic Errors: The UI shows a generic toast on enable failure and on max retries without surfacing
actionable context or handling specific API error states beyond 429, and confirmHandle
ignore-catches promise rejections.

Referred Code
        await disableBlueskyMutation.mutateAsync();
        setShowConfirm(false);
        toast.success('Bluesky sharing disabled');
    } finally {
        setLoading(false);
    }
};

const confirmHandle = useCallback(async () => {
    confirmBlueskyHandleMutation.mutateAsync().then((handle) => {
        if (handle) {
            setHandleConfirmed(true);
        }
    });
    // eslint-disable-next-line react-hooks/exhaustive-deps
}, []); // Empty deps - mutations are stable in practice

useEffect(() => {
    if (!account?.blueskyEnabled) {
        setHandleConfirmed(false);
        setLoading(false);


 ... (clipped 41 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Input Validation: New confirmBlueskyHandle GET endpoint response is minimally validated (only type check),
and UI polling trusts non-empty handle without additional format validation or throttling
guard, which may allow unexpected values from external service.

Referred Code
async confirmBlueskyHandle(): Promise<string> {
    const url = new URL('.ghost/activitypub/v2/actions/bluesky/confirm-handle', this.apiUrl);

    const json = await this.fetchJSON(url, 'GET');

    if (json === null || !('handle' in json) || typeof json.handle !== 'string') {
        return '';
    }

    return String(json.handle);
}

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Polling logic is flawed and inefficient

The polling logic should be refactored to use react-query's declarative useQuery
with refetchInterval and retry options, instead of the current manual and
error-prone setInterval implementation in a useEffect hook.

Examples:

apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx [73-126]
    const confirmHandle = useCallback(async () => {
        confirmBlueskyHandleMutation.mutateAsync().then((handle) => {
            if (handle) {
                setHandleConfirmed(true);
            }
        });
        // eslint-disable-next-line react-hooks/exhaustive-deps
    }, []); // Empty deps - mutations are stable in practice

    useEffect(() => {

 ... (clipped 44 lines)

Solution Walkthrough:

Before:

// in BlueskySharing.tsx
const confirmBlueskyHandleMutation = useConfirmBlueskyHandleMutationForUser('index');
const retryCountRef = useRef(0);

const confirmHandle = useCallback(async () => {
    // This can lead to an unhandled promise rejection if mutateAsync fails
    confirmBlueskyHandleMutation.mutateAsync().then((handle) => {
        if (handle) {
            setHandleConfirmed(true);
        }
    });
}, []);

useEffect(() => {
    // ... logic to check if polling should start ...
    const confirmHandleInterval = setInterval(async () => {
        retryCountRef.current += 1;
        if (retryCountRef.current > MAX_CONFIRMATION_RETRIES) {
            // ... handle timeout and disable ...
        }
        confirmHandle();
    }, CONFIRMATION_INTERVAL);

    return () => clearInterval(confirmHandleInterval);
}, [/* dependencies */]);

After:

// in use-activity-pub-queries.ts
export function useConfirmBlueskyHandleQuery(handle, options) {
    return useQuery({
        queryKey: ['confirm_bluesky_handle', handle],
        queryFn: () => { /* ... fetch handle ... */ },
        enabled: options.enabled,
        refetchInterval: CONFIRMATION_INTERVAL,
        retry: MAX_CONFIRMATION_RETRIES,
        onSuccess: (blueskyHandle) => { /* ... update cache ... */ },
        onError: () => { /* ... disable bluesky, show toast ... */ }
    });
}

// in BlueskySharing.tsx
const shouldPoll = account?.blueskyEnabled && !account?.blueskyHandleConfirmed;
useConfirmBlueskyHandleQuery('index', { enabled: shouldPoll });

// The component now just needs to react to the loading and data states
// from the query, with no manual setInterval or retry logic.
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a significant design flaw and a bug (unhandled promise rejection) in the new polling logic, and proposes a much more robust and idiomatic solution using react-query's built-in features.

High
Possible issue
Avoid incorrect handle confirmation state

In useConfirmBlueskyHandleMutationForUser, add a check in the onSuccess callback
to only update the cache if a non-empty blueskyHandle is returned, preventing an
incorrect confirmation state.

apps/activitypub/src/hooks/use-activity-pub-queries.ts [2810-2833]

 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) {
-            updateAccountBlueskyCache(queryClient, {
-                blueskyEnabled: true,
-                blueskyHandleConfirmed: true,
-                blueskyHandle: blueskyHandle
-            });
+            if (blueskyHandle) {
+                updateAccountBlueskyCache(queryClient, {
+                    blueskyEnabled: true,
+                    blueskyHandleConfirmed: true,
+                    blueskyHandle: blueskyHandle
+                });
+            }
         },
         onError(error: {message: string, statusCode: number}) {
             if (error.statusCode === 429) {
                 renderRateLimitError();
             }
         }
     });
 }
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a bug where the application cache could enter an inconsistent state (blueskyHandleConfirmed: true, blueskyHandle: ''), which would break the handle confirmation polling logic.

Medium
Prevent unhandled promise rejection errors

Refactor the confirmHandle function to use async/await with a try...catch block
instead of .then() to prevent unhandled promise rejections and improve error
handling.

apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx [73-80]

 const confirmHandle = useCallback(async () => {
-    confirmBlueskyHandleMutation.mutateAsync().then((handle) => {
+    try {
+        const handle = await confirmBlueskyHandleMutation.mutateAsync();
         if (handle) {
             setHandleConfirmed(true);
         }
-    });
+    } catch (e) {
+        // Errors are handled by the mutation's onError callback,
+        // but this catch block prevents unhandled promise rejections.
+    }
     // eslint-disable-next-line react-hooks/exhaustive-deps
 }, []); // Empty deps - mutations are stable in practice
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly points out an unhandled promise rejection, which is a potential bug. Using async/await with try/catch is a robust pattern for handling asynchronous errors and improves code quality.

Low
  • More

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments