feedback

Author: brendan-kellam

CLAUDE.md

@@ -96,7 +96,7 @@ When implementing a new API route, ask the user whether it should be part of the
 3. Add the endpoint to the relevant group in the `API Reference` tab of `docs/docs.json`.
 4. Regenerate the OpenAPI spec by running `yarn workspace @sourcebot/web openapi:generate`.
 
-Route handlers should validate inputs using Zod schemas.
+Route handlers should validate inputs using Zod schemas. Put coercion, defaults, minimums, maximums, and cross-field validation in the schema instead of scattering parsing logic through the handler.
 
 **Query parameters** (GET requests):
 
@@ -105,8 +105,9 @@ import { queryParamsSchemaValidationError, serviceErrorResponse } from "@/lib/se
 import { z } from "zod";
 
 const myQueryParamsSchema = z.object({
-    q: z.string().default(''),
+    repo: z.string(),
     page: z.coerce.number().int().positive().default(1),
+    perPage: z.coerce.number().int().positive().max(100).default(50),
 });
 
 export const GET = apiHandler(async (request: NextRequest) => {
@@ -124,11 +125,42 @@ export const GET = apiHandler(async (request: NextRequest) => {
         );
     }
 
-    const { q, page } = parsed.data;
+    const { page, perPage, ...searchParams } = parsed.data;
+    const skip = (page - 1) * perPage;
     // ... rest of handler
 });
 ```
 
+**Search query parameters**:
+
+```ts
+import { queryParamsSchemaValidationError, serviceErrorResponse } from "@/lib/serviceError";
+import { z } from "zod";
+
+const searchMembersQueryParamsSchema = z.object({
+    query: z.string().default(''),
+});
+
+export const GET = apiHandler(async (request: NextRequest) => {
+    const rawParams = Object.fromEntries(
+        Object.keys(searchMembersQueryParamsSchema.shape).map(key => [
+            key,
+            request.nextUrl.searchParams.get(key) ?? undefined
+        ])
+    );
+    const parsed = searchMembersQueryParamsSchema.safeParse(rawParams);
+
+    if (!parsed.success) {
+        return serviceErrorResponse(
+            queryParamsSchemaValidationError(parsed.error)
+        );
+    }
+
+    const { query } = parsed.data;
+    // ... use query in the database/search call
+});
+```
+
 **Request body** (POST/PUT/PATCH requests):
 
 ```ts
@@ -155,6 +187,13 @@ export const POST = apiHandler(async (request: NextRequest) => {
 });
 ```
 
+Use `safeParseAsync` when the schema has async refinements, as in search routes:
+
+```ts
+const body = await request.json();
+const parsed = await searchRequestSchema.safeParseAsync(body);
+```
+
 ## Data Fetching
 
 For GET requests, prefer using API routes with react-query over server actions. This provides caching benefits and better control over data refetching.

docs/docs/configuration/auth/scim.mdx

@@ -72,7 +72,7 @@ When a pending user signs in, Sourcebot moves them to **Active** and they count
 
 When your identity provider deactivates a user by sending `active: false`, Sourcebot marks the user as **Suspended**. Suspended users cannot access the organization, and Sourcebot revokes their active sessions, API keys, and OAuth tokens.
 
-If your identity provider reactivates the user by sending `active: true`, Sourcebot restores their membership. Users who had already become active return to active access; users who had never signed in return to pending.
+If your identity provider reactivates the user by sending `active: true`, Sourcebot restores their membership as **Pending**. They become **Active** and billable again only after they sign in and access the organization.
 
 
 ## Roles
@@ -106,6 +106,6 @@ Additional attributes may be sent by your identity provider, but Sourcebot ignor
         Sourcebot supports SCIM 2.0.
     </Accordion>
     <Accordion title="When do SCIM-created users become billable seats?">
-        SCIM-created users become billable seats after they sign in and access the organization for the first time. Until then, they appear as pending and do not count toward billing. Suspended users also do not count toward billing.
+        SCIM-created or reactivated users become billable seats after they sign in and access the organization. Until then, they appear as pending and do not count toward billing. Suspended users also do not count toward billing.
     </Accordion>
 </AccordionGroup>

packages/db/prisma/migrations/20260619214548_add_scim_users_support/migration.sql

@@ -23,7 +23,7 @@ CREATE UNIQUE INDEX "ScimToken_hash_key" ON "ScimToken"("hash");
 CREATE INDEX "ScimToken_orgId_idx" ON "ScimToken"("orgId");
 
 -- CreateIndex
-CREATE INDEX "UserToOrg_orgId_scimExternalId_idx" ON "UserToOrg"("orgId", "scimExternalId");
+CREATE UNIQUE INDEX "UserToOrg_orgId_scimExternalId_key" ON "UserToOrg"("orgId", "scimExternalId");
 
 -- AddForeignKey
 ALTER TABLE "ScimToken" ADD CONSTRAINT "ScimToken_orgId_fkey" FOREIGN KEY ("orgId") REFERENCES "Org"("id") ON DELETE CASCADE ON UPDATE CASCADE;

packages/db/prisma/schema.prisma

@@ -416,7 +416,7 @@ model UserToOrg {
   lastActiveAt DateTime?
 
   @@id([orgId, userId])
-  @@index([orgId, scimExternalId])
+  @@unique([orgId, scimExternalId])
 }
 
 model ApiKey {

packages/web/src/app/(app)/chats/chatsPage.tsx

@@ -102,7 +102,7 @@ function getColumns(context: ColumnsContext): ColumnDef<Chat>[] {
         },
         {
             id: "actions",
-            meta: { width: "50px" },
+            meta: { className: "w-[50px]" },
             cell: ({ row }) => <ChatRowActions chat={row.original} />,
         },
     ];

packages/web/src/app/(app)/settings/apiKeys/layout.tsx

@@ -3,8 +3,9 @@ import { OrgRole } from "@sourcebot/db";
 import { env } from "@sourcebot/shared";
 import { authenticatedPage } from "@/middleware/authenticatedPage";
 import { SettingsContainer } from "../components/settingsContainer";
+import { ReactNode } from "react";
 
-export default authenticatedPage<{ children: React.ReactNode }>(async ({ role }, { children }) => {
+export default authenticatedPage<{ children: ReactNode }>(async ({ role }, { children }) => {
     if (env.DISABLE_API_KEY_USAGE_FOR_NON_OWNER_USERS === 'true' && role !== OrgRole.OWNER) {
         return notFound();
     }

packages/web/src/app/(app)/settings/members/membersTable.tsx

@@ -320,8 +320,11 @@ const getColumns = (actionContext: Omit<MembersTableActionsProps, "row">): Colum
         header: ({ column }) => <SortableHeader column={column}>Joined</SortableHeader>,
         cell: ({ row }) => {
             const r = row.original;
-            const date = r.kind === "member" ? r.joinedAt : r.createdAt;
-            return <DisplayDate date={date} />;
+            if (r.kind !== "member") {
+                return <span className="text-sm text-muted-foreground">—</span>;
+            }
+
+            return <DisplayDate date={r.joinedAt} />;
         },
     },
     {
@@ -332,7 +335,7 @@ const getColumns = (actionContext: Omit<MembersTableActionsProps, "row">): Colum
         header: ({ column }) => <SortableHeader column={column}>Last seen</SortableHeader>,
         cell: ({ row }) => {
             const r = row.original;
-            if (r.kind !== "member") {
+            if (r.kind !== "member" || r.suspendedAt !== null) {
                 return <span className="text-sm text-muted-foreground">—</span>;
             }
             if (!r.lastActiveAt) {

packages/web/src/app/(app)/settings/security/components/scimProvisioningSettings.tsx

@@ -101,13 +101,18 @@ export function ScimProvisioningSettings({ baseUrl, tokens }: ScimProvisioningSe
     };
 
     const handleRevokeToken = async (name: string) => {
-        const result = await revokeScimToken(name);
-        if (isServiceError(result)) {
-            toast({ title: "Error", description: `Failed to revoke SCIM token: ${result.message}`, variant: "destructive" });
-            return;
+        try {
+            const result = await revokeScimToken(name);
+            if (isServiceError(result)) {
+                toast({ title: "Error", description: `Failed to revoke SCIM token: ${result.message}`, variant: "destructive" });
+                return;
+            }
+            router.refresh();
+            toast({ description: "SCIM token revoked" });
+        } catch (error) {
+            console.error(error);
+            toast({ title: "Error", description: `Failed to revoke SCIM token: ${error}`, variant: "destructive" });
         }
-        router.refresh();
-        toast({ description: "SCIM token revoked" });
     };
 
     const sortedTokens = useMemo(

packages/web/src/app/(app)/settings/security/page.tsx

@@ -20,6 +20,7 @@ import { SettingsCardGroup } from "../components/settingsCard";
 import { Alert, AlertDescription } from "@/components/ui/alert";
 import { Info } from "lucide-react";
 import { isScimEnabled } from "@/features/scim/utils";
+import { ServiceErrorException } from "@/lib/serviceError";
 
 export default authenticatedPage(async ({ org }) => {
     const anonymousAccessEnabled = await isAnonymousAccessEnabled();
@@ -29,8 +30,11 @@ export default authenticatedPage(async ({ org }) => {
 
     const hasScimEntitlement = await hasEntitlement("scim");
     const scimBaseUrl = `${env.AUTH_URL.replace(/\/$/, '')}/scim/v2`;
-    const scimTokensResult = hasScimEntitlement ? await getScimTokens() : [];
-    const scimTokens = isServiceError(scimTokensResult) ? [] : scimTokensResult;
+    const scimTokens = hasScimEntitlement ? await getScimTokens() : [];
+    if (isServiceError(scimTokens)) {
+        throw new ServiceErrorException(scimTokens);
+    }
+
     const scimEnabled = await isScimEnabled(org)
 
 

packages/web/src/app/api/(server)/ee/scim/v2/Users/[id]/route.ts

@@ -33,6 +33,17 @@ const applyActive = async (orgId: number, userId: string, current: boolean, next
     return null;
 };
 
+const ensureEmailAvailable = async (prisma: ScimAuthContext['prisma'], userId: string, email: string): Promise<Response | null> => {
+    const existing = await prisma.user.findUnique({
+        where: { email },
+        select: { id: true },
+    });
+    if (existing && existing.id !== userId) {
+        return scimError(409, 'User email is already in use', 'uniqueness');
+    }
+    return null;
+};
+
 // eslint-disable-next-line authz/require-auth-wrapper -- SCIM bearer auth via withScimAuth
 export const GET = apiHandler(async (request: NextRequest, { params }: { params: Promise<{ id: string }> }) =>
     withScimAuth(request, async ({ org, prisma }) => {
@@ -61,16 +72,21 @@ export const PUT = apiHandler(async (request: NextRequest, { params }: { params:
 
         const name = payload.name?.formatted ?? payload.displayName ?? undefined;
         const email = resolveEmail(payload);
-        await prisma.user.update({
-            where: { id },
-            data: { name, email },
-        });
+        const emailError = await ensureEmailAvailable(prisma, id, email);
+        if (emailError) {
+            return emailError;
+        }
 
         const activeError = await applyActive(org.id, id, membership.suspendedAt == null, coerceActive(payload.active));
         if (activeError) {
             return activeError;
         }
 
+        await prisma.user.update({
+            where: { id },
+            data: { name, email },
+        });
+
         const refreshed = await loadMembership(prisma, org.id, id);
         return scimJson(toScimUser(refreshed!), 200);
     }));
@@ -95,6 +111,18 @@ export const PATCH = apiHandler(async (request: NextRequest, { params }: { param
         // ignored rather than rejected, per the SCIM lenient-parsing convention.
         const changes = parseScimPatchOperations(parsed.data.Operations);
 
+        if (changes.email !== undefined) {
+            const emailError = await ensureEmailAvailable(prisma, id, changes.email);
+            if (emailError) {
+                return emailError;
+            }
+        }
+
+        const activeError = await applyActive(org.id, id, membership.suspendedAt == null, changes.active);
+        if (activeError) {
+            return activeError;
+        }
+
         if (changes.name !== undefined || changes.email !== undefined) {
             await prisma.user.update({
                 where: { id },
@@ -105,11 +133,6 @@ export const PATCH = apiHandler(async (request: NextRequest, { params }: { param
             });
         }
 
-        const activeError = await applyActive(org.id, id, membership.suspendedAt == null, changes.active);
-        if (activeError) {
-            return activeError;
-        }
-
         const refreshed = await loadMembership(prisma, org.id, id);
         return scimJson(toScimUser(refreshed!), 200);
     }));