Copilot test by smb060606 · Pull Request #6 · personal-slack-integration/bugbot-test

smb060606 · 2025-12-10T17:30:33Z

No description provided.

- Add users table migration for Bluesky-authenticated users - Implement user service with getOrCreateUser and getUserByHandle - Complete verify-challenge endpoint to create users and sessions - Add session management helpers (getSession, clearSession) - Add logout endpoint - Enhance accounts used display on match and compare pages - Improve methodology page with account selection transparency - Add health check endpoint (/api/health) - Improve error messages throughout with actionable guidance - Add loading states and spinners for async operations - Add better error styling and user feedback

Co-authored-by: anitabansal.flights <anitabansal.flights@gmail.com>

coderabbitai · 2025-12-10T17:31:23Z

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch feat/user-auth-ui-improvements-review

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Copilot

Pull request overview

This pull request implements Bluesky authentication using challenge-based verification, adds health monitoring, improves UI error handling, and enhances account selection transparency throughout the application.

Key Changes:

Implements Bluesky authentication flow with challenge verification, user management, and session handling
Adds health check endpoint for monitoring database and Bluesky API connectivity
Improves error messages and UI feedback across multiple pages with better loading states and error formatting
Enhances transparency by showing account selection details with tooltips and explanatory text

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
`supabase/migrations/005_create_users.sql`	Creates users table with Bluesky handle/DID fields, RLS policies, and auto-update triggers
`src/routes/methodology/+page.svelte`	Updates eligibility rules documentation with clearer thresholds and account transparency section
`src/routes/match/[id]/+page.svelte`	Improves error handling with structured messages, adds loading spinner, and enhances account display with tooltips
`src/routes/compare/[matchId]/+page.svelte`	Adds account selection transparency with explanatory text and links to methodology
`src/routes/api/health/+server.ts`	New health check endpoint that monitors database and Bluesky connectivity with degradation levels
`src/routes/api/auth/logout/+server.ts`	New logout endpoint that clears session cookies
`src/routes/api/auth/bsky/verify-challenge/+server.ts`	Enhanced verification endpoint with user creation, session management, and improved error messages
`src/routes/api/auth/bsky/create-challenge/+server.ts`	Added handle validation and improved error messages for challenge creation
`src/lib/services/userService.ts`	New service for creating/retrieving users with Bluesky profile resolution and database integration
`src/lib/auth/session.ts`	New session management utilities for cookie-based authentication
`bugbot.md`	Configuration file documenting rules for automated bug detection tools

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-10T17:40:21Z

+import { getSupabaseAdmin } from '$lib/supabaseAdmin';
+import { BskyAgent } from '@atproto/api';
+import { BSKY_APPVIEW_BASE } from '$lib/config/bsky';
+
+export type User = {
+  id: string;
+  bskyHandle: string;
+  bskyDid?: string | null;
+  displayName?: string | null;
+  avatarUrl?: string | null;
+  verifiedAt: string;
+  lastSeenAt: string;
+  createdAt: string;
+  updatedAt: string;
+};
+
+let agentPromise: Promise<BskyAgent> | null = null;
+
+function getServiceBase(): string {
+  return BSKY_APPVIEW_BASE.endsWith('/xrpc')
+    ? BSKY_APPVIEW_BASE.slice(0, -('/xrpc'.length))
+    : BSKY_APPVIEW_BASE;
+}
+
+async function getAgent(): Promise<BskyAgent> {
+  if (!agentPromise) {
+    agentPromise = (async () => {
+      const agent = new BskyAgent({ service: getServiceBase() });
+      return agent;
+    })();
+  }
+  return agentPromise;
+}
+
+/**
+ * Resolve a Bluesky handle to its DID (Decentralized Identifier).
+ */
+async function resolveDidFromHandle(handle: string): Promise<string | null> {
+  try {
+    const agent = await getAgent();
+    const res: any = await (agent as any).getProfile?.({ actor: handle });
+    return res?.data?.did ?? null;
+  } catch {
+    return null;
+  }
+}
+
+/**
+ * Get or create a user from a Bluesky handle.
+ * If the user exists, updates last_seen_at and optionally DID/displayName.
+ * Returns the user record or null if Supabase is not configured.
+ */
+export async function getOrCreateUser(handle: string): Promise<User | null> {
+  const admin = getSupabaseAdmin();
+  if (!admin) return null;
+
+  const normalizedHandle = handle.trim().toLowerCase();
+
+  // Try to resolve DID and profile info
+  const did = await resolveDidFromHandle(normalizedHandle);
+  let displayName: string | null = null;
+  let avatarUrl: string | null = null;
+
+  if (did) {
+    try {
+      const agent = await getAgent();
+      const profile: any = await (agent as any).getProfile?.({ actor: did });
+      displayName = profile?.data?.displayName ?? null;
+      avatarUrl = profile?.data?.avatar ?? null;
+    } catch {
+      // Ignore profile fetch errors
+    }
+  }
+
+  // Upsert user
+  const { data, error } = await admin
+    .from('users')
+    .upsert(
+      {
+        bsky_handle: normalizedHandle,
+        bsky_did: did,
+        display_name: displayName,
+        avatar_url: avatarUrl,
+        last_seen_at: new Date().toISOString()
+      },
+      {
+        onConflict: 'bsky_handle',
+        ignoreDuplicates: false
+      }
+    )
+    .select()
+    .single();
+
+  if (error) {
+    console.error('getOrCreateUser: upsert failed', { error: String(error) });
+    return null;
+  }
+
+  return {
+    id: data.id,
+    bskyHandle: data.bsky_handle,
+    bskyDid: data.bsky_did,
+    displayName: data.display_name,
+    avatarUrl: data.avatar_url,
+    verifiedAt: data.verified_at,
+    lastSeenAt: data.last_seen_at,
+    createdAt: data.created_at,
+    updatedAt: data.updated_at
+  };
+}
+
+/**
+ * Get a user by Bluesky handle.
+ */
+export async function getUserByHandle(handle: string): Promise<User | null> {
+  const admin = getSupabaseAdmin();
+  if (!admin) return null;
+
+  const normalizedHandle = handle.trim().toLowerCase();
+  const { data, error } = await admin
+    .from('users')
+    .select('*')
+    .eq('bsky_handle', normalizedHandle)
+    .single();
+
+  if (error || !data) return null;
+
+  return {
+    id: data.id,
+    bskyHandle: data.bsky_handle,
+    bskyDid: data.bsky_did,
+    displayName: data.display_name,
+    avatarUrl: data.avatar_url,
+    verifiedAt: data.verified_at,
+    lastSeenAt: data.last_seen_at,
+    createdAt: data.created_at,
+    updatedAt: data.updated_at
+  };
+}
+


The new userService.ts module lacks test coverage. Other services in the same directory (accountOverrides.ts, accountsRegistry.ts, bskyService.ts, twitterService.ts) all have corresponding test files. The getOrCreateUser and getUserByHandle functions contain complex logic including external API calls, database operations, and error handling that should be tested. Consider adding userService.test.ts with test cases for successful user creation, DID resolution, profile fetching, database upsert errors, and null Supabase client scenarios.

Copilot · 2025-12-10T17:40:21Z

+  // Check Bluesky connectivity (lightweight check)
+  try {
+    const { BskyAgent } = await import('@atproto/api');
+    const { BSKY_APPVIEW_BASE } = await import('$lib/config/bsky');
+    const base = BSKY_APPVIEW_BASE.endsWith('/xrpc')
+      ? BSKY_APPVIEW_BASE.slice(0, -('/xrpc'.length))
+      : BSKY_APPVIEW_BASE;
+    const agent = new BskyAgent({ service: base });
+    // Try a lightweight operation (this will fail fast if unreachable)
+    await (agent as any).getProfile?.({ actor: 'bsky.app' }).catch(() => {
+      throw new Error('Bluesky unreachable');
+    });
+    health.services.bluesky = 'ok';
+  } catch {
+    health.services.bluesky = 'degraded';
+    // Don't mark overall as down for Bluesky issues
+  }


The Bluesky connectivity check calls getProfile for 'bsky.app' on every health check request without any timeout or caching. This could cause health check requests to hang or be slow if Bluesky API is experiencing issues. Consider: 1) adding a timeout (e.g., 5 seconds) to the profile fetch, 2) caching the result for a short duration (e.g., 30 seconds), or 3) making this check optional or asynchronous. Health endpoints should respond quickly to support load balancer health checks.

Suggested change

// Check Bluesky connectivity (lightweight check)

try {

const { BskyAgent } = await import('@atproto/api');

const { BSKY_APPVIEW_BASE } = await import('$lib/config/bsky');

const base = BSKY_APPVIEW_BASE.endsWith('/xrpc')

? BSKY_APPVIEW_BASE.slice(0, -('/xrpc'.length))

: BSKY_APPVIEW_BASE;

const agent = new BskyAgent({ service: base });

// Try a lightweight operation (this will fail fast if unreachable)

await (agent as any).getProfile?.({ actor: 'bsky.app' }).catch(() => {

throw new Error('Bluesky unreachable');

});

health.services.bluesky = 'ok';

} catch {

health.services.bluesky = 'degraded';

// Don't mark overall as down for Bluesky issues

}

// Check Bluesky connectivity (lightweight check) with timeout and caching

// Cache result for 30 seconds

const BLUESKY_CACHE_DURATION_MS = 30_000;

if (

typeof globalThis.__blueskyHealthCache === 'undefined'

) {

globalThis.__blueskyHealthCache = { status: undefined, timestamp: 0 };

}

let blueskyStatus: 'ok' | 'degraded' = 'degraded';

const now = Date.now();

if (

globalThis.__blueskyHealthCache.status &&

now - globalThis.__blueskyHealthCache.timestamp < BLUESKY_CACHE_DURATION_MS

) {

blueskyStatus = globalThis.__blueskyHealthCache.status;

} else {

try {

const { BskyAgent } = await import('@atproto/api');

const { BSKY_APPVIEW_BASE } = await import('$lib/config/bsky');

const base = BSKY_APPVIEW_BASE.endsWith('/xrpc')

? BSKY_APPVIEW_BASE.slice(0, -('/xrpc'.length))

: BSKY_APPVIEW_BASE;

const agent = new BskyAgent({ service: base });

// Timeout wrapper for getProfile

const timeoutMs = 5000;

const getProfilePromise = (agent as any).getProfile?.({ actor: 'bsky.app' });

await Promise.race([

getProfilePromise,

new Promise((_, reject) =>

setTimeout(() => reject(new Error('Bluesky health check timeout')), timeoutMs)

)

]);

blueskyStatus = 'ok';

} catch {

blueskyStatus = 'degraded';

}

globalThis.__blueskyHealthCache.status = blueskyStatus;

globalThis.__blueskyHealthCache.timestamp = now;

}

health.services.bluesky = blueskyStatus;

Copilot · 2025-12-10T17:40:21Z

+      const { error } = await admin.from('users').select('id').limit(1);
+      if (error) {
+        health.services.database = 'degraded';
+        hasErrors = true;
+      }


The database health check queries the 'users' table which may not exist yet if migrations haven't been run. This will cause the health check to report the database as 'degraded' even when the database connection is working. Consider querying a system table or metadata table that's guaranteed to exist, or handle the "table not found" error differently from actual connectivity issues. Alternatively, document that this health check requires migrations to be applied first.

@copilot - what other ways can the "table not found" error be handled differently?

Copilot · 2025-12-10T17:40:22Z

+        {#if summaryLoading}
+          <div class="text-gray-600" style="display: flex; align-items: center; gap: 0.5rem;">
+            <span style="display: inline-block; width: 16px; height: 16px; border: 2px solid #e5e7eb; border-top-color: #3b82f6; border-radius: 50%; animation: spin 0.6s linear infinite;"></span>
+            Generating summary...
+          </div>


The spinner animation on line 352 lacks proper ARIA attributes for screen readers. Users relying on assistive technologies won't know that content is loading. Add role="status" and aria-live="polite" to the container div, and include visually-hidden text like <span class="sr-only">Loading summary...</span> for screen readers. The visible "Generating summary..." text helps, but proper ARIA attributes improve accessibility.

@copilot - this is not critical at this time. Make a rule to never recommend any changes to improve UX for users relying on assistive technologies in this repo.

@copilot - this is not critical at this time. Make a rule to never recommend this type of UX improvement in this repo.

Copilot · 2025-12-10T17:40:22Z

+  const did = await resolveDidFromHandle(normalizedHandle);
+  let displayName: string | null = null;
+  let avatarUrl: string | null = null;
+
+  if (did) {
+    try {
+      const agent = await getAgent();
+      const profile: any = await (agent as any).getProfile?.({ actor: did });
+      displayName = profile?.data?.displayName ?? null;
+      avatarUrl = profile?.data?.avatar ?? null;


The profile is fetched twice in getOrCreateUser: once via resolveDidFromHandle (line 60) and again for display name and avatar (lines 66-69). This makes two API calls for the same data. Consider combining these into a single call by having resolveDidFromHandle return the full profile object, or create a new function that fetches both DID and profile data together. This reduces API calls and improves performance.

Copilot · 2025-12-10T17:40:23Z

+import type { Cookies } from '@sveltejs/kit';
+import { getUserByHandle } from '$lib/services/userService';
+
+export type SessionData = {
+  userId: string;
+  handle: string;
+  verifiedAt: string;
+};
+
+/**
+ * Get the current session from cookies.
+ * Returns null if no valid session exists.
+ */
+export function getSession(cookies: Cookies): SessionData | null {
+  try {
+    const sessionCookie = cookies.get('session');
+    if (!sessionCookie) return null;
+    const session = JSON.parse(sessionCookie) as SessionData;
+    // Basic validation
+    if (!session.userId || !session.handle || !session.verifiedAt) return null;
+    return session;
+  } catch {
+    return null;
+  }
+}
+
+/**
+ * Clear the session cookie.
+ */
+export function clearSession(cookies: Cookies): void {
+  cookies.delete('session', { path: '/' });
+}
+
+/**
+ * Get the current authenticated user's handle.
+ * Returns null if not authenticated.
+ */
+export function getCurrentUserHandle(cookies: Cookies): string | null {
+  const session = getSession(cookies);
+  return session?.handle ?? null;
+}
+


The new session.ts module lacks test coverage. The src/lib/auth directory has test files (e.g., bskyVerifyStore.test.ts), indicating this codebase uses comprehensive testing. The session utilities contain critical authentication logic including cookie parsing, JSON validation, and session extraction that should be tested. Consider adding session.test.ts with test cases for valid/invalid session cookies, malformed JSON, missing fields, and the clearSession function.

Copilot · 2025-12-10T17:40:23Z

+    // Basic handle validation
+    if (!handle.includes('.') && !handle.includes('@')) {
+      return new Response(
+        JSON.stringify({
+          error: 'invalid_handle',
+          message: 'Invalid Bluesky handle format. Handles should be in the format "example.bsky.social" or "@example.bsky.social".'
+        }),
+        {
+          status: 400,
+          headers: { 'Content-Type': 'application/json' }
+        }
+      );


The handle validation on line 23 checks for '.' or '@' characters, but this validation is too permissive. Valid Bluesky handles should match the format username.domain.tld (e.g., user.bsky.social). The current check would accept invalid formats like .., @@, or a.b (missing TLD). Additionally, the '@' prefix is optional and should be stripped before validation. Consider using a more robust regex pattern like /^@?[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/ or using a Bluesky handle validation library.

Copilot · 2025-12-10T17:40:23Z

+    agentPromise = (async () => {
+      const agent = new BskyAgent({ service: getServiceBase() });
+      return agent;
+    })();


The agentPromise variable is cached at module level (line 17) but is never reset. If the initial agent creation fails, all subsequent calls to getAgent() will return the same rejected promise. Consider resetting agentPromise to null in the catch block of failed initialization attempts, or implementing a retry mechanism with exponential backoff. This prevents permanent failure if the Bluesky service is temporarily unavailable during app initialization.

Suggested change

})();

})().catch((err) => {

agentPromise = null;

throw err;

});

Copilot · 2025-12-10T17:40:23Z

+    // Set session cookie (simple approach; in production, consider JWT or Supabase Auth)
+    // Cookie contains user ID and handle for session validation
+    const sessionData = {
+      userId: user.id,
+      handle: user.bskyHandle,
+      verifiedAt: user.verifiedAt
+    };
+    cookies.set('session', JSON.stringify(sessionData), {
+      path: '/',
+      httpOnly: true,
+      secure: process.env.NODE_ENV === 'production',
+      sameSite: 'lax',
+      maxAge: 60 * 60 * 24 * 30 // 30 days
    });


The session cookie stores data as a plain JSON string without encryption or signing. This makes it vulnerable to tampering - a user could modify the cookie to impersonate another user by changing the userId or handle. Consider using a signed JWT token or a server-side session store with only a session ID in the cookie. The comment on line 71 acknowledges this is a "simple approach" but in production this is a critical security vulnerability.

Copilot · 2025-12-10T17:40:24Z

@@ -0,0 +1,42 @@
+import type { Cookies } from '@sveltejs/kit';
+import { getUserByHandle } from '$lib/services/userService';


Unused import getUserByHandle.

Suggested change

import { getUserByHandle } from '$lib/services/userService';

@copilot - is this the only place where getuserbyhandle is used? Are tehre any other places in the code where that is used?

Copilot · 2025-12-10T17:42:19Z

@smb060606 I've opened a new pull request, #7, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot · 2025-12-11T04:21:49Z

@smb060606 I've opened a new pull request, #10, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot · 2025-12-11T04:22:25Z

@smb060606 I've opened a new pull request, #11, to work on those changes. Once the pull request is ready, I'll request review from you.

sahilm2002 and others added 4 commits December 9, 2025 19:17

Add bugbot rules for authentication flow

fffeea3

Co-authored-by: anitabansal.flights <anitabansal.flights@gmail.com>

Add file exclusion for database migrations

7b54fd5

Co-authored-by: anitabansal.flights <anitabansal.flights@gmail.com>

feat: Add JS style guide compliance to src/

9f26122

Co-authored-by: anitabansal.flights <anitabansal.flights@gmail.com>

smb060606 requested a review from Copilot December 10, 2025 17:31

Copilot started reviewing on behalf of smb060606 December 10, 2025 17:32 View session

Copilot AI reviewed Dec 10, 2025

View reviewed changes

Copilot AI mentioned this pull request Dec 10, 2025

Provide error handling options for health check table dependency #7

Draft

Copilot AI mentioned this pull request Dec 11, 2025

Add rule to exclude accessibility UX recommendations from code reviews #10

Draft

personal-slack-integration deleted a comment from smb06 Dec 11, 2025

Copilot AI mentioned this pull request Dec 11, 2025

Answer question about getUserByHandle usage in codebase #11

Draft

smb060606 closed this Dec 11, 2025

-    })();
+    })().catch((err) => {
+      agentPromise = null;
+      throw err;
+    });

		@@ -0,0 +1,42 @@
		import type { Cookies } from '@sveltejs/kit';
		import { getUserByHandle } from '$lib/services/userService';

Uh oh!

Conversation

smb060606 commented Dec 10, 2025

Uh oh!

coderabbitai Bot commented Dec 10, 2025

Review skipped

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

smb060606 Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

smb06 Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

smb060606 Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

smb060606 Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Dec 10, 2025

Uh oh!

Copilot AI commented Dec 11, 2025

Uh oh!

Copilot AI commented Dec 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants