Merge pull request #45 from yusufipk/worktree-fix-test-findings

fix: close the findings the test suite surfaced

Dockerfile

@@ -6,6 +6,14 @@ COPY package.json bun.lock ./
 COPY prisma ./prisma
 RUN bun install --frozen-lockfile
 
+# The tree the runner ships. The build needs eslint, vitest, playwright and the rest; the
+# running app does not, and copying the full tree put them all in the image.
+FROM base AS prod-deps
+COPY package.json bun.lock ./
+COPY prisma ./prisma
+RUN bun install --frozen-lockfile --production
+RUN bun run db:generate
+
 FROM deps AS build
 COPY app ./app
 COPY components ./components
@@ -45,7 +53,10 @@ COPY --from=build /app/lib ./lib
 COPY --from=build /app/app ./app
 COPY --from=build /app/components ./components
 COPY --from=build /app/types ./types
-COPY --from=build /app/node_modules ./node_modules
+COPY --from=prod-deps /app/node_modules ./node_modules
+# next.config.ts and prisma.config.ts are TypeScript, and both are loaded at startup, so
+# the compiler has to be present even though nothing else here needs it.
+COPY --from=deps /app/node_modules/typescript ./node_modules/typescript
 COPY --from=build /app/.next ./.next
 COPY --from=build /app/tsconfig.json ./tsconfig.json
 COPY --from=build /app/postcss.config.mjs ./postcss.config.mjs

app/(auth)/login/login-form.tsx

@@ -124,10 +124,15 @@ function LoginFormInner({ googleEnabled, githubEnabled }: LoginFormInnerProps) {
         </CardDescription>
       </CardHeader>
       <CardContent>
+        {/*
+          `?registered=true` is only ever reached when email verification is off: the
+          register page sends a user who has to verify to /verify-email instead. Telling
+          this one to go and check a mailbox pointed them at a message that never arrives,
+          on a self-hosted deployment without SMTP, which is the documented default.
+        */}
         {showSuccess && (
           <div className="p-3 rounded-md bg-green-500/10 text-green-600 text-sm mb-4">
-            Account created successfully! Please check your email to verify your address before
-            signing in.
+            Account created successfully! You can sign in now.
           </div>
         )}
 

app/(dashboard)/projects/[projectId]/settings/project-settings-page-client.tsx

@@ -438,20 +438,32 @@ export default function ProjectSettingsPageClient({ projectId }: ProjectSettings
                         <input
                           type="color"
                           value={editTagColor}
+                          aria-label={`Tag colour for ${tag.name}`}
                           onChange={(e) => setEditTagColor(e.target.value)}
                           className="w-8 h-8 rounded cursor-pointer border-0"
                         />
                         <Input
                           value={editTagName}
+                          aria-label={`Tag name for ${tag.name}`}
                           onChange={(e) => setEditTagName(e.target.value)}
                           className="flex-1 h-8"
                           onKeyDown={(e) => e.key === 'Enter' && handleUpdateTag(tag.id)}
                         />
-                        <Button size="sm" variant="ghost" onClick={() => handleUpdateTag(tag.id)}>
-                          <Save className="h-4 w-4" />
+                        <Button
+                          size="sm"
+                          variant="ghost"
+                          aria-label={`Save tag ${tag.name}`}
+                          onClick={() => handleUpdateTag(tag.id)}
+                        >
+                          <Save className="h-4 w-4" aria-hidden="true" />
                         </Button>
-                        <Button size="sm" variant="ghost" onClick={() => setEditingTagId(null)}>
-                          <X className="h-4 w-4" />
+                        <Button
+                          size="sm"
+                          variant="ghost"
+                          aria-label={`Cancel editing tag ${tag.name}`}
+                          onClick={() => setEditingTagId(null)}
+                        >
+                          <X className="h-4 w-4" aria-hidden="true" />
                         </Button>
                       </>
                     ) : (
@@ -476,9 +488,10 @@ export default function ProjectSettingsPageClient({ projectId }: ProjectSettings
                           size="sm"
                           variant="ghost"
                           className="text-destructive hover:text-destructive"
+                          aria-label={`Delete tag ${tag.name}`}
                           onClick={() => handleDeleteTag(tag.id)}
                         >
-                          <Trash2 className="h-4 w-4" />
+                          <Trash2 className="h-4 w-4" aria-hidden="true" />
                         </Button>
                       </>
                     )}

app/(dashboard)/projects/[projectId]/videos/new/new-video-page-client.tsx

@@ -363,6 +363,7 @@ export default function NewVideoPageClient({
         failCount += 1;
         const message = error instanceof Error ? error.message : 'Upload failed';
         setSubmitError(`${file.name}: ${message}`);
+        setUploadStatus('');
       }
     }
 
@@ -447,6 +448,9 @@ export default function NewVideoPageClient({
     } catch (error: unknown) {
       console.error('Failed to add video:', error);
       setSubmitError(error instanceof Error ? error.message : 'An unexpected error occurred');
+      // Cleared on the failure path too. Leaving it set showed the error above a stale
+      // "Initializing upload...", so the form claimed to be doing both at once.
+      setUploadStatus('');
     } finally {
       activeTusUploadRef.current = null;
       pendingUploadRef.current = null;

app/admin/page.tsx

@@ -275,6 +275,17 @@ export default async function AdminDashboardPage() {
                 <div className="text-2xl font-bold">{stripeStats.canceledUsers}</div>
               </CardContent>
             </Card>
+            {/* UNPAID, INCOMPLETE and INCOMPLETE_EXPIRED, which belonged to none of the
+                buckets above and so were counted nowhere. */}
+            <Card>
+              <CardHeader className="flex flex-row items-center justify-between space-y-0 pb-2">
+                <CardTitle className="text-sm font-medium">Unpaid or Incomplete</CardTitle>
+                <AlertCircle className="h-4 w-4 text-muted-foreground" />
+              </CardHeader>
+              <CardContent>
+                <div className="text-2xl font-bold">{stripeStats.otherStatusUsers}</div>
+              </CardContent>
+            </Card>
           </div>
         </>
       )}

app/api/approvals/[requestId]/cancel/route.ts

@@ -40,11 +40,7 @@ export async function POST(request: NextRequest, { params }: RouteParams) {
     });
     if (!approvalRequest) return apiErrors.notFound('Approval request');
 
-    const access = await checkProjectAccess(
-      approvalRequest.version.video.project,
-      session.user.id,
-      { intent: 'manage' }
-    );
+    const access = await checkProjectAccess(approvalRequest.version.video.project, session.user.id);
     const canCancel = approvalRequest.requestedById === session.user.id || access.canEdit;
     if (!canCancel) return apiErrors.forbidden('Access denied');
 

app/api/comments/[commentId]/route.ts

@@ -133,7 +133,7 @@ export async function PATCH(request: NextRequest, { params }: RouteParams) {
 
     const project = comment.version.video.project;
     const userId = session?.user?.id ?? null;
-    const access = await checkProjectAccess(project, userId ?? undefined, { intent: 'manage' });
+    const access = await checkProjectAccess(project, userId ?? undefined);
     const isOwner = userId === project.ownerId;
     const isAuthor = !!userId && comment.authorId === userId;
     const guestIdentityId = !userId ? getGuestIdentityFromRequest(request) : null;
@@ -295,7 +295,7 @@ export async function DELETE(request: NextRequest, { params }: RouteParams) {
     const isAuthor = !!userId && comment.authorId === userId;
 
     // Project owners/admins and workspace admins can delete any comment
-    const access = userId ? await checkProjectAccess(project, userId, { intent: 'manage' }) : null;
+    const access = userId ? await checkProjectAccess(project, userId) : null;
     const isPrivilegedUser = !!access?.canEdit;
 
     let canDelete = isAuthor || isPrivilegedUser;

app/api/projects/[projectId]/approval-candidates/route.ts

@@ -20,7 +20,7 @@ export async function GET(_request: NextRequest, { params }: RouteParams) {
     });
     if (!project) return apiErrors.notFound('Project');
 
-    const access = await checkProjectAccess(project, session.user.id, { intent: 'manage' });
+    const access = await checkProjectAccess(project, session.user.id);
     if (!access.canEdit) return apiErrors.forbidden('Access denied');
 
     const candidates = await getApprovalCandidatesForProject(projectId);

app/api/projects/[projectId]/members/[memberId]/route.ts

@@ -30,7 +30,7 @@ export async function PATCH(request: NextRequest, { params }: RouteParams) {
       return apiErrors.notFound('Project');
     }
 
-    const access = await checkProjectAccess(project, session.user.id, { intent: 'manage' });
+    const access = await checkProjectAccess(project, session.user.id);
     const isOwner = project.ownerId === session.user.id;
     const isAdmin = project.members[0]?.role === ProjectMemberRole.ADMIN;
 
@@ -93,7 +93,7 @@ export async function DELETE(request: NextRequest, { params }: RouteParams) {
       return apiErrors.notFound('Project');
     }
 
-    const access = await checkProjectAccess(project, session.user.id, { intent: 'manage' });
+    const access = await checkProjectAccess(project, session.user.id);
     const isOwner = project.ownerId === session.user.id;
     const isAdmin = project.members[0]?.role === ProjectMemberRole.ADMIN;
 

app/api/projects/[projectId]/members/invitations/[invitationId]/route.ts

@@ -30,7 +30,7 @@ export async function DELETE(request: NextRequest, { params }: RouteParams) {
       return apiErrors.notFound('Project');
     }
 
-    const access = await checkProjectAccess(project, session.user.id, { intent: 'manage' });
+    const access = await checkProjectAccess(project, session.user.id);
     const isOwner = project.ownerId === session.user.id;
     const isAdmin = project.members[0]?.role === ProjectMemberRole.ADMIN;
 

app/api/projects/[projectId]/members/route.ts

@@ -112,7 +112,7 @@ export async function POST(request: NextRequest, { params }: RouteParams) {
       return apiErrors.notFound('Project');
     }
 
-    const access = await checkProjectAccess(project, session.user.id, { intent: 'manage' });
+    const access = await checkProjectAccess(project, session.user.id);
     const isOwner = project.ownerId === session.user.id;
     const isAdmin = project.members[0]?.role === ProjectMemberRole.ADMIN;
 

app/api/projects/[projectId]/route.ts

@@ -103,7 +103,7 @@ export async function PATCH(request: NextRequest, { params }: RouteParams) {
       select: { id: true, ownerId: true, workspaceId: true, visibility: true },
     });
     const access = projectAccessTarget
-      ? await checkProjectAccess(projectAccessTarget, session.user.id, { intent: 'manage' })
+      ? await checkProjectAccess(projectAccessTarget, session.user.id)
       : null;
     if (!access?.canEdit) {
       return apiErrors.forbidden('Access denied');
@@ -181,7 +181,7 @@ export async function DELETE(request: NextRequest, { params }: RouteParams) {
       return apiErrors.notFound('Project');
     }
 
-    const access = await checkProjectAccess(project, session.user.id, { intent: 'delete' });
+    const access = await checkProjectAccess(project, session.user.id);
     if (!access.canDelete) {
       return apiErrors.forbidden('Only the project owner can delete it');
     }

app/api/projects/[projectId]/tags/[tagId]/route.ts

@@ -28,7 +28,7 @@ export async function PATCH(request: NextRequest, { params }: RouteParams) {
       return apiErrors.notFound('Project');
     }
 
-    const access = await checkProjectAccess(project, session.user.id, { intent: 'manage' });
+    const access = await checkProjectAccess(project, session.user.id);
     if (!access.canEdit) {
       return apiErrors.forbidden('Access denied');
     }
@@ -98,7 +98,7 @@ export async function DELETE(request: NextRequest, { params }: RouteParams) {
       return apiErrors.notFound('Project');
     }
 
-    const access = await checkProjectAccess(project, session.user.id, { intent: 'manage' });
+    const access = await checkProjectAccess(project, session.user.id);
     if (!access.canEdit) {
       return apiErrors.forbidden('Access denied');
     }

app/api/projects/[projectId]/tags/route.ts

@@ -97,7 +97,7 @@ export async function POST(request: NextRequest, { params }: RouteParams) {
       return apiErrors.notFound('Project');
     }
 
-    const access = await checkProjectAccess(project, session.user.id, { intent: 'manage' });
+    const access = await checkProjectAccess(project, session.user.id);
     if (!access.canEdit) {
       return apiErrors.forbidden('Access denied');
     }

app/api/projects/[projectId]/videos/[videoId]/route.ts

@@ -170,7 +170,7 @@ export async function PATCH(request: NextRequest, { params }: RouteParams) {
       return apiErrors.notFound('Video');
     }
 
-    const access = await checkProjectAccess(video.project, session.user.id, { intent: 'manage' });
+    const access = await checkProjectAccess(video.project, session.user.id);
     if (!access.canEdit) {
       return apiErrors.forbidden('Access denied');
     }
@@ -251,7 +251,7 @@ export async function DELETE(request: NextRequest, { params }: RouteParams) {
       return apiErrors.notFound('Video');
     }
 
-    const access = await checkProjectAccess(video.project, session.user.id, { intent: 'manage' });
+    const access = await checkProjectAccess(video.project, session.user.id);
     if (!access.canEdit) {
       return apiErrors.forbidden('Only project owner or admin can delete videos');
     }

app/api/projects/[projectId]/videos/[videoId]/versions/[versionId]/route.ts

@@ -32,7 +32,7 @@ async function getVersionWithAccess(
   }
 
   const project = version.video.project;
-  const access = await checkProjectAccess(project, userId, { intent: 'manage' });
+  const access = await checkProjectAccess(project, userId);
 
   return { version, canEdit: access.canEdit, isOwner: access.isOwner };
 }

app/api/projects/[projectId]/videos/[videoId]/versions/route.ts

@@ -74,7 +74,7 @@ export async function POST(request: NextRequest, { params }: RouteParams) {
       return apiErrors.notFound('Video');
     }
 
-    const access = await checkProjectAccess(video.project, session.user.id, { intent: 'manage' });
+    const access = await checkProjectAccess(video.project, session.user.id);
     if (!access.canEdit) {
       return apiErrors.forbidden('Access denied');
     }

app/api/projects/[projectId]/videos/bulk-delete/route.ts

@@ -5,7 +5,7 @@ import { db } from '@/lib/db';
 import { logCleanupWarnings } from '@/lib/cleanup-warnings';
 import { logError } from '@/lib/logger';
 import { rateLimit } from '@/lib/rate-limit';
-import { deleteProjectVideosWithCleanup } from '@/lib/video-delete';
+import { deleteProjectVideosWithCleanup, VideoStorageCleanupError } from '@/lib/video-delete';
 
 type RouteParams = { params: Promise<{ projectId: string }> };
 
@@ -32,7 +32,7 @@ export async function POST(request: NextRequest, { params }: RouteParams) {
       return apiErrors.notFound('Project');
     }
 
-    const access = await checkProjectAccess(project, session.user.id, { intent: 'manage' });
+    const access = await checkProjectAccess(project, session.user.id);
     if (!access.canEdit) {
       return apiErrors.forbidden('Only project owner or admin can delete videos');
     }
@@ -59,6 +59,17 @@ export async function POST(request: NextRequest, { params }: RouteParams) {
       if (error instanceof Error && error.message === 'VIDEO_NOT_FOUND') {
         return apiErrors.badRequest('One or more selected videos do not belong to this project');
       }
+      // Storage refused a delete, so nothing was removed and the videos are still there.
+      // Saying so lets the caller retry, which is the whole point of leaving the rows.
+      if (error instanceof VideoStorageCleanupError) {
+        logCleanupWarnings(
+          { entityType: 'video', entityId: `bulk:${normalizedIds.join(',')}` },
+          error.cleanupInput
+        );
+        return apiErrors.internalError(
+          'Could not delete the stored media for these videos. Nothing was deleted; please try again.'
+        );
+      }
       throw error;
     }
 

app/api/projects/[projectId]/videos/bunny-init/route.ts

@@ -27,7 +27,7 @@ async function getProjectWithEditAccess(projectId: string, userId: string) {
 
   if (!project) return null;
 
-  const access = await checkProjectAccess(project, userId, { intent: 'manage' });
+  const access = await checkProjectAccess(project, userId);
   const canEdit = access.canEdit;
 
   if (!canEdit) return null;

app/api/projects/[projectId]/videos/move/route.ts

@@ -38,7 +38,7 @@ export async function GET(request: NextRequest, { params }: RouteParams) {
       return apiErrors.notFound('Project');
     }
 
-    const access = await checkProjectAccess(project, userId, { intent: 'manage' });
+    const access = await checkProjectAccess(project, userId);
     if (!access.canEdit) {
       return apiErrors.forbidden('Access denied');
     }
@@ -141,8 +141,8 @@ export async function POST(request: NextRequest, { params }: RouteParams) {
     }
 
     const [sourceAccess, targetAccess] = await Promise.all([
-      checkProjectAccess(sourceProject, userId, { intent: 'manage' }),
-      checkProjectAccess(targetProject, userId, { intent: 'manage' }),
+      checkProjectAccess(sourceProject, userId),
+      checkProjectAccess(targetProject, userId),
     ]);
     if (!sourceAccess.canEdit) {
       return apiErrors.forbidden('You cannot move videos out of this project');

app/api/projects/[projectId]/videos/r2-complete/route.ts

@@ -26,7 +26,7 @@ async function getProjectWithEditAccess(projectId: string, userId: string) {
 
   if (!project) return null;
 
-  const access = await checkProjectAccess(project, userId, { intent: 'manage' });
+  const access = await checkProjectAccess(project, userId);
   if (!access.canEdit) return null;
 
   return project;

app/api/projects/[projectId]/videos/r2-init/route.ts

@@ -58,7 +58,7 @@ async function getProjectWithEditAccess(projectId: string, userId: string) {
 
   if (!project) return null;
 
-  const access = await checkProjectAccess(project, userId, { intent: 'manage' });
+  const access = await checkProjectAccess(project, userId);
   if (!access.canEdit) return null;
 
   return project;

app/api/projects/[projectId]/videos/route.ts

@@ -83,7 +83,7 @@ export async function POST(request: NextRequest, { params }: RouteParams) {
       return apiErrors.notFound('Project');
     }
 
-    const access = await checkProjectAccess(project, session.user.id, { intent: 'manage' });
+    const access = await checkProjectAccess(project, session.user.id);
     if (!access.canEdit) {
       return apiErrors.forbidden('Access denied');
     }

app/api/projects/route.ts

@@ -42,22 +42,15 @@ export async function GET(request: NextRequest) {
       return apiErrors.badRequest('Invalid page range. Offset must be 10000 or less.');
     }
 
-    // Build base filter: user is owner OR a member
+    // Build base filter: user is the project owner, a project member, or a member of the
+    // workspace the project lives in. The third branch used to be dropped whenever a
+    // workspaceId was supplied, so filtering by their own workspace showed a workspace
+    // member an empty list while the unfiltered call returned the same project.
     const baseFilter: Record<string, unknown> = {
       OR: [
         { ownerId: session.user.id },
         { members: { some: { userId: session.user.id } } },
-        // Also include projects in workspaces where the user is a workspace member
-        ...(workspaceId
-          ? []
-          : [
-              {
-                workspace: {
-                  owner: buildBillingAccessWhereInput(),
-                  members: { some: { userId: session.user.id } },
-                },
-              },
-            ]),
+        { workspace: { members: { some: { userId: session.user.id } } } },
       ],
       workspace: {
         owner: buildBillingAccessWhereInput(),

app/api/search/route.ts

@@ -2,6 +2,7 @@ import { NextRequest } from 'next/server';
 import { db } from '@/lib/db';
 import { auth } from '@/lib/auth';
 import { apiErrors, successResponse } from '@/lib/api-response';
+import { buildBillingAccessWhereInput } from '@/lib/billing';
 import { checkRateLimit, rateLimitHeaders, RATE_LIMIT_CONFIGS } from '@/lib/rate-limit';
 import { logError } from '@/lib/logger';
 
@@ -38,8 +39,15 @@ export async function GET(request: NextRequest) {
       return apiErrors.badRequest('Query too long.');
     }
 
-    // Access filter reused across queries
+    // Access filter reused across queries. The billing condition is the same one every
+    // other read path carries (GET /api/projects, checkProjectAccess): without it search
+    // kept returning project names, descriptions and video titles for a tenant whose
+    // access had otherwise been cut off, which is a lapsed-tenant surface no other read
+    // path leaves open.
+    const ownerWithBillingAccess = buildBillingAccessWhereInput();
+
     const projectAccessFilter = {
+      workspace: { owner: ownerWithBillingAccess },
       OR: [
         { ownerId: userId },
         { members: { some: { userId } } },
@@ -48,6 +56,7 @@ export async function GET(request: NextRequest) {
     };
 
     const workspaceAccessFilter = {
+      owner: ownerWithBillingAccess,
       OR: [{ ownerId: userId }, { members: { some: { userId } } }],
     };
 

app/api/versions/[versionId]/approvals/route.ts

@@ -84,9 +84,7 @@ export async function POST(request: NextRequest, { params }: RouteParams) {
     });
     if (!version) return apiErrors.notFound('Version');
 
-    const access = await checkProjectAccess(version.video.project, session.user.id, {
-      intent: 'manage',
-    });
+    const access = await checkProjectAccess(version.video.project, session.user.id);
     if (!access.canEdit) return apiErrors.forbidden('Access denied');
 
     const body = (await request.json().catch(() => ({}))) as {

app/api/versions/[versionId]/download/route.ts

@@ -308,6 +308,15 @@ export async function GET(request: NextRequest, { params }: RouteParams) {
     const canDownloadViaShareLink = shareAccess.hasAccess && shareAccess.canDownload;
     const canDownloadViaMembership = canDownloadProjectMedia(version.video.project, access);
     if (!canDownloadViaMembership && !canDownloadViaShareLink) {
+      // A caller with no relationship to the project at all is told the version does not
+      // exist, matching the comment export route: answering 403 for an id belonging to
+      // another tenant confirms that the id exists. Anyone who does have a relationship,
+      // including an owner whose billing has lapsed, already knows it exists and gets the
+      // more informative 403.
+      const belongsToProject = access.isOwner || access.isProjectMember || access.isWorkspaceMember;
+      if (!belongsToProject && !shareAccess.hasAccess) {
+        return apiErrors.notFound('Version');
+      }
       return apiErrors.forbidden('Access denied');
     }
 

app/api/videos/[videoId]/assets/[assetId]/download/route.ts

@@ -84,7 +84,15 @@ export async function GET(request: NextRequest, { params }: RouteParams) {
     const { videoId, assetId } = await params;
     const context = await getVideoAssetAccessContext(request, videoId, 'VIEW');
     if (!context) return apiErrors.notFound('Video');
-    if (!context.hasViewAccess) return apiErrors.forbidden('Access denied');
+    // A caller with no relationship to the project is told the video does not exist.
+    // Answering 403 for an id belonging to another tenant confirms that the id exists,
+    // and the comment export route already answers 404 for the identical shape. Somebody
+    // who does belong, including an owner whose billing lapsed, gets the 403.
+    if (!context.hasViewAccess) {
+      return context.viewerBelongsToProject
+        ? apiErrors.forbidden('Access denied')
+        : apiErrors.notFound('Video');
+    }
     if (!context.canDownloadAssets) {
       return apiErrors.forbidden('Downloads are disabled for this project');
     }

bun.lock

@@ -62,7 +62,6 @@
         "shadcn": "^3.8.3",
         "tailwindcss": "^4",
         "typescript": "^5",
-        "vite-tsconfig-paths": "^6.1.1",
         "vitest": "^4.1.10",
       },
     },
@@ -1315,8 +1314,6 @@
 
     "globalthis": ["[email protected]", "", { "dependencies": { "define-properties": "^1.2.1", "gopd": "^1.0.1" } }, "sha512-DpLKbNU4WylpxJykQujfCcwYWiV/Jhm50Goo0wrVILAv5jOr9d+H+UR3PhSCD2rCCEIg0uc+G+muBTwD54JhDQ=="],
 
-    "globrex": ["[email protected]", "", {}, "sha512-uHJgbwAMwNFf5mLst7IWLNg14x1CkeqglJb/K3doi4dw6q2IvAAmM/Y81kevy83wP+Sst+nutFTYOGg3d1lsxg=="],
-
     "gopd": ["[email protected]", "", {}, "sha512-ZUKRh6/kUFoAiTAtTYPZJ3hw9wNxx+BIBOijnlG9PnrJsCcSjs1wyyD6vJpaYtgnzDrKYRSqf3OO6Rfa93xsRg=="],
 
     "graceful-fs": ["[email protected]", "", {}, "sha512-RbJ5/jmFcNNCcDV5o9eTnBLJ/HszWV0P73bc+Ff4nS/rJj+YaS6IGyiOL0VoBYX+l1Wrl3k63h/KrH+nhJ0XvQ=="],
@@ -2083,8 +2080,6 @@
 
     "ts-morph": ["[email protected]", "", { "dependencies": { "@ts-morph/common": "~0.27.0", "code-block-writer": "^13.0.3" } }, "sha512-ztMO++owQnz8c/gIENcM9XfCEzgoGphTv+nKpYNM1bgsdOVC/jRZuEBf6N+mLLDNg68Kl+GgUZfOySaRiG1/Ug=="],
 
-    "tsconfck": ["[email protected]", "", { "peerDependencies": { "typescript": "^5.0.0" }, "optionalPeers": ["typescript"], "bin": { "tsconfck": "bin/tsconfck.js" } }, "sha512-ks6Vjr/jEw0P1gmOVwutM3B7fWxoWBL2KRDb1JfqGVawBmO5UsvmWOQFGHBPl5yxYz4eERr19E6L7NMv+Fej4w=="],
-
     "tsconfig-paths": ["[email protected]", "", { "dependencies": { "json5": "^2.2.2", "minimist": "^1.2.6", "strip-bom": "^3.0.0" } }, "sha512-NoZ4roiN7LnbKn9QqE1amc9DJfzvZXxF4xDavcOWt1BPkdx+m+0gJuPM+S0vCe7zTJMYUP0R8pO2XMr+Y8oLIg=="],
 
     "tslib": ["[email protected]", "", {}, "sha512-oJFu94HQb+KVduSUQL7wnpmqnfmLsOA/nAh6b6EH0wCEoK0/mPeXU6c3wKDV83MkOuHPRHtSXKKU99IBazS/2w=="],
@@ -2157,8 +2152,6 @@
 
     "vite": ["[email protected]", "", { "dependencies": { "lightningcss": "^1.32.0", "picomatch": "^4.0.5", "postcss": "^8.5.17", "rolldown": "~1.1.5", "tinyglobby": "^0.2.17" }, "optionalDependencies": { "fsevents": "~2.3.3" }, "peerDependencies": { "@types/node": "^20.19.0 || >=22.12.0", "@vitejs/devtools": "^0.3.0", "esbuild": "^0.27.0 || ^0.28.0", "jiti": ">=1.21.0", "less": "^4.0.0", "sass": "^1.70.0", "sass-embedded": "^1.70.0", "stylus": ">=0.54.8", "sugarss": "^5.0.0", "terser": "^5.16.0", "tsx": "^4.8.1", "yaml": "^2.4.2" }, "optionalPeers": ["@types/node", "@vitejs/devtools", "esbuild", "jiti", "less", "sass", "sass-embedded", "stylus", "sugarss", "terser", "tsx", "yaml"], "bin": { "vite": "bin/vite.js" } }, "sha512-7ULLwsCdYx/nRyrpiEwvqb5TFHrMVZyBt+rg/OAXT7rgj/z+DtTDyKFeLAdDkubDVDKD8jOsndmy7m55XcfUsw=="],
 
-    "vite-tsconfig-paths": ["[email protected]", "", { "dependencies": { "debug": "^4.1.1", "globrex": "^0.1.2", "tsconfck": "^3.0.3" }, "peerDependencies": { "vite": "*" } }, "sha512-2cihq7zliibCCZ8P9cKJrQBkfgdvcFkOOc3Y02o3GWUDLgqjWsZudaoiuOwO/gzTzy17cS5F7ZPo4bsnS4DGkg=="],
-
     "vitest": ["[email protected]", "", { "dependencies": { "@vitest/expect": "4.1.10", "@vitest/mocker": "4.1.10", "@vitest/pretty-format": "4.1.10", "@vitest/runner": "4.1.10", "@vitest/snapshot": "4.1.10", "@vitest/spy": "4.1.10", "@vitest/utils": "4.1.10", "es-module-lexer": "^2.0.0", "expect-type": "^1.3.0", "magic-string": "^0.30.21", "obug": "^2.1.1", "pathe": "^2.0.3", "picomatch": "^4.0.3", "std-env": "^4.0.0-rc.1", "tinybench": "^2.9.0", "tinyexec": "^1.0.2", "tinyglobby": "^0.2.15", "tinyrainbow": "^3.1.0", "vite": "^6.0.0 || ^7.0.0 || ^8.0.0", "why-is-node-running": "^2.3.0" }, "peerDependencies": { "@edge-runtime/vm": "*", "@opentelemetry/api": "^1.9.0", "@types/node": "^20.0.0 || ^22.0.0 || >=24.0.0", "@vitest/browser-playwright": "4.1.10", "@vitest/browser-preview": "4.1.10", "@vitest/browser-webdriverio": "4.1.10", "@vitest/coverage-istanbul": "4.1.10", "@vitest/coverage-v8": "4.1.10", "@vitest/ui": "4.1.10", "happy-dom": "*", "jsdom": "*" }, "optionalPeers": ["@edge-runtime/vm", "@opentelemetry/api", "@types/node", "@vitest/browser-playwright", "@vitest/browser-preview", "@vitest/browser-webdriverio", "@vitest/coverage-istanbul", "@vitest/coverage-v8", "@vitest/ui", "happy-dom", "jsdom"], "bin": { "vitest": "./vitest.mjs" } }, "sha512-R9jUTe5S4Qb0HCd4TNqpC7oGcrMssMRGXLW80ubjWsW9VH5GF8y1Y0SFLY9AbqSk6nt0PnOx4H4WNJYZ13GUPw=="],
 
     "w3c-xmlserializer": ["[email protected]", "", { "dependencies": { "xml-name-validator": "^5.0.0" } }, "sha512-o8qghlI8NZHU1lLPrpi2+Uq7abh4GGPpYANlalzWxyWteJOCsr/P+oPBA49TOLu5FTZO4d3F9MnWJfiMo4BkmA=="],

components/guest-gate.tsx

@@ -26,8 +26,10 @@ export function GuestGate({ children }: { children: ReactNode }) {
   }
 
   const confirm = () => {
+    // The length check the input's own maxLength={100} already enforces is gone: it was
+    // unreachable, and an unreachable guard reads as protection that is not there.
     const trimmed = guestName.trim();
-    if (!trimmed || trimmed.length > 100) return;
+    if (!trimmed) return;
     localStorage.setItem('openframe_guest_name', trimmed);
     setConfirmed(true);
   };
@@ -45,7 +47,11 @@ export function GuestGate({ children }: { children: ReactNode }) {
           </p>
         </div>
         <div className="space-y-3">
+          <label htmlFor="guest-gate-name" className="sr-only">
+            Your name
+          </label>
           <Input
+            id="guest-gate-name"
             placeholder="Your name"
             value={guestName}
             maxLength={100}
@@ -55,11 +61,7 @@ export function GuestGate({ children }: { children: ReactNode }) {
             }}
             autoFocus
           />
-          <Button
-            className="w-full"
-            disabled={!guestName.trim() || guestName.trim().length > 100}
-            onClick={confirm}
-          >
+          <Button className="w-full" disabled={!guestName.trim()} onClick={confirm}>
             Continue
           </Button>
         </div>

components/members-management-page.tsx

@@ -105,6 +105,11 @@ export function MembersManagementPage({
           router.push('/dashboard');
           return;
         }
+        // Any other status has to say so. Returning silently rendered "No members yet"
+        // and "No pending invitations" on a workspace that has both, and the user's next
+        // move was to re-invite somebody who is already a member, which answers 409 and
+        // reads as a second, unrelated bug.
+        setError('Failed to load members. Please refresh to try again.');
         return;
       }
       const data = await res.json();

components/share-link-unlock.tsx

@@ -59,7 +59,11 @@ export function ShareLinkUnlock({ videoId }: ShareLinkUnlockProps) {
         </div>
 
         <div className="space-y-3">
+          <label htmlFor="share-link-password" className="sr-only">
+            Password
+          </label>
           <Input
+            id="share-link-password"
             type="password"
             placeholder="Password"
             value={password}
@@ -74,7 +78,14 @@ export function ShareLinkUnlock({ videoId }: ShareLinkUnlockProps) {
           />
 
           <Button className="w-full" disabled={isSubmitting} onClick={() => void submitPassword()}>
-            {isSubmitting ? <Loader2 className="h-4 w-4 animate-spin" /> : 'Continue'}
+            {isSubmitting ? (
+              <>
+                <Loader2 className="h-4 w-4 animate-spin" aria-hidden="true" />
+                <span className="sr-only">Unlocking</span>
+              </>
+            ) : (
+              'Continue'
+            )}
           </Button>
 
           {error && <p className="text-sm text-destructive">{error}</p>}

components/video-page-content.tsx

@@ -163,7 +163,7 @@ export function VideoPageContent({
     assets,
     isLoadingAssets,
     isCreatingAsset,
-    activeDeleteAssetId,
+    deletingAssetIds,
     activeDownloadAssetId,
     hasMoreAssets,
     isLoadingMoreAssets,
@@ -908,7 +908,7 @@ export function VideoPageContent({
               assets={assets}
               isLoadingAssets={isLoadingAssets}
               isCreatingAsset={isCreatingAsset}
-              activeDeleteAssetId={activeDeleteAssetId}
+              deletingAssetIds={deletingAssetIds}
               activeDownloadAssetId={activeDownloadAssetId}
               canUploadAssets={canUploadAssets}
               canDownloadAssets={canDownloadAssets}

components/video-page/asset-list-section.tsx

@@ -20,7 +20,7 @@ interface AssetListSectionProps {
   bunnyProcessingByAssetId: Record<string, boolean>;
   bunnyReadyByAssetId: Record<string, boolean>;
   activeDownloadAssetId: string | null;
-  activeDeleteAssetId: string | null;
+  deletingAssetIds: string[];
   canDownloadAssets: boolean;
   hasMoreAssets: boolean;
   isLoadingMoreAssets: boolean;
@@ -38,7 +38,7 @@ export const AssetListSection = memo(function AssetListSection({
   bunnyProcessingByAssetId,
   bunnyReadyByAssetId,
   activeDownloadAssetId,
-  activeDeleteAssetId,
+  deletingAssetIds,
   canDownloadAssets,
   hasMoreAssets,
   isLoadingMoreAssets,
@@ -186,10 +186,10 @@ export const AssetListSection = memo(function AssetListSection({
                     className="h-7 w-7"
                     title="Delete asset"
                     aria-label="Delete asset"
-                    disabled={activeDeleteAssetId === asset.id}
+                    disabled={deletingAssetIds.includes(asset.id)}
                     onClick={() => onDeleteAsset(asset.id)}
                   >
-                    {activeDeleteAssetId === asset.id ? (
+                    {deletingAssetIds.includes(asset.id) ? (
                       <Loader2 className="h-3 w-3 animate-spin" />
                     ) : (
                       <Trash2 className="h-3 w-3" />

components/video-page/assets-pane.tsx

@@ -82,7 +82,7 @@ interface AssetsPaneProps {
   assets: VideoAsset[];
   isLoadingAssets: boolean;
   isCreatingAsset: boolean;
-  activeDeleteAssetId: string | null;
+  deletingAssetIds: string[];
   activeDownloadAssetId: string | null;
   canUploadAssets: boolean;
   canDownloadAssets: boolean;
@@ -112,7 +112,7 @@ export const AssetsPane = memo(function AssetsPane({
   assets,
   isLoadingAssets,
   isCreatingAsset,
-  activeDeleteAssetId,
+  deletingAssetIds,
   activeDownloadAssetId,
   canUploadAssets,
   canDownloadAssets,
@@ -1518,7 +1518,7 @@ export const AssetsPane = memo(function AssetsPane({
         bunnyProcessingByAssetId={bunnyProcessingByAssetId}
         bunnyReadyByAssetId={bunnyReadyByAssetId}
         activeDownloadAssetId={activeDownloadAssetId}
-        activeDeleteAssetId={activeDeleteAssetId}
+        deletingAssetIds={deletingAssetIds}
         canDownloadAssets={canDownloadAssets}
         hasMoreAssets={hasMoreAssets}
         isLoadingMoreAssets={isLoadingMoreAssets}

components/video-page/comment-rich-text.tsx

@@ -13,13 +13,16 @@ interface CommentRichTextProps {
   assets?: VideoAsset[];
 }
 
-function renderUrls(text: string): React.ReactNode[] {
+// `keyPrefix` scopes the indices to this slice. The function runs once per gap between
+// mentions, so keying on the index alone emitted `txt-0` for several siblings and React
+// warned about duplicate keys.
+function renderUrls(text: string, keyPrefix: string): React.ReactNode[] {
   const parts = text.split(URL_REGEX);
   return parts.map((part, index) => {
     if (/^https?:\/\/[^\s]+$/.test(part)) {
       return (
         <a
-          key={`url-${index}`}
+          key={`${keyPrefix}-url-${index}`}
           href={part}
           target="_blank"
           rel="noopener noreferrer"
@@ -30,7 +33,7 @@ function renderUrls(text: string): React.ReactNode[] {
         </a>
       );
     }
-    return <React.Fragment key={`txt-${index}`}>{part}</React.Fragment>;
+    return <React.Fragment key={`${keyPrefix}-txt-${index}`}>{part}</React.Fragment>;
   });
 }
 
@@ -43,7 +46,7 @@ export function CommentRichText({ text, onAssetMentionClick, assets = [] }: Comm
     if (mentionIndex < 0) continue;
 
     if (mentionIndex > lastIndex) {
-      nodes.push(...renderUrls(text.slice(lastIndex, mentionIndex)));
+      nodes.push(...renderUrls(text.slice(lastIndex, mentionIndex), `s${lastIndex}`));
     }
 
     const fallbackLabel = match[1] || 'asset';
@@ -88,7 +91,7 @@ export function CommentRichText({ text, onAssetMentionClick, assets = [] }: Comm
   }
 
   if (lastIndex < text.length) {
-    nodes.push(...renderUrls(text.slice(lastIndex)));
+    nodes.push(...renderUrls(text.slice(lastIndex), `s${lastIndex}`));
   }
 
   return <>{nodes}</>;

components/video-page/comments-pane.tsx

@@ -66,8 +66,8 @@ interface CommentsPaneProps {
   setEditingCommentId: (id: string | null) => void;
   editText: string;
   setEditText: (value: string) => void;
-  editTagId: string | null;
-  setEditTagId: (value: string | null) => void;
+  editTagId: string | null | undefined;
+  setEditTagId: (value: string | null | undefined) => void;
   setEditAnnotationData: (value: string | null | undefined) => void;
   setIsEditingAnnotation: (value: boolean) => void;
   onStartEditAnnotation: () => void;
@@ -488,7 +488,7 @@ export const CommentsPane = memo(function CommentsPane({
                             if (e.key === 'Escape') {
                               setEditingCommentId(null);
                               setEditText('');
-                              setEditTagId(null);
+                              setEditTagId(undefined);
                               setEditAnnotationData(undefined);
                               setIsEditingAnnotation(false);
                             }
@@ -513,7 +513,7 @@ export const CommentsPane = memo(function CommentsPane({
                             onClick={() => {
                               setEditingCommentId(null);
                               setEditText('');
-                              setEditTagId(null);
+                              setEditTagId(undefined);
                               setEditAnnotationData(undefined);
                               setIsEditingAnnotation(false);
                             }}
@@ -726,6 +726,9 @@ export const CommentsPane = memo(function CommentsPane({
                                           onClick={() => {
                                             setEditingCommentId(reply.id);
                                             setEditText(reply.content || '');
+                                            // No tag picker on a reply: undefined keeps
+                                            // the PATCH from carrying a tagId at all.
+                                            setEditTagId(undefined);
                                           }}
                                         >
                                           <Pencil className="h-4 w-4 mr-2" />

components/video-page/guest-name-gate.tsx

@@ -30,7 +30,11 @@ export const GuestNameGate = memo(function GuestNameGate({
           </p>
         </div>
         <div className="space-y-3">
+          <label htmlFor="guest-name" className="sr-only">
+            Your name
+          </label>
           <Input
+            id="guest-name"
             placeholder="Your name"
             value={guestName}
             onChange={(e) => setGuestName(e.target.value)}

components/video-page/hooks/use-comment-actions.ts

@@ -116,7 +116,12 @@ export function useCommentActions({
 
   const [editingCommentId, setEditingCommentId] = useState<string | null>(null);
   const [editText, setEditText] = useState('');
-  const [editTagId, setEditTagId] = useState<string | null>(null);
+  // `undefined` means "this editor does not manage a tag", which is the reply editor:
+  // replies have no tag picker. `null` means "no tag", which the comment editor seeds
+  // from the comment itself. Initialising to `null` made `editTagId !== undefined` always
+  // true, so editing a reply's text sent `tagId: null` and cleared its tag, or sent a
+  // stale value left over from a previous edit.
+  const [editTagId, setEditTagId] = useState<string | null | undefined>(undefined);
   const [editAnnotationData, setEditAnnotationData] = useState<string | null | undefined>(
     undefined
   );
@@ -569,6 +574,11 @@ export function useCommentActions({
       if (!activeVersionId) return;
 
       isMutatingRef.current = true;
+      // The optimistic flip, the request body and the rollback all derive from the same
+      // value. Flipping relative to the row (`!c.isResolved`) while the body and the
+      // rollback came from `currentlyResolved` meant a failed request could leave the
+      // comment in a state it was never in whenever the two disagreed.
+      const nextResolved = !currentlyResolved;
       setVideo((prev) => {
         if (!prev) return prev;
         return {
@@ -578,7 +588,7 @@ export function useCommentActions({
               ? {
                   ...v,
                   comments: v.comments.map((c) =>
-                    c.id === commentId ? { ...c, isResolved: !c.isResolved } : c
+                    c.id === commentId ? { ...c, isResolved: nextResolved } : c
                   ),
                 }
               : v
@@ -590,7 +600,7 @@ export function useCommentActions({
         const res = await fetch(`/api/comments/${commentId}`, {
           method: 'PATCH',
           headers: { 'Content-Type': 'application/json' },
-          body: JSON.stringify({ isResolved: !currentlyResolved }),
+          body: JSON.stringify({ isResolved: nextResolved }),
         });
 
         if (!res.ok) {
@@ -1027,7 +1037,7 @@ export function useCommentActions({
           });
           setEditingCommentId(null);
           setEditText('');
-          setEditTagId(null);
+          setEditTagId(undefined);
           setEditAnnotationData(undefined);
           setIsEditingAnnotation(false);
           if (finalAnnotationData !== undefined && finalAnnotationData) {
@@ -1070,9 +1080,17 @@ export function useCommentActions({
 
       isMutatingRef.current = true;
 
+      // The snapshot is taken once, however many times React runs the updater. React is
+      // free to invoke an updater more than once (StrictMode, and React 19 retries a
+      // render that threw), and a second run would otherwise capture the post-delete
+      // state, turning a failed delete into a silent confirmation of it.
       const previousVideoRef: { current: VideoData | null } = { current: null };
+      let capturedSnapshot = false;
       setVideo((prev) => {
-        previousVideoRef.current = prev;
+        if (!capturedSnapshot) {
+          previousVideoRef.current = prev;
+          capturedSnapshot = true;
+        }
         if (!prev) return prev;
         return {
           ...prev,

components/video-page/hooks/use-download-actions.ts

@@ -1,6 +1,6 @@
 'use client';
 
-import { useCallback, useState } from 'react';
+import { useCallback, useRef, useState } from 'react';
 import { toast } from 'sonner';
 import type {
   BunnyDownloadPreference,
@@ -70,10 +70,14 @@ interface UseDownloadActionsParams {
 export function useDownloadActions({ activeVersion, video }: UseDownloadActionsParams) {
   const [activeDownloadTarget, setActiveDownloadTarget] = useState<DownloadTarget | null>(null);
   const isDownloadingVideo = activeDownloadTarget !== null;
+  // The guard reads a ref, not the state. Two calls originating in the same render both
+  // saw the old state value and both proceeded, so a fast double-click downloaded the
+  // file twice.
+  const isDownloadingRef = useRef(false);
 
   const startDownload = useCallback(
     async (preference: BunnyDownloadPreference = 'compressed') => {
-      if (!activeVersion || !video || isDownloadingVideo) return;
+      if (!activeVersion || !video || isDownloadingRef.current) return;
       if (!video.canDownload) {
         toast.error('Download is disabled for this shared link');
         return;
@@ -88,6 +92,7 @@ export function useDownloadActions({ activeVersion, video }: UseDownloadActionsP
       }
 
       const target: DownloadTarget = activeVersion.providerId === 'bunny' ? preference : 'direct';
+      isDownloadingRef.current = true;
       setActiveDownloadTarget(target);
       let progressToast: DownloadProgressToastHandle | null = null;
       try {
@@ -191,10 +196,11 @@ export function useDownloadActions({ activeVersion, video }: UseDownloadActionsP
           toast.error('Failed to start download');
         }
       } finally {
+        isDownloadingRef.current = false;
         setActiveDownloadTarget(null);
       }
     },
-    [activeVersion, video, isDownloadingVideo]
+    [activeVersion, video]
   );
 
   return {

components/video-page/hooks/use-version-actions.ts

@@ -13,6 +13,11 @@ import type { VersionActionsConfig, VideoData } from '@/components/video-page/ty
 import { resolvePublicBunnyCdnHostname } from '@/lib/bunny-cdn';
 import { cleanupPendingR2VideoUpload, uploadVideoToR2 } from '@/lib/client/r2-video-upload';
 
+/** What a failed version upload has to undo, depending on which provider it started on. */
+type PendingVersionCleanup =
+  | { objectKey: string; uploadToken: string; reservationId: string | null }
+  | { bunnyVideoId: string; uploadToken: string };
+
 interface UseVersionActionsParams extends VersionActionsConfig {
   setVideo: Dispatch<SetStateAction<VideoData | null>>;
   activeVersionId: string | null;
@@ -60,7 +65,18 @@ export function useVersionActions({
     }
   };
 
-  const uploadNewVersionFile = async (file: File, title: string) => {
+  /**
+   * `onPendingCleanup` is called the moment there is something to clean up, which for a
+   * Bunny upload is when bunny-init answers and the remote video already exists. Waiting
+   * for this function to return instead meant a tus `onError` threw past the assignment,
+   * so every failed Bunny upload left a video behind on the Bunny side: billed, and
+   * invisible in the app.
+   */
+  const uploadNewVersionFile = async (
+    file: File,
+    title: string,
+    onPendingCleanup: (cleanup: PendingVersionCleanup) => void
+  ) => {
     if (!projectId) throw new Error('Missing project');
 
     if (directUploadProvider === 'r2') {
@@ -102,6 +118,9 @@ export function useVersionActions({
       data: { videoId: bunnyVideoId, libraryId, signature, expirationTime, uploadToken },
     } = await initRes.json();
 
+    // The remote video exists from here on, so it is cleanable from here on.
+    onPendingCleanup({ bunnyVideoId, uploadToken });
+
     await new Promise((resolve, reject) => {
       setNewVersionUploadStatus('Uploading video...');
       const upload = new tus.Upload(file, {
@@ -154,17 +173,7 @@ export function useVersionActions({
     setIsCreatingVersion(true);
     setNewVersionUploadStatus('');
     setNewVersionUploadProgress(0);
-    let pendingCleanup:
-      | {
-          objectKey: string;
-          uploadToken: string;
-          reservationId: string | null;
-        }
-      | {
-          bunnyVideoId: string;
-          uploadToken: string;
-        }
-      | null = null;
+    let pendingCleanup: PendingVersionCleanup | null = null;
 
     try {
       let finalVideoUrl = '';
@@ -194,7 +203,9 @@ export function useVersionActions({
           title = title.replace(/\.[^/.]+$/, '');
         }
 
-        const uploaded = await uploadNewVersionFile(newVersionFile, title);
+        const uploaded = await uploadNewVersionFile(newVersionFile, title, (cleanup) => {
+          pendingCleanup = cleanup;
+        });
         finalVideoUrl = uploaded.finalVideoUrl;
         finalProviderId = uploaded.finalProviderId;
         finalProviderVideoId = uploaded.finalProviderVideoId;

components/video-page/hooks/use-video-assets.ts

@@ -57,13 +57,18 @@ export function useVideoAssets({
   const [assets, setAssets] = useState<VideoAsset[]>([]);
   const [isLoadingAssets, setIsLoadingAssets] = useState(true);
   const [isCreatingAsset, setIsCreatingAsset] = useState(false);
-  const [activeDeleteAssetId, setActiveDeleteAssetId] = useState<string | null>(null);
+  // A set, not a single slot. Two overlapping deletes used to clear each other's
+  // spinner, so the first one stopped indicating progress while it was still running.
+  const [deletingAssetIds, setDeletingAssetIds] = useState<string[]>([]);
   const [activeDownloadAssetId, setActiveDownloadAssetId] = useState<string | null>(null);
   const [hasMoreAssets, setHasMoreAssets] = useState(false);
   const [nextAssetsOffset, setNextAssetsOffset] = useState(0);
   const [isLoadingMoreAssets, setIsLoadingMoreAssets] = useState(false);
   const assetsEtagRef = useRef<string | null>(null);
   const isMutatingRef = useRef(false);
+  // The double-call guard reads a ref, not the state: two calls originating in the same
+  // render both saw the old state value and both fetched the next page.
+  const isLoadingMoreRef = useRef(false);
 
   const fetchAssets = useCallback(
     async (options?: { useEtag?: boolean; silent?: boolean }) => {
@@ -106,7 +111,8 @@ export function useVideoAssets({
   );
 
   const loadMoreAssets = useCallback(async () => {
-    if (isLoadingMoreAssets || !hasMoreAssets) return;
+    if (isLoadingMoreRef.current || !hasMoreAssets) return;
+    isLoadingMoreRef.current = true;
     setIsLoadingMoreAssets(true);
     try {
       const res = await fetch(
@@ -129,9 +135,10 @@ export function useVideoAssets({
     } catch {
       toast.error('Failed to load more assets');
     } finally {
+      isLoadingMoreRef.current = false;
       setIsLoadingMoreAssets(false);
     }
-  }, [hasMoreAssets, isLoadingMoreAssets, nextAssetsOffset, videoId]);
+  }, [hasMoreAssets, nextAssetsOffset, videoId]);
 
   useEffect(() => {
     void fetchAssets({ useEtag: true });
@@ -198,7 +205,7 @@ export function useVideoAssets({
 
   const deleteAsset = useCallback(
     async (assetId: string) => {
-      setActiveDeleteAssetId(assetId);
+      setDeletingAssetIds((prev) => (prev.includes(assetId) ? prev : [...prev, assetId]));
       isMutatingRef.current = true;
       try {
         const res = await fetch(`/api/videos/${videoId}/assets/${assetId}`, {
@@ -217,7 +224,7 @@ export function useVideoAssets({
         toast.error('Failed to delete asset');
         return false;
       } finally {
-        setActiveDeleteAssetId(null);
+        setDeletingAssetIds((prev) => prev.filter((id) => id !== assetId));
         isMutatingRef.current = false;
       }
     },
@@ -292,7 +299,7 @@ export function useVideoAssets({
     assets,
     isLoadingAssets,
     isCreatingAsset,
-    activeDeleteAssetId,
+    deletingAssetIds,
     activeDownloadAssetId,
     hasMoreAssets,
     isLoadingMoreAssets,

components/video-page/hooks/use-video-page-data.ts

@@ -129,6 +129,14 @@ export function useVideoPageData({ mode, videoId, propProjectId }: UseVideoPageD
     void fetchVersionComments(activeVersionId, true);
   }, [activeVersionId, fetchVersionComments]);
 
+  // The auto-select reads the current selection from a ref rather than the dependency
+  // array. Depending on `selectedTagId` meant setting it re-ran the effect, so
+  // /api/projects/<id>/tags was requested a second time on every page load.
+  const selectedTagIdRef = useRef(selectedTagId);
+  useEffect(() => {
+    selectedTagIdRef.current = selectedTagId;
+  }, [selectedTagId]);
+
   useEffect(() => {
     if (!projectId) return;
     async function fetchTags() {
@@ -139,7 +147,7 @@ export function useVideoPageData({ mode, videoId, propProjectId }: UseVideoPageD
           const data = await res.json();
           const tags = data.data || [];
           setAvailableTags(tags);
-          if (tags.length > 0 && !selectedTagId) {
+          if (tags.length > 0 && !selectedTagIdRef.current) {
             setSelectedTagId(tags[0].id);
           }
         }
@@ -148,7 +156,7 @@ export function useVideoPageData({ mode, videoId, propProjectId }: UseVideoPageD
       }
     }
     void fetchTags();
-  }, [projectId, selectedTagId, videoId]);
+  }, [projectId, videoId]);
 
   return {
     video,

components/video-page/hooks/use-video-player.ts

@@ -242,8 +242,15 @@ export function useVideoPlayer({
 
     const tag = document.createElement('script');
     tag.src = 'https://www.youtube.com/iframe_api';
+    // A document with no <script> is unusual but not impossible, and dereferencing the
+    // first one threw on mount when there was none. Next always emits one in the app;
+    // appending to <head> covers everything else.
     const firstScriptTag = document.getElementsByTagName('script')[0];
-    firstScriptTag.parentNode?.insertBefore(tag, firstScriptTag);
+    if (firstScriptTag?.parentNode) {
+      firstScriptTag.parentNode.insertBefore(tag, firstScriptTag);
+    } else {
+      document.head.appendChild(tag);
+    }
 
     window.onYouTubeIframeAPIReady = () => {
       setIsApiLoaded(true);

components/video-page/hooks/video-player-utils.ts

@@ -15,8 +15,23 @@ const STANDARD_FRAME_RATES = [23.976, 24, 25, 29.97, 30, 48, 50, 59.94, 60, 120]
 
 export function normalizeFrameRate(rate: number | undefined): number | null {
   if (typeof rate !== 'number' || !Number.isFinite(rate) || rate < 12 || rate > 120) return null;
-  const standard = STANDARD_FRAME_RATES.find((value) => Math.abs(rate - value) / value < 0.015);
-  return standard ?? rate;
+
+  // Nearest, not first-within-tolerance. The NTSC pairs (23.976/24, 29.97/30, 59.94/60)
+  // are 0.1 percent apart and the tolerance is 1.5 percent, so taking the first match made
+  // an exactly 30 fps source snap to 29.97 and left 24, 30 and 60 unreachable entirely.
+  // That produced the very drift the snapping exists to prevent, roughly 18 frames after
+  // ten minutes.
+  let nearest: number | null = null;
+  let nearestDistance = Infinity;
+  for (const value of STANDARD_FRAME_RATES) {
+    const distance = Math.abs(rate - value);
+    if (distance / value < 0.015 && distance < nearestDistance) {
+      nearest = value;
+      nearestDistance = distance;
+    }
+  }
+
+  return nearest ?? rate;
 }
 
 /**

components/video-page/version-actions-dialog.tsx

@@ -97,10 +97,11 @@ export const VersionActionsDialog = memo(function VersionActionsDialog({
 
           {newVersionMode === 'url' ? (
             <div className="space-y-2">
-              <Label>Video URL</Label>
+              <Label htmlFor="versionUrl">Video URL</Label>
               <div className="relative">
                 <LinkIcon className="absolute left-3 top-1/2 -translate-y-1/2 h-4 w-4 text-muted-foreground" />
                 <Input
+                  id="versionUrl"
                   placeholder="https://youtube.com/watch?v=..."
                   value={newVersionUrl}
                   onChange={(e) => onNewVersionUrlChange(e.target.value)}
@@ -173,8 +174,9 @@ export const VersionActionsDialog = memo(function VersionActionsDialog({
           )}
 
           <div className="space-y-2">
-            <Label>Version Label (optional)</Label>
+            <Label htmlFor="versionLabel">Version Label (optional)</Label>
             <Input
+              id="versionLabel"
               placeholder="e.g. Final Cut, Review Round 2"
               value={newVersionLabel}
               onChange={(e) => onNewVersionLabelChange(e.target.value)}

lib/admin-stats.ts

@@ -2,7 +2,7 @@ import { unstable_cache } from 'next/cache';
 import { db } from '@/lib/db';
 import { r2Client, R2_BUCKET_NAME } from '@/lib/r2';
 import { ListObjectsV2Command, type ListObjectsV2CommandInput } from '@aws-sdk/client-s3';
-import { isBunnyUploadsFeatureEnabled, isStripeBillingEnabled } from '@/lib/feature-flags';
+import { isBunnyUploadsEnabled, isStripeBillingEnabled } from '@/lib/feature-flags';
 import { getStripe, getStripePriceId } from '@/lib/stripe';
 import { logError } from '@/lib/logger';
 
@@ -117,14 +117,31 @@ async function getR2StorageSnapshot(): Promise<R2StorageSnapshot> {
 }
 
 export async function refreshR2StorageSnapshot(): Promise<string> {
-  const snapshot = await buildR2StorageSnapshot();
-  globalForAdminStats.adminR2StorageSnapshot = snapshot;
-  globalForAdminStats.adminR2StorageSnapshotPromise = undefined;
-  return snapshot.refreshedAt;
+  // Single-flight. The promise slot was declared and cleared but never read, so two
+  // concurrent admin refreshes each walked the whole bucket. A second caller now joins
+  // the walk already in progress.
+  const inFlight = globalForAdminStats.adminR2StorageSnapshotPromise;
+  if (inFlight) {
+    return (await inFlight).refreshedAt;
+  }
+
+  const pending = buildR2StorageSnapshot();
+  globalForAdminStats.adminR2StorageSnapshotPromise = pending;
+  try {
+    const snapshot = await pending;
+    globalForAdminStats.adminR2StorageSnapshot = snapshot;
+    return snapshot.refreshedAt;
+  } finally {
+    globalForAdminStats.adminR2StorageSnapshotPromise = undefined;
+  }
 }
 
 async function fetchBunnyStorageStats(): Promise<BunnyStorageStats> {
-  if (!isBunnyUploadsFeatureEnabled()) {
+  // isBunnyUploadsEnabled(), not isBunnyUploadsFeatureEnabled(): the flag alone defaults
+  // to on, so a self-hosted install that never configured Bunny threw
+  // "Missing Bunny Stream credentials." out of getBunnyConfig() below and the dashboard
+  // reported -1 instead of zero.
+  if (!isBunnyUploadsEnabled()) {
     return { totalBytes: 0, byVideoId: {} };
   }
 
@@ -219,7 +236,12 @@ export const getCachedUserBunnyStorage = unstable_cache(
             video: {
               select: {
                 project: {
-                  select: { ownerId: true },
+                  // The workspace owner, not the project owner. lib/storage-quota.ts bills
+                  // R2 versions to the workspace owner and comment media below does the
+                  // same, and getCachedUserBunnyStorage feeds getUserTotalStorageBytes, so
+                  // the moment project and workspace ownership can differ one workspace's
+                  // Bunny bytes and its R2 bytes would count against two different quotas.
+                  select: { workspace: { select: { ownerId: true } } },
                 },
               },
             },
@@ -239,7 +261,7 @@ export const getCachedUserBunnyStorage = unstable_cache(
 
       const seenVideoIds = new Set<string>();
       for (const version of bunnyVersions) {
-        const ownerId = version.video.project.ownerId;
+        const ownerId = version.video.project.workspace.ownerId;
         const dedupeKey = `${ownerId}:${version.videoId}`;
         if (seenVideoIds.has(dedupeKey)) continue;
         seenVideoIds.add(dedupeKey);
@@ -427,6 +449,8 @@ export interface StripeStats {
   pastDueUsers: number;
   canceledUsers: number;
   freeUsers: number;
+  /** UNPAID, INCOMPLETE and INCOMPLETE_EXPIRED, which belong to none of the buckets above. */
+  otherStatusUsers: number;
   mrrCents: number;
   currency: string;
 }
@@ -445,8 +469,7 @@ export const getCachedStripeStats = unstable_cache(
 
       const counts: Record<string, number> = {};
       for (const row of statusCounts) {
-        const key = row.subscriptionStatus ?? 'UNKNOWN';
-        counts[key] = row._count.id;
+        counts[row.subscriptionStatus] = row._count.id;
       }
 
       const activeSubscribers = counts['ACTIVE'] ?? 0;
@@ -454,6 +477,11 @@ export const getCachedStripeStats = unstable_cache(
       const pastDueUsers = counts['PAST_DUE'] ?? 0;
       const canceledUsers = counts['CANCELED'] ?? 0;
       const freeUsers = counts['FREE'] ?? 0;
+      // UNPAID, INCOMPLETE and INCOMPLETE_EXPIRED belonged to none of the five buckets
+      // above, so those users were counted nowhere and the totals silently did not add
+      // up to the user table.
+      const otherStatusUsers =
+        (counts['UNPAID'] ?? 0) + (counts['INCOMPLETE'] ?? 0) + (counts['INCOMPLETE_EXPIRED'] ?? 0);
 
       let mrrCents = 0;
       let currency = 'usd';
@@ -475,6 +503,7 @@ export const getCachedStripeStats = unstable_cache(
         pastDueUsers,
         canceledUsers,
         freeUsers,
+        otherStatusUsers,
         mrrCents,
         currency,
       };

lib/auth.ts

@@ -152,8 +152,6 @@ export const { handlers, signIn, signOut, auth } = NextAuth({
   },
 });
 
-type ProjectAccessIntent = 'view' | 'manage' | 'delete';
-
 // ---------------------------------------------------------------------------
 // Fast-path: pre-fetch access data alongside any existing DB query so that
 // computeProjectAccess() can resolve the result with zero extra round-trips.
@@ -313,13 +311,20 @@ export function computeProjectAccess(
   });
 }
 
-// Helper to check project access including workspace membership
+/**
+ * Helper to check project access including workspace membership.
+ *
+ * This used to take an `intent`, which skipped both workspace queries for an owner at
+ * `view` intent. That made it report `isWorkspaceMember: false, isWorkspaceAdmin: false`
+ * for the actor computeProjectAccess reports `true, true` for: the project owner who also
+ * owns the workspace, which is the shape every real signup produces. The two are meant to
+ * answer the same question, so they resolve their inputs the same way now and the option
+ * is gone rather than kept as a parameter that changes nothing.
+ */
 export async function checkProjectAccess(
   project: { id: string; ownerId: string; workspaceId: string; visibility: string },
-  userId: string | undefined,
-  options?: { intent?: ProjectAccessIntent }
+  userId: string | undefined
 ) {
-  const intent = options?.intent ?? 'view';
   const isOwner = userId === project.ownerId;
   const isPublic = project.visibility === 'PUBLIC';
 
@@ -333,50 +338,18 @@ export async function checkProjectAccess(
   const isProjectAdmin = projectMember?.role === ProjectMemberRole.ADMIN;
 
   // The workspace role decides `canEdit`/`isWorkspaceMember`, not just whether the viewer
-  // gets in at all, so it has to be resolved for every signed-in non-owner. Skipping it
-  // once access was already granted some other way (public project, or an existing project
-  // membership) silently downgraded workspace admins to read-only on `intent: 'view'`,
-  // the intent that pages and GET routes use to decide which actions to render.
-  // Owners pass every check on their own; resolve their role only when they mutate.
-  const shouldLoadWorkspaceRole = !!userId && (!isOwner || intent !== 'view');
-
-  // Check workspace membership/role
-  let workspaceRole: WorkspaceMemberRole | 'OWNER' | null = null;
-  let workspaceOwnerBillingAccess = false;
-  if (shouldLoadWorkspaceRole && userId) {
-    const [wsMember, wsOwner] = await Promise.all([
-      db.workspaceMember.findUnique({
-        where: { workspaceId_userId: { workspaceId: project.workspaceId, userId } },
-      }),
-      db.workspace.findUnique({
-        where: { id: project.workspaceId },
-        select: {
-          ownerId: true,
-          owner: {
-            select: {
-              subscriptionStatus: true,
-              trialEndsAt: true,
-              stripeCurrentPeriodEnd: true,
-              billingAccessEndedAt: true,
-            },
-          },
-        },
-      }),
-    ]);
-
-    if (wsOwner?.ownerId === userId) {
-      workspaceRole = 'OWNER';
-    } else if (wsMember) {
-      workspaceRole = wsMember.role;
-    }
-
-    if (wsOwner?.owner) {
-      workspaceOwnerBillingAccess = hasBillingAccess(wsOwner.owner);
-    }
-  } else {
-    const wsOwner = await db.workspace.findUnique({
+  // gets in at all, so it is resolved for every signed-in caller, owners included. The two
+  // queries run together, so this costs one extra indexed lookup and no extra latency.
+  const [wsMember, wsOwner] = await Promise.all([
+    userId
+      ? db.workspaceMember.findUnique({
+          where: { workspaceId_userId: { workspaceId: project.workspaceId, userId } },
+        })
+      : null,
+    db.workspace.findUnique({
       where: { id: project.workspaceId },
       select: {
+        ownerId: true,
         owner: {
           select: {
             subscriptionStatus: true,
@@ -386,9 +359,18 @@ export async function checkProjectAccess(
           },
         },
       },
-    });
-    workspaceOwnerBillingAccess = wsOwner?.owner ? hasBillingAccess(wsOwner.owner) : false;
+    }),
+  ]);
+
+  let workspaceRole: WorkspaceMemberRole | 'OWNER' | null = null;
+  if (userId && wsOwner?.ownerId === userId) {
+    workspaceRole = 'OWNER';
+  } else if (wsMember) {
+    workspaceRole = wsMember.role;
   }
+
+  const workspaceOwnerBillingAccess = wsOwner?.owner ? hasBillingAccess(wsOwner.owner) : false;
+
   return resolveProjectPermissions({
     isOwner,
     isPublic,

lib/billing.ts

@@ -359,11 +359,11 @@ function getInactiveBillingAccessEndedAt(
 }
 
 function getEntitledStripePriceId(subscription: Stripe.Subscription) {
-  const configuredPriceId = getStripePriceId();
+  return hasEntitledPrice(subscription, getStripePriceId()) ? getStripePriceId() : null;
+}
 
-  return (
-    subscription.items.data.find((item) => item.price.id === configuredPriceId)?.price.id ?? null
-  );
+function hasEntitledPrice(subscription: Stripe.Subscription, configuredPriceId: string): boolean {
+  return subscription.items.data.some((item) => item.price.id === configuredPriceId);
 }
 
 export async function syncStripeSubscriptionToUser(subscription: Stripe.Subscription) {
@@ -456,9 +456,15 @@ export function selectAuthoritativeSubscription(
     return null;
   }
 
+  // Read once, up front. Reading it inside the comparator meant a deployment with no
+  // STRIPE_PRICE_ID configured worked for every customer holding one subscription and
+  // threw only for those holding two, because a comparator never runs for a one-element
+  // array. That is a miserable failure mode to diagnose in production.
+  const configuredPriceId = getStripePriceId();
+
   return [...subscriptions].sort((a, b) => {
-    const aEntitled = Boolean(getEntitledStripePriceId(a));
-    const bEntitled = Boolean(getEntitledStripePriceId(b));
+    const aEntitled = hasEntitledPrice(a, configuredPriceId);
+    const bEntitled = hasEntitledPrice(b, configuredPriceId);
     if (aEntitled !== bEntitled) {
       return aEntitled ? -1 : 1;
     }

lib/bunny-upload-token.ts

@@ -65,6 +65,10 @@ export function createBunnyUploadToken(
 }
 
 export function verifyBunnyUploadToken(token: string, subject: BunnyUploadTokenSubject): boolean {
+  // Resolved before the try. A missing signing secret is a configuration fault, and
+  // swallowing that throw made every upload grant look like a forgery instead.
+  const secret = getBunnyUploadTokenSecret();
+
   try {
     const parts = token.split('.');
     if (parts.length !== 2) return false;
@@ -72,7 +76,7 @@ export function verifyBunnyUploadToken(token: string, subject: BunnyUploadTokenS
     const [encodedPayload, providedSignature] = parts;
     if (!encodedPayload || !providedSignature) return false;
 
-    const expectedSignature = signPayload(encodedPayload, getBunnyUploadTokenSecret());
+    const expectedSignature = signPayload(encodedPayload, secret);
     const providedBuffer = Buffer.from(providedSignature, 'utf8');
     const expectedBuffer = Buffer.from(expectedSignature, 'utf8');
 

lib/client/r2-asset-video-upload.ts

@@ -1,4 +1,5 @@
 import { captureVideoThumbnail } from '@/lib/client/video-thumbnail';
+import { uploadBytesWithProgress, type UploadProgressHandler } from '@/lib/client/r2-video-upload';
 
 export type R2AssetVideoInitResponse = {
   presignedPutUrl: string;
@@ -16,44 +17,8 @@ export type R2AssetVideoUploadResult = R2AssetVideoInitResponse & {
   thumbnailUrl: string | null;
 };
 
-type UploadProgressHandler = (progress: number) => void;
-
-function uploadBytesWithProgress(
-  url: string,
-  body: Blob | File,
-  contentType: string,
-  onProgress?: UploadProgressHandler
-): Promise<void> {
-  return new Promise((resolve, reject) => {
-    const xhr = new XMLHttpRequest();
-    xhr.open('PUT', url);
-    xhr.setRequestHeader('Content-Type', contentType);
-
-    xhr.upload.onprogress = (event) => {
-      if (!onProgress || !event.lengthComputable) return;
-      onProgress(Math.round((event.loaded / event.total) * 100));
-    };
-
-    xhr.onload = () => {
-      if (xhr.status >= 200 && xhr.status < 300) {
-        resolve();
-        return;
-      }
-      reject(new Error(`Upload failed with status ${xhr.status}`));
-    };
-
-    xhr.onerror = () => {
-      reject(
-        new Error(
-          'Network error during upload. If you use direct S3/R2 uploads, configure bucket CORS to allow PUT from this site origin.'
-        )
-      );
-    };
-    xhr.onabort = () => reject(new Error('Upload aborted'));
-
-    xhr.send(body);
-  });
-}
+// uploadBytesWithProgress used to be duplicated here, progress arithmetic included. There
+// is one copy now, in r2-video-upload.ts, which is where the multipart path already lives.
 
 export async function initR2AssetVideoUpload(
   videoId: string,

lib/client/r2-video-upload.ts

@@ -4,6 +4,7 @@ import {
   getPartByteRange,
   getRetryDelayMs,
   getUploadProgressPercent,
+  isRetryableUploadError,
   PART_RETRY_DELAYS_MS,
 } from '@/lib/client/upload-chunking';
 
@@ -33,9 +34,9 @@ export type R2VideoUploadResult = R2VideoInitResponse & {
   thumbnailUrl: string | null;
 };
 
-type UploadProgressHandler = (progress: number) => void;
+export type UploadProgressHandler = (progress: number) => void;
 
-function uploadBytesWithProgress(
+export function uploadBytesWithProgress(
   url: string,
   body: Blob | File,
   contentType: string,
@@ -128,6 +129,9 @@ async function withRetry<T>(fn: () => Promise<T>, delays: number[]): Promise<T>
       return await fn();
     } catch (error) {
       lastError = error;
+      // An abort or a permanent 4xx will fail the same way every time, so repeating it
+      // only delays the error the caller is waiting for.
+      if (!isRetryableUploadError(error)) break;
     }
   }
   throw lastError instanceof Error ? lastError : new Error('Upload failed after retries');

lib/client/upload-chunking.ts

@@ -53,6 +53,7 @@ export function getPartByteRange(
 
 /** Whole-percent progress for a single-request upload. */
 export function getUploadProgressPercent(loadedBytes: number, totalBytes: number): number {
+  if (totalBytes <= 0) return 0;
   return Math.round((loadedBytes / totalBytes) * 100);
 }
 
@@ -60,11 +61,43 @@ export function getUploadProgressPercent(loadedBytes: number, totalBytes: number
  * Whole-percent progress across a multipart upload, given the bytes reported so
  * far for each part. Clamped at 100: parts report their own progress
  * independently and a re-tried part can briefly double-count.
+ *
+ * A total of zero reports 0 rather than dividing. The division produced NaN, which
+ * reached the UI as "Uploading... NaN%".
  */
 export function getMultipartProgressPercent(
   loadedBytesPerPart: number[],
   totalBytes: number
 ): number {
+  if (totalBytes <= 0) return 0;
   const loaded = loadedBytesPerPart.reduce((sum, value) => sum + value, 0);
   return Math.min(100, Math.round((loaded / totalBytes) * 100));
 }
+
+/**
+ * Whether a failed attempt is worth repeating.
+ *
+ * The retry loop used to repeat every rejection, including the user's own cancellation
+ * and permanently-failing statuses. Cancelling an upload therefore did not cancel it: the
+ * part sat through the full 2s, 5s and 10s backoff and fired three more PUTs before the
+ * error surfaced. An expired presigned URL behaved the same way, turning one dead part
+ * into four requests and 17 seconds of apparent hanging.
+ */
+export function isRetryableUploadError(error: unknown): boolean {
+  const message = error instanceof Error ? error.message : String(error);
+
+  if (/aborted/i.test(message)) return false;
+
+  const status = statusFromUploadErrorMessage(message);
+  if (status === null) return true; // A network error has no status and is worth a retry.
+  if (status === 408 || status === 429) return true;
+  return status < 400 || status >= 500;
+}
+
+/** The status code an upload error message carries, if it carries one. */
+export function statusFromUploadErrorMessage(message: string): number | null {
+  const match = /failed with status (\d{3})\b/i.exec(message);
+  if (!match) return null;
+  const status = Number(match[1]);
+  return Number.isFinite(status) ? status : null;
+}

lib/comment-export.ts

@@ -42,9 +42,16 @@ export interface ExportCommentRow {
   createdAtIso: string;
 }
 
+// A leading =, +, - or @ is what a spreadsheet reads as the start of a formula, so those
+// cells get an apostrophe. A plain negative number is not a formula, and prefixing one
+// stopped the spreadsheet reading a negative timestamp as a number at all.
+const FORMULA_START = /^[\s]*[=+\-@]/;
+const PLAIN_NUMBER = /^-?\d+(\.\d+)?$/;
+
 function csvCell(value: string | number | boolean | null): string {
   const raw = value === null ? '' : String(value);
-  const neutralized = /^[\s]*[=+\-@]/.test(raw) ? `'${raw}` : raw;
+  const needsPrefix = FORMULA_START.test(raw) && !PLAIN_NUMBER.test(raw);
+  const neutralized = needsPrefix ? `'${raw}` : raw;
   return `"${neutralized.replace(/"/g, '""')}"`;
 }
 

lib/content-security-policy.ts

@@ -35,9 +35,15 @@ function resolveR2ConnectOrigins(): string[] {
     origins.add('https://*.r2.cloudflarestorage.com');
   }
 
-  // Docker/MinIO self-hosted defaults for local development.
-  origins.add('http://localhost:9000');
-  origins.add('http://127.0.0.1:9000');
+  // Docker/MinIO defaults, for local development only. A production build has no reason
+  // to allow plaintext loopback object storage, and adding it there weakened the policy of
+  // every deployment to accommodate a developer's machine. A self-hosted install whose
+  // storage really is on loopback still works: it sets R2_ENDPOINT, which is picked up
+  // above.
+  if (process.env.NODE_ENV !== 'production') {
+    origins.add('http://localhost:9000');
+    origins.add('http://127.0.0.1:9000');
+  }
 
   return [...origins];
 }

lib/email-brand.ts

@@ -22,7 +22,29 @@ export function escapeHtml(str: string): string {
     .replace(/&/g, '&')
     .replace(/</g, '&lt;')
     .replace(/>/g, '&gt;')
-    .replace(/"/g, '&quot;');
+    .replace(/"/g, '&quot;')
+    .replace(/'/g, '&#39;');
+}
+
+const RAW_EMAIL_HTML = Symbol('rawEmailHtml');
+
+/** Markup a caller has already built and vouches for. See {@link rawEmailHtml}. */
+export type RawEmailHtml = { readonly [RAW_EMAIL_HTML]: string };
+
+/**
+ * Opt a value out of escaping. The helpers below escape everything they are given, so a
+ * caller that genuinely needs markup, a `<span>` around one half of a label, has to say
+ * so here. That keeps the default safe: a project name or a display name passed straight
+ * into a helper is escaped whether or not the caller remembered to.
+ */
+export function rawEmailHtml(html: string): RawEmailHtml {
+  return { [RAW_EMAIL_HTML]: html };
+}
+
+export type EmailText = string | RawEmailHtml;
+
+function renderEmailText(value: EmailText): string {
+  return typeof value === 'string' ? escapeHtml(value) : value[RAW_EMAIL_HTML];
 }
 
 export function escapeAttr(str: string): string {
@@ -33,14 +55,20 @@ export function escapeAttr(str: string): string {
     .replace(/>/g, '&gt;');
 }
 
+/**
+ * `bodyHtml` is markup, not text: it is assembled from the helpers below, so it is the one
+ * value here that is inserted verbatim. Everything a caller supplies as text, the footer
+ * included, is escaped.
+ */
 export function brandedEmailTemplate(
-  body: string,
+  bodyHtml: string,
   options?: {
     footerText?: string;
     footerLinkText?: string;
     footerLinkUrl?: string;
   }
 ): string {
+  const body = bodyHtml;
   const footerText = options?.footerText || '';
   const footerLinkText = options?.footerLinkText || '';
   const footerLinkUrl = options?.footerLinkUrl || '';
@@ -67,7 +95,7 @@ export function brandedEmailTemplate(
           footerText || (footerLinkText && footerLinkUrl)
             ? `
         <tr><td style="padding:20px 0 0;text-align:center;">
-          ${footerText ? `<p style="margin:0 0 6px;font-size:11px;color:${EMAIL_COLORS.textDim};">${footerText}</p>` : ''}
+          ${footerText ? `<p style="margin:0 0 6px;font-size:11px;color:${EMAIL_COLORS.textDim};">${escapeHtml(footerText)}</p>` : ''}
           ${footerLinkText && footerLinkUrl ? `<a href="${escapeAttr(footerLinkUrl)}" style="font-size:11px;color:${EMAIL_COLORS.accent};text-decoration:underline;">${escapeHtml(footerLinkText)}</a>` : ''}
         </td></tr>`
             : ''
@@ -79,26 +107,26 @@ export function brandedEmailTemplate(
 </html>`;
 }
 
-export function emailHeading(icon: string, title: string): string {
+export function emailHeading(icon: EmailText, title: EmailText): string {
   return `<td style="padding:16px 20px;border-bottom:1px solid ${EMAIL_COLORS.border};background-color:${EMAIL_COLORS.accentDark};">
-      <span style="font-size:14px;font-weight:600;color:${EMAIL_COLORS.accent};">${icon} &nbsp;${title}</span>
+      <span style="font-size:14px;font-weight:600;color:${EMAIL_COLORS.accent};">${renderEmailText(icon)} &nbsp;${renderEmailText(title)}</span>
     </td>`;
 }
 
-export function emailRow(label: string, value: string, isHighlight = false): string {
+export function emailRow(label: EmailText, value: EmailText, isHighlight = false): string {
   const valStyle = isHighlight
     ? `color:${EMAIL_COLORS.text};font-weight:600;`
     : `color:${EMAIL_COLORS.textSecondary};`;
   return `<tr>
-      <td style="padding:6px 16px 6px 0;color:${EMAIL_COLORS.textDim};font-size:13px;white-space:nowrap;vertical-align:top;">${label}</td>
-      <td style="padding:6px 0;font-size:13px;${valStyle}">${value}</td>
+      <td style="padding:6px 16px 6px 0;color:${EMAIL_COLORS.textDim};font-size:13px;white-space:nowrap;vertical-align:top;">${renderEmailText(label)}</td>
+      <td style="padding:6px 0;font-size:13px;${valStyle}">${renderEmailText(value)}</td>
     </tr>`;
 }
 
-export function emailButton(text: string, url: string): string {
-  return `<a href="${escapeAttr(url)}" style="display:inline-block;padding:9px 22px;background-color:${EMAIL_COLORS.accent};color:#0f1114;font-size:13px;font-weight:700;text-decoration:none;letter-spacing:0.2px;">${text}</a>`;
+export function emailButton(text: EmailText, url: string): string {
+  return `<a href="${escapeAttr(url)}" style="display:inline-block;padding:9px 22px;background-color:${EMAIL_COLORS.accent};color:#0f1114;font-size:13px;font-weight:700;text-decoration:none;letter-spacing:0.2px;">${renderEmailText(text)}</a>`;
 }
 
-export function emailHighlight(text: string): string {
-  return `<div style="border:1px solid ${EMAIL_COLORS.border};padding:10px 12px;margin:0 0 16px;background-color:${EMAIL_COLORS.cardInner};color:${EMAIL_COLORS.text};font-size:13px;line-height:1.5;">${text}</div>`;
+export function emailHighlight(text: EmailText): string {
+  return `<div style="border:1px solid ${EMAIL_COLORS.border};padding:10px 12px;margin:0 0 16px;background-color:${EMAIL_COLORS.cardInner};color:${EMAIL_COLORS.text};font-size:13px;line-height:1.5;">${renderEmailText(text)}</div>`;
 }

lib/email-verification.ts

@@ -6,7 +6,6 @@ import {
   emailButton,
   emailHeading,
   emailRow,
-  escapeHtml,
   EMAIL_COLORS,
 } from '@/lib/email-brand';
 import { logError } from '@/lib/logger';
@@ -124,14 +123,14 @@ export async function sendVerificationEmail(
         <tr>${emailHeading('✉', 'Verify your email address')}</tr>
         <tr><td style="padding:20px;">
           <table cellpadding="0" cellspacing="0" style="width:100%;margin-bottom:20px;">
-            ${emailRow('Account', escapeHtml(email), true)}
+            ${emailRow('Account', email, true)}
             ${emailRow('Expires in', `${TOKEN_EXPIRY_HOURS} hours`)}
           </table>
           <p style="margin:0 0 20px;font-size:14px;color:${EMAIL_COLORS.textSecondary};line-height:1.6;">
             Click the button below to verify your email address and activate your OpenFrame account.
             If you did not create an account, you can safely ignore this email.
           </p>
-          ${emailButton('Verify Email Address  &#8594;', verifyUrl)}
+          ${emailButton('Verify Email Address  →', verifyUrl)}
         </td></tr>
         `,
     {

lib/invitations.ts

@@ -15,7 +15,6 @@ import {
   emailHeading,
   emailHighlight,
   emailRow,
-  escapeHtml,
 } from '@/lib/email-brand';
 import { logError } from '@/lib/logger';
 
@@ -101,17 +100,17 @@ function invitationEmailTemplate(input: {
 }): string {
   return brandedEmailTemplate(
     `
-      <tr>${emailHeading('&#10003;', `${escapeHtml(input.scope.charAt(0).toUpperCase() + input.scope.slice(1))} Invitation`)}</tr>
+      <tr>${emailHeading('✓', `${input.scope.charAt(0).toUpperCase() + input.scope.slice(1)} Invitation`)}</tr>
       <tr><td style="padding:20px;">
         ${emailHighlight('You were invited to join OpenFrame.')}
         <table cellpadding="0" cellspacing="0" style="width:100%;margin-bottom:16px;">
-          ${emailRow('Invited by', escapeHtml(input.inviterName), true)}
-          ${emailRow('Target', `${escapeHtml(input.targetName)} (${escapeHtml(input.scope)})`, true)}
-          ${emailRow('Role', escapeHtml(input.role))}
+          ${emailRow('Invited by', input.inviterName, true)}
+          ${emailRow('Target', `${input.targetName} (${input.scope})`, true)}
+          ${emailRow('Role', input.role)}
           ${emailRow('Expires', `${INVITATION_TTL_DAYS} days`)}
         </table>
         ${emailHighlight('Create an account (or sign in with this email) to accept this invitation.')}
-        ${emailButton('Accept Invitation &#8594;', input.invitationUrl)}
+        ${emailButton('Accept Invitation →', input.invitationUrl)}
       </td></tr>
     `,
     {
@@ -300,6 +299,18 @@ async function acceptInvitation(tx: Prisma.TransactionClient, invitationId: stri
   });
 }
 
+/**
+ * Applies the invited membership and marks the invitation accepted.
+ *
+ * Returns false when the invitation grants nothing: a scoped row whose target id is null,
+ * or one pointing at a workspace or project that no longer exists. The invitation is left
+ * PENDING in that case, so the caller can report the failure rather than show a success
+ * screen for a no-op the user has no way to detect.
+ *
+ * An existing membership is never downgraded. Applying the invited role unconditionally
+ * turned an invitation into a privilege-change primitive: re-invite a sitting ADMIN as a
+ * COMMENTATOR, get them to click the link once, and they are demoted.
+ */
 async function applyInvitationMembership(
   tx: Prisma.TransactionClient,
   invitation: {
@@ -310,13 +321,17 @@ async function applyInvitationMembership(
     projectId: string | null;
   },
   userId: string
-) {
-  if (invitation.scope === InvitationScope.WORKSPACE && invitation.workspaceId) {
+): Promise<boolean> {
+  const invitedAsAdmin = invitation.role === InvitationRole.ADMIN;
+
+  if (invitation.scope === InvitationScope.WORKSPACE) {
+    if (!invitation.workspaceId) return false;
+
     const workspace = await tx.workspace.findUnique({
       where: { id: invitation.workspaceId },
       select: { ownerId: true },
     });
-    if (!workspace) return;
+    if (!workspace) return false;
 
     if (workspace.ownerId !== userId) {
       await tx.workspaceMember.upsert({
@@ -326,33 +341,28 @@ async function applyInvitationMembership(
             userId,
           },
         },
-        update: {
-          role:
-            invitation.role === InvitationRole.ADMIN
-              ? WorkspaceMemberRole.ADMIN
-              : WorkspaceMemberRole.COMMENTATOR,
-        },
+        // Only ever a promotion. An empty update leaves a sitting ADMIN as they were.
+        update: invitedAsAdmin ? { role: WorkspaceMemberRole.ADMIN } : {},
         create: {
           workspaceId: invitation.workspaceId,
           userId,
-          role:
-            invitation.role === InvitationRole.ADMIN
-              ? WorkspaceMemberRole.ADMIN
-              : WorkspaceMemberRole.COMMENTATOR,
+          role: invitedAsAdmin ? WorkspaceMemberRole.ADMIN : WorkspaceMemberRole.COMMENTATOR,
         },
       });
     }
 
     await acceptInvitation(tx, invitation.id);
-    return;
+    return true;
   }
 
-  if (invitation.scope === InvitationScope.PROJECT && invitation.projectId) {
+  if (invitation.scope === InvitationScope.PROJECT) {
+    if (!invitation.projectId) return false;
+
     const project = await tx.project.findUnique({
       where: { id: invitation.projectId },
       select: { ownerId: true },
     });
-    if (!project) return;
+    if (!project) return false;
 
     if (project.ownerId !== userId) {
       await tx.projectMember.upsert({
@@ -362,25 +372,20 @@ async function applyInvitationMembership(
             userId,
           },
         },
-        update: {
-          role:
-            invitation.role === InvitationRole.ADMIN
-              ? ProjectMemberRole.ADMIN
-              : ProjectMemberRole.COMMENTATOR,
-        },
+        update: invitedAsAdmin ? { role: ProjectMemberRole.ADMIN } : {},
         create: {
           projectId: invitation.projectId,
           userId,
-          role:
-            invitation.role === InvitationRole.ADMIN
-              ? ProjectMemberRole.ADMIN
-              : ProjectMemberRole.COMMENTATOR,
+          role: invitedAsAdmin ? ProjectMemberRole.ADMIN : ProjectMemberRole.COMMENTATOR,
         },
       });
     }
 
     await acceptInvitation(tx, invitation.id);
+    return true;
   }
+
+  return false;
 }
 
 export async function acceptInvitationTokenForUser(input: {
@@ -407,8 +412,10 @@ export async function acceptInvitationTokenForUser(input: {
       return 'expired';
     }
 
-    await applyInvitationMembership(tx, invitation, input.userId);
-    return 'accepted';
+    const applied = await applyInvitationMembership(tx, invitation, input.userId);
+    // A scoped invitation pointing at nothing grants no membership. Reporting 'accepted'
+    // for it showed a success screen for a no-op and left the row PENDING for good.
+    return applied ? 'accepted' : 'not_found';
   });
 }
 

lib/logger.ts

@@ -23,12 +23,18 @@ function sanitizeError(err: unknown): SanitizedError | unknown {
     return err;
   }
 
-  const name = err.constructor?.name ?? err.name ?? 'Error';
+  const constructorName = err.constructor?.name;
+  const name = constructorName || err.name || 'Error';
   const anyErr = err as unknown as Record<string, unknown>;
 
   // Prisma client errors: their `.message` can embed raw SQL, WHERE-clause
   // values, and schema internals. Only safe to expose the Prisma error code.
-  if (name.startsWith('PrismaClient')) {
+  //
+  // Both names are checked. An Error instance always has a constructor, so keying on
+  // `constructor.name` alone would silently stop redacting for an error that identifies
+  // itself only through `name`: one that was re-thrown or deserialised and lost its
+  // prototype, or a production build whose minifier renamed the class.
+  if (name.startsWith('PrismaClient') || err.name.startsWith('PrismaClient')) {
     const code = typeof anyErr.code === 'string' ? anyErr.code : 'UNKNOWN';
     return {
       type: 'PrismaError',
@@ -59,3 +65,15 @@ function sanitizeError(err: unknown): SanitizedError | unknown {
 export function logError(context: string, err: unknown): void {
   console.error(context, sanitizeError(err));
 }
+
+/**
+ * Log a configuration or operational warning. Same sanitisation as {@link logError} for
+ * the optional detail, so a warning cannot become the leak the error path guards against.
+ */
+export function logWarn(context: string, detail?: unknown): void {
+  if (detail === undefined) {
+    console.warn(context);
+    return;
+  }
+  console.warn(context, sanitizeError(detail));
+}

lib/notifications.ts

@@ -8,6 +8,7 @@ import {
   emailHighlight,
   emailRow,
   escapeHtml,
+  rawEmailHtml,
 } from '@/lib/email-brand';
 import { logError } from '@/lib/logger';
 
@@ -311,15 +312,15 @@ function formatEmail(
       return {
         subject: `[OpenFrame] New video in ${event.projectName}: ${event.videoTitle}`,
         html: emailTemplate(`
-                    <tr>${emailHeading('&#9654;', 'New Video Added')}</tr>
+                    <tr>${emailHeading('▶', 'New Video Added')}</tr>
                     <tr><td style="padding:20px;">
                       <table cellpadding="0" cellspacing="0" style="width:100%;margin-bottom:20px;">
-                        ${emailRow('Project', escapeHtml(event.projectName), true)}
-                        ${emailRow('Video', escapeHtml(event.videoTitle), true)}
-                        ${emailRow('Added by', escapeHtml(event.addedBy))}
+                        ${emailRow('Project', event.projectName, true)}
+                        ${emailRow('Video', event.videoTitle, true)}
+                        ${emailRow('Added by', event.addedBy)}
                         ${emailRow('When', now)}
                       </table>
-                      ${emailButton('View Video  &#8594;', event.url)}
+                      ${emailButton('View Video  →', event.url)}
                     </td></tr>
                 `),
       };
@@ -327,16 +328,16 @@ function formatEmail(
       return {
         subject: `[OpenFrame] New version of ${event.videoTitle} in ${event.projectName}`,
         html: emailTemplate(`
-                    <tr>${emailHeading('&#9654;', 'New Version Added')}</tr>
+                    <tr>${emailHeading('▶', 'New Version Added')}</tr>
                     <tr><td style="padding:20px;">
                       <table cellpadding="0" cellspacing="0" style="width:100%;margin-bottom:20px;">
-                        ${emailRow('Project', escapeHtml(event.projectName), true)}
-                        ${emailRow('Video', escapeHtml(event.videoTitle), true)}
-                        ${emailRow('Version', escapeHtml(event.versionLabel))}
-                        ${emailRow('Added by', escapeHtml(event.addedBy))}
+                        ${emailRow('Project', event.projectName, true)}
+                        ${emailRow('Video', event.videoTitle, true)}
+                        ${emailRow('Version', event.versionLabel)}
+                        ${emailRow('Added by', event.addedBy)}
                         ${emailRow('When', now)}
                       </table>
-                      ${emailButton('View Version  &#8594;', event.url)}
+                      ${emailButton('View Version  →', event.url)}
                     </td></tr>
                 `),
       };
@@ -344,19 +345,19 @@ function formatEmail(
       return {
         subject: `[OpenFrame] New comment on ${event.videoTitle}`,
         html: emailTemplate(`
-                    <tr>${emailHeading('&#9679;', 'New Comment')}</tr>
+                    <tr>${emailHeading('●', 'New Comment')}</tr>
                     <tr><td style="padding:20px;">
                       <table cellpadding="0" cellspacing="0" style="width:100%;margin-bottom:16px;">
-                        ${emailRow('Project', escapeHtml(event.projectName), true)}
-                        ${emailRow('Video', escapeHtml(event.videoTitle), true)}
-                        ${emailRow('From', escapeHtml(event.commentAuthor))}
+                        ${emailRow('Project', event.projectName, true)}
+                        ${emailRow('Video', event.videoTitle, true)}
+                        ${emailRow('From', event.commentAuthor)}
                         ${emailRow('At', event.timestamp)}
                         ${emailRow('When', now)}
                       </table>
                       <div style="border-left:2px solid #7aa7ff;padding:10px 14px;margin:0 0 20px;background-color:#2f2f2f;color:#c6c6cc;font-size:13px;line-height:1.6;">
                         ${escapeHtml(truncate(event.commentText, 300))}
                       </div>
-                      ${emailButton('View Comment  &#8594;', event.url)}
+                      ${emailButton('View Comment  →', event.url)}
                     </td></tr>
                 `),
       };
@@ -364,18 +365,18 @@ function formatEmail(
       return {
         subject: `[OpenFrame] ${event.replyAuthor} replied on ${event.videoTitle}`,
         html: emailTemplate(`
-                    <tr>${emailHeading('&#8617;', 'New Reply')}</tr>
+                    <tr>${emailHeading('↵', 'New Reply')}</tr>
                     <tr><td style="padding:20px;">
                       <table cellpadding="0" cellspacing="0" style="width:100%;margin-bottom:16px;">
-                        ${emailRow('Project', escapeHtml(event.projectName), true)}
-                        ${emailRow('Video', escapeHtml(event.videoTitle), true)}
-                        ${emailRow('From', `<span style="color:${EMAIL_COLORS.text};font-weight:500;">${escapeHtml(event.replyAuthor)}</span> <span style="color:${EMAIL_COLORS.textDim};">&#8594;</span> ${escapeHtml(event.parentAuthor)}`)}
+                        ${emailRow('Project', event.projectName, true)}
+                        ${emailRow('Video', event.videoTitle, true)}
+                        ${emailRow('From', rawEmailHtml(`<span style="color:${EMAIL_COLORS.text};font-weight:500;">${escapeHtml(event.replyAuthor)}</span> <span style="color:${EMAIL_COLORS.textDim};">&#8594;</span> ${escapeHtml(event.parentAuthor)}`))}
                         ${emailRow('When', now)}
                       </table>
                       <div style="border-left:2px solid #7aa7ff;padding:10px 14px;margin:0 0 20px;background-color:#2f2f2f;color:#c6c6cc;font-size:13px;line-height:1.6;">
                         ${escapeHtml(truncate(event.replyText, 300))}
                       </div>
-                      ${emailButton('View Reply  &#8594;', event.url)}
+                      ${emailButton('View Reply  →', event.url)}
                     </td></tr>
                 `),
       };
@@ -383,18 +384,18 @@ function formatEmail(
       return {
         subject: `[OpenFrame] Approval requested for ${event.versionLabel} in ${event.projectName}`,
         html: emailTemplate(`
-                    <tr>${emailHeading('&#10003;', 'Approval Requested')}</tr>
+                    <tr>${emailHeading('✓', 'Approval Requested')}</tr>
                     <tr><td style="padding:20px;">
                       ${emailHighlight(`A new approval request is waiting for your response.`)}
                       <table cellpadding="0" cellspacing="0" style="width:100%;margin-bottom:16px;">
-                        ${emailRow('Project', escapeHtml(event.projectName), true)}
-                        ${emailRow('Video', escapeHtml(event.videoTitle), true)}
-                        ${emailRow('Version', escapeHtml(event.versionLabel))}
-                        ${emailRow('Requested by', escapeHtml(event.requestedBy))}
+                        ${emailRow('Project', event.projectName, true)}
+                        ${emailRow('Video', event.videoTitle, true)}
+                        ${emailRow('Version', event.versionLabel)}
+                        ${emailRow('Requested by', event.requestedBy)}
                         ${emailRow('When', now)}
                       </table>
                       ${event.message ? `<div style="border-left:2px solid #7aa7ff;padding:10px 14px;margin:0 0 20px;background-color:#2f2f2f;color:#c6c6cc;font-size:13px;line-height:1.6;">${escapeHtml(truncate(event.message, 300))}</div>` : ''}
-                      ${emailButton('Review Request  &#8594;', event.url)}
+                      ${emailButton('Review Request  →', event.url)}
                     </td></tr>
                 `),
       };
@@ -402,18 +403,18 @@ function formatEmail(
       return {
         subject: `[OpenFrame] Approval ${event.action} by ${event.actorName}`,
         html: emailTemplate(`
-                    <tr>${emailHeading('&#10003;', 'Approval Update')}</tr>
+                    <tr>${emailHeading('✓', 'Approval Update')}</tr>
                     <tr><td style="padding:20px;">
-                      ${emailHighlight(`${escapeHtml(event.actorName)} ${escapeHtml(event.action)} this request.`)}
+                      ${emailHighlight(`${event.actorName} ${event.action} this request.`)}
                       <table cellpadding="0" cellspacing="0" style="width:100%;margin-bottom:16px;">
-                        ${emailRow('Project', escapeHtml(event.projectName), true)}
-                        ${emailRow('Video', escapeHtml(event.videoTitle), true)}
-                        ${emailRow('Version', escapeHtml(event.versionLabel))}
-                        ${emailRow('Action', escapeHtml(`${event.actorName} ${event.action}`))}
+                        ${emailRow('Project', event.projectName, true)}
+                        ${emailRow('Video', event.videoTitle, true)}
+                        ${emailRow('Version', event.versionLabel)}
+                        ${emailRow('Action', `${event.actorName} ${event.action}`)}
                         ${emailRow('When', now)}
                       </table>
                       ${event.note ? `<div style="border-left:2px solid #7aa7ff;padding:10px 14px;margin:0 0 20px;background-color:#2f2f2f;color:#c6c6cc;font-size:13px;line-height:1.6;">${escapeHtml(truncate(event.note, 300))}</div>` : ''}
-                      ${emailButton('Open Request  &#8594;', event.url)}
+                      ${emailButton('Open Request  →', event.url)}
                     </td></tr>
                 `),
       };
@@ -421,17 +422,17 @@ function formatEmail(
       return {
         subject: `[OpenFrame] Approval completed for ${event.versionLabel}`,
         html: emailTemplate(`
-                    <tr>${emailHeading('&#10003;', 'Approval Completed')}</tr>
+                    <tr>${emailHeading('✓', 'Approval Completed')}</tr>
                     <tr><td style="padding:20px;">
                       ${emailHighlight(`All approvers accepted this request.`)}
                       <table cellpadding="0" cellspacing="0" style="width:100%;margin-bottom:20px;">
-                        ${emailRow('Project', escapeHtml(event.projectName), true)}
-                        ${emailRow('Video', escapeHtml(event.videoTitle), true)}
-                        ${emailRow('Version', escapeHtml(event.versionLabel))}
+                        ${emailRow('Project', event.projectName, true)}
+                        ${emailRow('Video', event.videoTitle, true)}
+                        ${emailRow('Version', event.versionLabel)}
                         ${emailRow('Approvals', String(event.approvedByCount))}
                         ${emailRow('When', now)}
                       </table>
-                      ${emailButton('Open Version  &#8594;', event.url)}
+                      ${emailButton('Open Version  →', event.url)}
                     </td></tr>
                 `),
       };
@@ -439,18 +440,18 @@ function formatEmail(
       return {
         subject: `[OpenFrame] Approval rejected by ${event.rejectedBy}`,
         html: emailTemplate(`
-                    <tr>${emailHeading('&#9940;', 'Approval Rejected')}</tr>
+                    <tr>${emailHeading('⛔', 'Approval Rejected')}</tr>
                     <tr><td style="padding:20px;">
-                      ${emailHighlight(`${escapeHtml(event.rejectedBy)} rejected this request.`)}
+                      ${emailHighlight(`${event.rejectedBy} rejected this request.`)}
                       <table cellpadding="0" cellspacing="0" style="width:100%;margin-bottom:16px;">
-                        ${emailRow('Project', escapeHtml(event.projectName), true)}
-                        ${emailRow('Video', escapeHtml(event.videoTitle), true)}
-                        ${emailRow('Version', escapeHtml(event.versionLabel))}
-                        ${emailRow('Rejected by', escapeHtml(event.rejectedBy))}
+                        ${emailRow('Project', event.projectName, true)}
+                        ${emailRow('Video', event.videoTitle, true)}
+                        ${emailRow('Version', event.versionLabel)}
+                        ${emailRow('Rejected by', event.rejectedBy)}
                         ${emailRow('When', now)}
                       </table>
                       ${event.note ? `<div style="border-left:2px solid #7aa7ff;padding:10px 14px;margin:0 0 20px;background-color:#2f2f2f;color:#c6c6cc;font-size:13px;line-height:1.6;">${escapeHtml(truncate(event.note, 300))}</div>` : ''}
-                      ${emailButton('Open Request  &#8594;', event.url)}
+                      ${emailButton('Open Request  →', event.url)}
                     </td></tr>
                 `),
       };
@@ -462,7 +463,7 @@ function formatEmail(
  */
 export function testEmailHtml(): string {
   return emailTemplate(`
-        <tr>${emailHeading('&#10003;', 'Test Notification')}</tr>
+        <tr>${emailHeading('✓', 'Test Notification')}</tr>
         <tr><td style="padding:20px;">
           <p style="margin:0 0 8px;font-size:14px;color:${EMAIL_COLORS.text};">Email notifications are working.</p>
           <p style="margin:0;font-size:13px;color:${EMAIL_COLORS.textSecondary};">You&rsquo;ll receive emails when there&rsquo;s activity on your projects.</p>

lib/project-download.ts

@@ -86,10 +86,19 @@ function getSafeDirectDownloadUrl(rawUrl: string): string | null {
   }
 }
 
+// A file extension is appended after sanitizeFileName() has run, so it has to be safe on
+// its own: anything that is not a short alphanumeric run falls back. Slicing from the last
+// dot of a whole URL would otherwise let `https://example.com/download` contribute
+// `.com/download`, a path separator inside an archive entry name.
+const SAFE_EXTENSION = /^[a-z0-9]{1,10}$/i;
+
 function extensionFromUrl(url: string, fallback: string): string {
   const withoutQuery = url.split('?')[0] ?? url;
-  const ext = withoutQuery.includes('.') ? withoutQuery.slice(withoutQuery.lastIndexOf('.')) : '';
-  return ext || fallback;
+  const baseName = withoutQuery.slice(withoutQuery.lastIndexOf('/') + 1);
+  const dotIndex = baseName.lastIndexOf('.');
+  if (dotIndex <= 0) return fallback;
+  const ext = baseName.slice(dotIndex + 1);
+  return SAFE_EXTENSION.test(ext) ? `.${ext.toLowerCase()}` : fallback;
 }
 
 type VersionRow = {
@@ -162,18 +171,15 @@ function buildAssetFileName(videoIndex: number, videoTitle: string, asset: Asset
 
   if (asset.provider === VideoAssetProvider.R2_IMAGE) {
     const fileName = extractImageFileNameFromProxyUrl(asset.sourceUrl);
-    const ext = fileName?.includes('.') ? fileName.slice(fileName.lastIndexOf('.')) : '.png';
-    return `${stem}${ext}`;
+    return `${stem}${extensionFromUrl(fileName ?? '', '.png')}`;
   }
   if (asset.provider === VideoAssetProvider.R2_AUDIO) {
     const fileName = extractAudioFileNameFromProxyUrl(asset.sourceUrl);
-    const ext = fileName?.includes('.') ? fileName.slice(fileName.lastIndexOf('.')) : '.webm';
-    return `${stem}${ext}`;
+    return `${stem}${extensionFromUrl(fileName ?? '', '.webm')}`;
   }
   if (asset.provider === VideoAssetProvider.R2_VIDEO) {
     const fileName = extractVideoFileNameFromProxyUrl(asset.sourceUrl);
-    const ext = fileName?.includes('.') ? fileName.slice(fileName.lastIndexOf('.')) : '.mp4';
-    return `${stem}${ext}`;
+    return `${stem}${extensionFromUrl(fileName ?? '', '.mp4')}`;
   }
   if (asset.provider === VideoAssetProvider.BUNNY) {
     return `${stem}.mp4`;
@@ -189,9 +195,10 @@ function versionDownloadUrl(version: VersionRow): string | null {
     return `/api/versions/${version.id}/download?source=original`;
   }
   if (version.providerId === 'r2') {
-    if (version.originalUrl.startsWith('/api/upload/video/')) {
-      return version.originalUrl;
-    }
+    // Only the strict proxy-path shape is accepted. A `startsWith` check here would let
+    // `/api/upload/video/clip.mp4/../../../../etc/passwd` through as a download URL.
+    // Every r2 version is written through finalizeR2VideoUpload(), which stores exactly
+    // this shape, so nothing legitimate is lost.
     const fileName = extractVideoFileNameFromProxyUrl(version.originalUrl);
     if (fileName) return `/api/upload/video/${fileName}`;
   }
@@ -294,6 +301,11 @@ export function validateProjectDownloadManifest(manifest: ProjectDownloadManifes
   }
 
   if (manifest.totalBytes) {
+    // The contract is to return a message, so a malformed total has to become one rather
+    // than a SyntaxError escaping into the route as a 500.
+    if (!/^\d+$/.test(manifest.totalBytes)) {
+      return 'Could not determine the size of this download';
+    }
     const knownTotal = BigInt(manifest.totalBytes);
     if (knownTotal > maxBytes) {
       const maxGiB = Number(maxBytes / BigInt(1024 * 1024 * 1024));

lib/r2-media-proxy.ts

@@ -19,6 +19,17 @@ type ProxyR2MediaOptions = {
   internalErrorMessage: string;
 };
 
+// Every key this proxy is ever asked for is a prefix plus a stored uuid file name. The
+// guard lives here rather than in each caller so it travels with the function: all three
+// call sites gate the file name on a strict pattern first, and a fourth that forgot would
+// otherwise hand a traversal straight to GetObject.
+const SAFE_MEDIA_OBJECT_KEY =
+  /^(?:images|voice|videos)\/[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\.[a-z0-9]+$/i;
+
+export function isSafeR2MediaKey(key: string): boolean {
+  return SAFE_MEDIA_OBJECT_KEY.test(key);
+}
+
 type R2LikeError = {
   name?: string;
   Code?: string;
@@ -92,6 +103,10 @@ export async function proxyR2MediaObject({
   notFoundLabel = 'File',
   internalErrorMessage,
 }: ProxyR2MediaOptions): Promise<NextResponse> {
+  if (!isSafeR2MediaKey(key)) {
+    return apiErrors.badRequest('Invalid media key');
+  }
+
   const range = request.headers.get('range');
   const ifRange = request.headers.get('if-range');
   const commandInput: GetObjectCommandInput = {

lib/r2-upload-session.ts

@@ -32,12 +32,21 @@ export async function createR2UploadSession(input: CreateR2UploadSessionInput) {
   });
 }
 
+/**
+ * Cancels an initiated session whether or not it has expired.
+ *
+ * The expiry condition that used to be here made the update match zero rows once a
+ * session lapsed, so the status stayed INITIATED and `consumedAt` stayed null for good.
+ * The r2-init DELETE route releases the quota reservation only when the update reports a
+ * row, so every abandoned upload held its reserved bytes against the user's quota
+ * permanently, and no sweeper reclaims them. Cancelling something already expired is the
+ * case that most needs to work.
+ */
 export async function cancelR2UploadSession(sessionId: string) {
   return db.videoUploadSession.updateMany({
     where: {
       id: sessionId,
       status: 'INITIATED',
-      expiresAt: { gt: new Date() },
     },
     data: {
       status: 'CANCELLED',

lib/r2-upload-token.ts

@@ -100,6 +100,12 @@ export function verifyR2UploadToken(token: string, subject: R2UploadTokenSubject
 }
 
 export function parseR2UploadToken(token: string): R2UploadTokenPayload | null {
+  // Resolved before the try. A server booted with neither R2_UPLOAD_TOKEN_SECRET nor
+  // NEXTAUTH_SECRET set is misconfigured, and swallowing that throw made it answer
+  // "invalid token" for every upload grant: a total upload outage that looks like a
+  // client bug and says nothing about why.
+  const secret = getR2UploadTokenSecret();
+
   try {
     const parts = token.split('.');
     if (parts.length !== 2) return null;
@@ -107,7 +113,7 @@ export function parseR2UploadToken(token: string): R2UploadTokenPayload | null {
     const [encodedPayload, providedSignature] = parts;
     if (!encodedPayload || !providedSignature) return null;
 
-    const expectedSignature = signPayload(encodedPayload, getR2UploadTokenSecret());
+    const expectedSignature = signPayload(encodedPayload, secret);
     const providedBuffer = Buffer.from(providedSignature, 'utf8');
     const expectedBuffer = Buffer.from(expectedSignature, 'utf8');
 

lib/r2.ts

@@ -12,11 +12,13 @@ import {
   PutObjectCommand,
   UploadPartCommand,
   S3Client,
+  type CORSRule,
 } from '@aws-sdk/client-s3';
 import { getSignedUrl } from '@aws-sdk/s3-request-presigner';
 import { VIDEO_OBJECT_KEY_PREFIX } from '@/lib/video-upload-validation';
 
 const IMAGE_OBJECT_KEY_PREFIX = 'images/';
+const AUDIO_OBJECT_KEY_PREFIX = 'voice/';
 
 const R2_ACCOUNT_ID = process.env.R2_ACCOUNT_ID;
 const R2_ACCESS_KEY_ID = process.env.R2_ACCESS_KEY_ID;
@@ -104,12 +106,16 @@ export const r2Client = new Proxy({} as S3Client, {
   get(_target, prop, receiver) {
     if (prop === 'destroy') {
       return () => {
-        if (!cachedR2Client) return;
-        cachedR2Client.destroy();
-        cachedR2Client = null;
-        if (!cachedR2PresignClient) return;
-        cachedR2PresignClient.destroy();
-        cachedR2PresignClient = null;
+        // Both are destroyed independently. Returning early when the send client was
+        // never created leaked the presign client in a process that only ever presigned.
+        if (cachedR2Client) {
+          cachedR2Client.destroy();
+          cachedR2Client = null;
+        }
+        if (cachedR2PresignClient) {
+          cachedR2PresignClient.destroy();
+          cachedR2PresignClient = null;
+        }
       };
     }
 
@@ -234,13 +240,22 @@ export async function ensureR2UploadCors(extraOrigins: string[] = []): Promise<s
     MaxAgeSeconds: 3600,
   };
 
+  // The catch covers the read only. Wrapping the write in it too meant a transient write
+  // failure was mistaken for "this bucket has no CORS config", and the retry below then
+  // sent the managed rule on its own, discarding whatever the bucket already had.
+  let existingRules: CORSRule[] | null = null;
   try {
     const existing = await r2Client.send(
       new GetBucketCorsCommand({
         Bucket: R2_BUCKET_NAME,
       })
     );
-    const existingRules = existing.CORSRules ?? [];
+    existingRules = existing.CORSRules ?? [];
+  } catch {
+    // No CORS config yet, or insufficient permissions to read — write the managed rule.
+  }
+
+  if (existingRules) {
     if (existingRules.some((rule) => corsRulesMatchOrigins(rule, allowedOrigins))) {
       return allowedOrigins;
     }
@@ -254,8 +269,6 @@ export async function ensureR2UploadCors(extraOrigins: string[] = []): Promise<s
       })
     );
     return allowedOrigins;
-  } catch {
-    // No CORS config yet, or insufficient permissions to read — attempt to write.
   }
 
   await r2Client.send(
@@ -291,7 +304,13 @@ export async function createPresignedVideoPutUrl(
     ContentLength: Number(contentLength),
   });
 
-  return getSignedUrl(getOrCreateR2PresignClient(), command, { expiresIn: expiresInSeconds });
+  return getSignedUrl(getOrCreateR2PresignClient(), command, {
+    expiresIn: expiresInSeconds,
+    // Passing ContentType to the command is not enough: unless the header is signable the
+    // grant does not bind it, and whoever holds the url can put any media type at the key.
+    // The client sends the same value back, so the signature covers what actually lands.
+    signableHeaders: new Set(['content-type']),
+  });
 }
 
 export async function createMultipartVideoUpload(
@@ -397,7 +416,12 @@ export async function createPresignedImagePutUrl(
     ContentType: contentType,
   });
 
-  return getSignedUrl(getOrCreateR2PresignClient(), command, { expiresIn: expiresInSeconds });
+  return getSignedUrl(getOrCreateR2PresignClient(), command, {
+    expiresIn: expiresInSeconds,
+    // Without this the grant binds only the host, so an image upload url accepts any
+    // media type at an `images/` key the app then serves as an image.
+    signableHeaders: new Set(['content-type']),
+  });
 }
 
 export async function headVideoObject(key: string): Promise<{
@@ -464,7 +488,14 @@ export async function readVideoObjectBytes(
 }
 
 function assertAllowedObjectKey(key: string): void {
-  if (!key.startsWith(VIDEO_OBJECT_KEY_PREFIX) && !key.startsWith(IMAGE_OBJECT_KEY_PREFIX)) {
+  // `voice/` belongs here because uploadAudio() writes under it. Leaving it out meant
+  // deleteR2Object('voice/...') always threw, so a voice note attached to a comment could
+  // never be removed by the module that stored it and outlived the comment in the bucket.
+  if (
+    !key.startsWith(VIDEO_OBJECT_KEY_PREFIX) &&
+    !key.startsWith(IMAGE_OBJECT_KEY_PREFIX) &&
+    !key.startsWith(AUDIO_OBJECT_KEY_PREFIX)
+  ) {
     throw new Error('Invalid object key');
   }
 }

lib/rate-limit.ts

@@ -1,6 +1,7 @@
+import { createHash } from 'crypto';
 import { db } from '@/lib/db';
 import { NextResponse } from 'next/server';
-import { logError } from '@/lib/logger';
+import { logError, logWarn } from '@/lib/logger';
 
 const RATE_LIMIT_CLEANUP_INTERVAL_MS = 5 * 60 * 1000;
 
@@ -33,6 +34,19 @@ if (process.env.NODE_ENV === 'production' && isRateLimitDisabled()) {
   );
 }
 
+// Without a proxy mode every caller resolves to 127.0.0.1, so the limiter counts the whole
+// world in one bucket. That is the deliberate trade-off (trusting a spoofable header is
+// worse), but a deployment behind a proxy should know it is running with global rather
+// than per-client limits rather than discover it under load.
+if (process.env.NODE_ENV === 'production' && !process.env.TRUSTED_PROXY_MODE?.trim()) {
+  logWarn(
+    'TRUSTED_PROXY_MODE is not set. Every request resolves to 127.0.0.1, so rate limits ' +
+      'apply per process rather than per client. Set TRUSTED_PROXY_MODE=cloudflare or ' +
+      'TRUSTED_PROXY_MODE=nginx once you have confirmed your proxy overwrites the ' +
+      'corresponding header on every inbound request.'
+  );
+}
+
 // Industry-standard rate limit defaults per action
 export const RATE_LIMIT_CONFIGS: Record<string, RateLimitConfig> = {
   // Auth — strict to prevent brute force / credential stuffing
@@ -95,6 +109,21 @@ export const RATE_LIMIT_CONFIGS: Record<string, RateLimitConfig> = {
   api: { windowMs: 60 * 1000, maxRequests: 100 }, // 100 per minute
 };
 
+// Column widths of rate_limits.key and rate_limits.action in prisma/schema.prisma. A value
+// wider than its column would fail the INSERT with SQLSTATE 22001.
+const RATE_LIMIT_KEY_MAX_LENGTH = 255;
+const RATE_LIMIT_ACTION_MAX_LENGTH = 50;
+
+/**
+ * Fits a value to its column without ever giving up on counting it. A SHA-256 hex digest
+ * is 64 characters, so it is truncated for the narrower action column; 50 hex characters
+ * is 200 bits, far past any collision that matters for a rate limit bucket.
+ */
+function fitToColumn(value: string, maxLength: number): string {
+  if (value.length <= maxLength) return value;
+  return createHash('sha256').update(value).digest('hex').slice(0, maxLength);
+}
+
 /**
  * Check and update rate limit for a given key and action
  * Uses PostgreSQL UNLOGGED table for performance
@@ -116,12 +145,12 @@ export async function checkRateLimit(
 
   const windowSeconds = Math.floor(windowMs / 1000);
 
-  // Validate inputs before passing to query — defence in depth.
-  // Prisma's tagged template $queryRaw already parameterizes these values,
-  // but we enforce sane bounds to reject obviously malicious input.
-  if (key.length > 256 || action.length > 64) {
-    return { allowed: true, remaining: maxRequests, resetAt: new Date(Date.now() + windowMs) };
-  }
+  // Anything wider than its column is replaced by a digest rather than skipped. Skipping
+  // meant the limit stopped applying altogether, and letting the value through meant the
+  // INSERT failed with SQLSTATE 22001 and the catch below allowed the request anyway.
+  // Both were fail-open. A digest is stable, so the same caller keeps the same bucket.
+  const storedKey = fitToColumn(key, RATE_LIMIT_KEY_MAX_LENGTH);
+  const storedAction = fitToColumn(action, RATE_LIMIT_ACTION_MAX_LENGTH);
 
   try {
     // Atomic upsert with window check
@@ -134,7 +163,7 @@ export async function checkRateLimit(
       }>
     >`
             INSERT INTO rate_limits (key, action, count, window_start)
-            VALUES (${key}, ${action}, 1, NOW())
+            VALUES (${storedKey}, ${storedAction}, 1, NOW())
             ON CONFLICT (key, action) DO UPDATE SET
                 count = CASE 
                     WHEN rate_limits.window_start < NOW() - (${windowSeconds} || ' seconds')::INTERVAL 

lib/route-access.ts

@@ -9,15 +9,19 @@ const LOGIN_REDIRECT = '/login';
 const FORBIDDEN_REDIRECT = '/dashboard';
 const BILLING_REDIRECT = '/settings';
 
-function redirectForMissingAuth() {
+// `never` rather than `void`, so a caller that puts a second redirect after one of these
+// gets a compile error instead of silently unreachable code. `redirect()` throws, and
+// these are authorization decisions: a helper that ever returned would let the branch
+// below it run.
+function redirectForMissingAuth(): never {
   redirect(LOGIN_REDIRECT);
 }
 
-function redirectForForbidden() {
+function redirectForForbidden(): never {
   redirect(FORBIDDEN_REDIRECT);
 }
 
-function redirectForBilling() {
+function redirectForBilling(): never {
   redirect(BILLING_REDIRECT);
 }
 
@@ -46,7 +50,7 @@ async function assertProjectAccessOrRedirect(
 
   ensureGuestPolicy({ userId, intent, allowPublicView });
 
-  const access = await checkProjectAccess(project, userId, { intent });
+  const access = await checkProjectAccess(project, userId);
 
   if (!access.hasAccess) {
     if (!userId) {
@@ -162,15 +166,22 @@ export async function requireWorkspaceAccessOrRedirect(options: {
 
   const access = await checkWorkspaceAccess(workspace, resolvedUserId);
 
+  // Only the owner is sent to billing. Keying this off the owner's billing status alone
+  // made the redirect target an oracle: a signed-in stranger probing workspace ids landed
+  // on /dashboard when the owner was paying and on /settings when the owner had lapsed,
+  // which reads off whose subscription is in arrears. It also sent a member whose owner
+  // had lapsed to their own billing page, where nothing they can do resolves it.
+  const ownerWithLapsedBilling = access.isOwner && !access.ownerBillingActive;
+
   if (!access.hasAccess) {
-    if (!access.ownerBillingActive) {
+    if (ownerWithLapsedBilling) {
       redirectForBilling();
     }
     redirectForForbidden();
   }
 
   if (intent === 'manage' && !access.canEdit) {
-    if (!access.ownerBillingActive) {
+    if (ownerWithLapsedBilling) {
       redirectForBilling();
     }
     redirectForForbidden();

lib/validation.ts

@@ -54,8 +54,17 @@ export function validateAnnotationStrokes(
     }
 
     if (typeof color !== 'string' || !ANNOTATION_COLOR_RE.test(color)) return null;
-    if (typeof width !== 'number' || width < MIN_STROKE_WIDTH || width > MAX_STROKE_WIDTH)
+    // isFinite as well as the bounds: both comparisons are false for NaN, so a NaN width
+    // cleared the range check and reached the stored annotation JSON, where
+    // JSON.stringify renders it as null. Coordinates already had this guard.
+    if (
+      typeof width !== 'number' ||
+      !isFinite(width) ||
+      width < MIN_STROKE_WIDTH ||
+      width > MAX_STROKE_WIDTH
+    ) {
       return null;
+    }
 
     result.push({ points: safePoints, color, width });
   }

lib/video-assets.ts

@@ -38,6 +38,13 @@ export type VideoAssetAccessContext = {
     };
   };
   hasViewAccess: boolean;
+  /**
+   * Whether the viewer has any relationship to the project: owner, project member,
+   * workspace member, or a valid share link. Distinguishes "you may not" from "there is
+   * no such thing", so a route can answer 404 for another tenant's id without answering
+   * 404 to somebody whose access merely lapsed.
+   */
+  viewerBelongsToProject: boolean;
   canUploadAssets: boolean;
   canDownloadAssets: boolean;
   canManageAssets: boolean;
@@ -98,18 +105,6 @@ export function extractVideoFileNameFromProxyUrl(url: string): string | null {
   return filename || null;
 }
 
-export function mediaUrlToR2Key(url: string): string | null {
-  if (url.includes(IMAGE_PROXY_PREFIX)) {
-    const filename = url.slice(url.indexOf(IMAGE_PROXY_PREFIX) + IMAGE_PROXY_PREFIX.length);
-    return filename ? `images/${filename}` : null;
-  }
-  if (url.includes(AUDIO_PROXY_PREFIX)) {
-    const filename = url.slice(url.indexOf(AUDIO_PROXY_PREFIX) + AUDIO_PROXY_PREFIX.length);
-    return filename ? `voice/${filename}` : null;
-  }
-  return null;
-}
-
 export function canDeleteAssetForViewer(
   asset: Pick<VideoAsset, 'uploadedByUserId' | 'uploadedByGuestIdentityId'>,
   viewer: Pick<
@@ -194,9 +189,13 @@ export async function getVideoAssetAccessContext(
   const viewerUserId = session?.user?.id ?? null;
   const viewerGuestIdentityId = viewerUserId ? null : getGuestIdentityFromRequest(request);
 
+  const viewerBelongsToProject =
+    access.isOwner || access.isProjectMember || access.isWorkspaceMember || shareAccess.hasAccess;
+
   return {
     video,
     hasViewAccess,
+    viewerBelongsToProject,
     canUploadAssets,
     canDownloadAssets,
     canManageAssets: access.canEdit,

lib/video-delete.ts

@@ -9,16 +9,32 @@ type BunnyRef = {
   videoId: string;
 };
 
+type CleanupInput = {
+  bunny: Awaited<ReturnType<typeof cleanupBunnyStreamVideosBestEffort>>;
+  r2: Awaited<ReturnType<typeof deleteMediaFilesBestEffort>>;
+};
+
+/**
+ * Storage refused at least one delete, so the rows were left in place and nothing was
+ * removed. Carries the cleanup detail so the route can log which keys failed.
+ */
+export class VideoStorageCleanupError extends Error {
+  readonly cleanupInput: CleanupInput;
+
+  constructor(cleanupInput: CleanupInput) {
+    super('STORAGE_CLEANUP_FAILED');
+    this.name = 'VideoStorageCleanupError';
+    this.cleanupInput = cleanupInput;
+  }
+}
+
 export async function deleteProjectVideosWithCleanup(
   projectId: string,
   videoIds: string[]
 ): Promise<{
   deletedCount: number;
   cleanupWarnings: CleanupWarnings | undefined;
-  cleanupInput: {
-    bunny: Awaited<ReturnType<typeof cleanupBunnyStreamVideosBestEffort>>;
-    r2: Awaited<ReturnType<typeof deleteMediaFilesBestEffort>>;
-  };
+  cleanupInput: CleanupInput;
 }> {
   const uniqueVideoIds = [...new Set(videoIds)];
   if (uniqueVideoIds.length === 0) {
@@ -66,15 +82,11 @@ export async function deleteProjectVideosWithCleanup(
     );
   }
 
-  await db.video.deleteMany({
-    where: {
-      projectId,
-      id: { in: uniqueVideoIds },
-    },
-  });
-
-  revalidatePath(`/projects/${projectId}`);
-
+  // Storage first, rows second. The other order committed the deleteMany before the R2 and
+  // Bunny calls ran, with nothing spanning the two, so a refused storage DELETE left the
+  // object in the bucket with no row pointing at it and no way to retry: the video id no
+  // longer resolved to anything. Leaving the rows in place instead keeps the delete
+  // repeatable, and a second attempt cleans up whatever the first one could not.
   const [bunnyCleanupResult, r2CleanupResult] = await Promise.all([
     cleanupBunnyStreamVideosBestEffort(bunnyRefs),
     deleteMediaFilesBestEffort(mediaUrls),
@@ -85,9 +97,23 @@ export async function deleteProjectVideosWithCleanup(
     r2: r2CleanupResult,
   };
 
+  const cleanupWarnings = buildCleanupWarnings(cleanupInput);
+  if (cleanupWarnings) {
+    throw new VideoStorageCleanupError(cleanupInput);
+  }
+
+  await db.video.deleteMany({
+    where: {
+      projectId,
+      id: { in: uniqueVideoIds },
+    },
+  });
+
+  revalidatePath(`/projects/${projectId}`);
+
   return {
     deletedCount: videos.length,
-    cleanupWarnings: buildCleanupWarnings(cleanupInput),
+    cleanupWarnings,
     cleanupInput,
   };
 }

lib/video-providers/direct.ts

@@ -39,12 +39,13 @@ export const directProvider: VideoProvider = {
   getEmbedUrl(videoId: string, options: EmbedOptions = {}): string {
     // For direct videos, we'll use HTML5 video player
     // The videoId IS the URL for direct uploads
-    const params = new URLSearchParams();
+    // A direct video is played by the HTML5 element, which reads the start time from the
+    // media fragment. The URLSearchParams that used to be built here never reached the
+    // returned string; only its emptiness was tested, and the fragment then carried the
+    // unfloored value, so the floor accomplished nothing.
+    const startTime = options.startTime ? Math.floor(options.startTime) : 0;
 
-    if (options.startTime) params.set('t', String(Math.floor(options.startTime)));
-
-    const queryString = params.toString();
-    return `${videoId}${queryString ? `#t=${options.startTime}` : ''}`;
+    return `${videoId}${startTime > 0 ? `#t=${startTime}` : ''}`;
   },
 
   getThumbnailUrl(videoId: string): string {

lib/video-upload-validation.ts

@@ -36,20 +36,24 @@ export function getVideoExtensionFromFileName(fileName: string): string | null {
   return ext;
 }
 
+/**
+ * The file name decides, always. A declared MIME type is a client claim, so trusting it
+ * on its own let `payload.exe` through as long as it said `video/mp4`. The declared type
+ * is only consulted to pick between two types that share an extension.
+ */
 export function resolveVideoContentType(fileName: string, mime: string | undefined): string | null {
+  const ext = getVideoExtensionFromFileName(fileName);
+  if (!ext) return null;
+
+  const typeFromName = EXT_TO_MIME[ext];
+  if (!typeFromName) return null;
+
   const normalizedMime = normalizeVideoMime(mime);
-  if (normalizedMime) {
-    const extFromMime = getVideoExtensionFromMime(normalizedMime);
-    const extFromName = getVideoExtensionFromFileName(fileName);
-    if (extFromMime && extFromName && extFromMime !== extFromName) {
-      return EXT_TO_MIME[extFromName] ?? normalizedMime;
-    }
+  if (normalizedMime && getVideoExtensionFromMime(normalizedMime) === ext) {
     return normalizedMime;
   }
 
-  const ext = getVideoExtensionFromFileName(fileName);
-  if (!ext) return null;
-  return EXT_TO_MIME[ext] ?? null;
+  return typeFromName;
 }
 
 export function isAllowedVideoFile(fileName: string, mime: string | undefined): boolean {

package.json

@@ -24,7 +24,7 @@
     "test:db:up": "podman compose -f docker-compose.test.yml up -d --wait postgres-test",
     "test:db:down": "podman compose -f docker-compose.test.yml down -v",
     "test:db:bootstrap": "bun run tests/setup/db-bootstrap.ts",
-    "prepare": "husky",
+    "prepare": "husky || true",
     "postinstall": "prisma generate",
     "db:generate": "prisma generate",
     "db:push": "prisma db push",
@@ -96,7 +96,6 @@
     "shadcn": "^3.8.3",
     "tailwindcss": "^4",
     "typescript": "^5",
-    "vite-tsconfig-paths": "^6.1.1",
     "vitest": "^4.1.10"
   },
   "lint-staged": {

tests/api/assets-authz.test.ts

@@ -442,7 +442,10 @@ describe('DELETE /api/videos/[videoId]/assets/[assetId]', () => {
 // owner has turned exports off. Both are pinned, because merging them would look
 // like a tidy-up and would quietly hand the files to every viewer.
 describe('GET /api/videos/[videoId]/assets/[assetId]/download', () => {
-  it('returns 403 to a signed-in stranger', async () => {
+  // 404 rather than 403 for a caller with no relationship to the project: a 403 confirms
+  // the id exists. The comment export route has always answered 404 for the identical
+  // shape, and the three download paths now agree.
+  it('returns 404 to a signed-in stranger', async () => {
     const fixture = await seedAsset({ allowDownloads: true });
     await seedProject();
     const stranger = await createUser();
@@ -454,8 +457,7 @@ describe('GET /api/videos/[videoId]/assets/[assetId]/download', () => {
       { videoId: fixture.video.id, assetId: fixture.asset.id }
     );
 
-    expect(response.status).toBe(403);
-    expect(await readError(response)).toContain('Access denied');
+    expect(response.status).toBe(404);
   });
 
   it('returns 403 to a project COMMENTATOR when downloads are disabled', async () => {
@@ -529,7 +531,7 @@ describe('GET /api/videos/[videoId]/assets/[assetId]/download', () => {
     expect(response.status).toBe(404);
   });
 
-  it('returns 403 for a foreign asset reached through its own foreign video id', async () => {
+  it('returns 404 for a foreign asset reached through its own foreign video id', async () => {
     const mine = await seedAsset({ allowDownloads: true });
     const theirs = await seedAsset({ allowDownloads: true });
     signedInAs(mine.owner);
@@ -540,7 +542,7 @@ describe('GET /api/videos/[videoId]/assets/[assetId]/download', () => {
       { videoId: theirs.video.id, assetId: theirs.asset.id }
     );
 
-    expect(response.status).toBe(403);
+    expect(response.status).toBe(404);
   });
 });
 

tests/api/auth-matrix.test.ts

@@ -765,7 +765,21 @@ const NON_AUTHORIZATION_REFUSALS = new Map<string, string>();
  * fails and tells you to delete it, so nothing can rot into a permanent
  * exemption.
  */
-const NOT_FOUND_IS_THE_GUARD = new Map<string, string>();
+const NOT_FOUND_IS_THE_GUARD = new Map<string, string>([
+  // Both download routes take a bare resource id with no project in the path, so a 403
+  // for an id belonging to another tenant would confirm that the id exists. They answer
+  // 404 to any caller with no relationship to the project, which is what
+  // versions/[versionId]/comments/export has always done for the identical shape.
+  // Somebody who does belong, an owner whose billing lapsed for instance, still gets 403.
+  [
+    'GET versions/[versionId]/download/route.ts',
+    'hides whether the version id exists from a caller with no relationship to it',
+  ],
+  [
+    'GET videos/[videoId]/assets/[assetId]/download/route.ts',
+    'hides whether the video id exists from a caller with no relationship to it',
+  ],
+]);
 
 function discoverRouteModules(): string[] {
   const apiDir = path.join(REPO_ROOT, 'app', 'api');

tests/api/download-authz.test.ts

@@ -332,7 +332,10 @@ describe('GET /api/projects/[projectId]/download', () => {
 // DownloadEgressEvent row was written, because that row is the billing record: a
 // refusal that still bills the workspace owner would be its own bug.
 describe('GET /api/versions/[versionId]/download', () => {
-  it('returns 403 to a signed-in stranger and records no egress', async () => {
+  // 404 rather than 403 for a caller with no relationship to the project: a 403 would
+  // confirm the id exists. The comment export route has always answered 404 for the
+  // identical shape, and the three download paths now agree.
+  it('returns 404 to a signed-in stranger and records no egress', async () => {
     const fixture = await seedDownloadable({ allowDownloads: true });
     await seedProject();
     const stranger = await createUser();
@@ -344,7 +347,7 @@ describe('GET /api/versions/[versionId]/download', () => {
       { versionId: fixture.version.id }
     );
 
-    expect(response.status).toBe(403);
+    expect(response.status).toBe(404);
     expect(await db.downloadEgressEvent.count()).toBe(0);
   });
 
@@ -429,9 +432,9 @@ describe('GET /api/versions/[versionId]/download', () => {
 
   // Straight identifier substitution. There is no projectId in this URL to
   // cross-check against, so the version id alone decides which project gets
-  // authorized. A caller who owns a perfectly good project of their own gets 403
+  // authorized. A caller who owns a perfectly good project of their own gets 404
   // for somebody else's version, and never learns whether it exists.
-  it('returns 403 for a version id belonging to another workspace', async () => {
+  it('returns 404 for a version id belonging to another workspace', async () => {
     const mine = await seedDownloadable({ allowDownloads: true });
     const theirs = await seedDownloadable({ allowDownloads: true });
     signedInAs(mine.owner);
@@ -442,7 +445,7 @@ describe('GET /api/versions/[versionId]/download', () => {
       { versionId: theirs.version.id }
     );
 
-    expect(response.status).toBe(403);
+    expect(response.status).toBe(404);
     expect(await db.downloadEgressEvent.count()).toBe(0);
   });
 

tests/api/invitations.test.ts

@@ -450,11 +450,10 @@ describe('acceptInvitationTokenForUser', () => {
     expect(membership.role).toBe('ADMIN');
   });
 
-  // The invited role wins over the role the member already holds, so accepting
-  // a COMMENTATOR invitation demotes a sitting workspace ADMIN. Pinned because
-  // it is a privilege change, and a surprising one: the accept link looks like
-  // it can only ever add access.
-  it('applies a COMMENTATOR invitation over an existing ADMIN membership', async () => {
+  // An invitation can only ever add access. Applying the invited role over an existing
+  // membership made it a privilege-change primitive: re-invite a sitting ADMIN at a lower
+  // role, get them to click the link once, and they are demoted.
+  it('leaves an existing ADMIN membership alone for a COMMENTATOR invitation', async () => {
     const scenario = await seedProject();
     const member = await createUser({ email: '[email protected]' });
     await addWorkspaceMember({
@@ -480,7 +479,66 @@ describe('acceptInvitationTokenForUser', () => {
     const membership = await db.workspaceMember.findUniqueOrThrow({
       where: { workspaceId_userId: { workspaceId: scenario.workspace.id, userId: member.id } },
     });
-    expect(membership.role).toBe('COMMENTATOR');
+    expect(membership.role).toBe('ADMIN');
+  });
+
+  it('leaves an existing project ADMIN alone for a COMMENTATOR invitation', async () => {
+    const scenario = await seedProject();
+    const member = await createUser({ email: '[email protected]' });
+    await addProjectMember({
+      projectId: scenario.project.id,
+      userId: member.id,
+      role: 'ADMIN',
+    });
+    const invitation = await createInvitation({
+      invitedById: scenario.owner.id,
+      email: '[email protected]',
+      scope: 'PROJECT',
+      projectId: scenario.project.id,
+      role: 'COMMENTATOR',
+    });
+
+    const result = await acceptInvitationTokenForUser({
+      token: invitation.token,
+      userId: member.id,
+      email: member.email!,
+    });
+
+    expect(result).toBe('accepted');
+    const membership = await db.projectMember.findUniqueOrThrow({
+      where: { projectId_userId: { projectId: scenario.project.id, userId: member.id } },
+    });
+    expect(membership.role).toBe('ADMIN');
+  });
+
+  // The other direction still has to work: an invitation is allowed to promote.
+  it('promotes an existing COMMENTATOR to ADMIN for an ADMIN invitation', async () => {
+    const scenario = await seedProject();
+    const member = await createUser({ email: '[email protected]' });
+    await addWorkspaceMember({
+      workspaceId: scenario.workspace.id,
+      userId: member.id,
+      role: 'COMMENTATOR',
+    });
+    const invitation = await createInvitation({
+      invitedById: scenario.owner.id,
+      email: '[email protected]',
+      scope: 'WORKSPACE',
+      workspaceId: scenario.workspace.id,
+      role: 'ADMIN',
+    });
+
+    const result = await acceptInvitationTokenForUser({
+      token: invitation.token,
+      userId: member.id,
+      email: member.email!,
+    });
+
+    expect(result).toBe('accepted');
+    const membership = await db.workspaceMember.findUniqueOrThrow({
+      where: { workspaceId_userId: { workspaceId: scenario.workspace.id, userId: member.id } },
+    });
+    expect(membership.role).toBe('ADMIN');
   });
 
   // The owner already outranks any membership row. Writing one would put them
@@ -532,11 +590,10 @@ describe('acceptInvitationTokenForUser', () => {
     );
   });
 
-  // Pins today's behaviour for a malformed row (scope WORKSPACE with no
-  // workspaceId): the caller is told "accepted" while nothing is granted and
-  // the invitation stays PENDING, so the accept page shows a success screen.
-  // See the report accompanying this suite.
-  it('reports accepted for a scoped invitation that points at nothing', async () => {
+  // A malformed row (scope WORKSPACE with no workspaceId) grants nothing. Reporting
+  // "accepted" for it showed the user a success screen for a no-op they had no way to
+  // detect, while the invitation stayed PENDING for good.
+  it('refuses a scoped invitation that points at nothing', async () => {
     const scenario = await seedProject();
     const invitee = await createUser({ email: '[email protected]' });
     const invitation = await createInvitation({
@@ -552,12 +609,32 @@ describe('acceptInvitationTokenForUser', () => {
       email: invitee.email!,
     });
 
-    expect(result).toBe('accepted');
+    expect(result).toBe('not_found');
     expect(await db.workspaceMember.count()).toBe(0);
     expect((await db.invitation.findUniqueOrThrow({ where: { id: invitation.id } })).status).toBe(
       'PENDING'
     );
   });
+
+  it('refuses a project invitation that points at nothing', async () => {
+    const scenario = await seedProject();
+    const invitee = await createUser({ email: '[email protected]' });
+    const invitation = await createInvitation({
+      invitedById: scenario.owner.id,
+      email: '[email protected]',
+      scope: 'PROJECT',
+      projectId: null,
+    });
+
+    const result = await acceptInvitationTokenForUser({
+      token: invitation.token,
+      userId: invitee.id,
+      email: invitee.email!,
+    });
+
+    expect(result).toBe('not_found');
+    expect(await db.projectMember.count()).toBe(0);
+  });
 });
 
 describe('acceptPendingInvitationsForUser', () => {

tests/api/lib-admin-stats.test.ts

@@ -437,16 +437,18 @@ describe('getCachedBunnyStorageStats', () => {
     expect(await getCachedBunnyStorageStats()).toEqual({ totalBytes: -1, byVideoId: {} });
   });
 
-  // The flag defaults to on, so a self-hosted deployment that never configured
-  // Bunny lands here: credentials missing while the feature is nominally
-  // enabled.
-  it('degrades to -1 when the Bunny credentials are not configured', async () => {
+  // The flag defaults to on, so a self-hosted deployment that never configured Bunny
+  // lands here: credentials missing while the feature is nominally enabled. That used to
+  // key on the flag alone, throw "Missing Bunny Stream credentials." out of
+  // getBunnyConfig() and report -1, which reads as "we could not measure" rather than
+  // "there is nothing to measure".
+  it('reports a genuine zero when the Bunny credentials are not configured', async () => {
     vi.stubEnv('BUNNY_STREAM_API_KEY', undefined);
     vi.stubEnv('BUNNY_STREAM_LIBRARY_ID', undefined);
     vi.stubEnv('NEXT_PUBLIC_BUNNY_STREAM_LIBRARY_ID', undefined);
     const calls = stubBunnyPages([]);
 
-    expect(await getCachedBunnyStorageStats()).toEqual({ totalBytes: -1, byVideoId: {} });
+    expect(await getCachedBunnyStorageStats()).toEqual({ totalBytes: 0, byVideoId: {} });
     expect(calls.urls).toEqual([]);
   });
 
@@ -875,16 +877,18 @@ describe('getCachedStripeStats', () => {
       pastDueUsers: 1,
       canceledUsers: 1,
       freeUsers: 3,
+      otherStatusUsers: 0,
       mrrCents: 3800,
       currency: 'eur',
     });
     expect(stripe.retrievedPriceIds).toEqual(['price_admin_stats_test']);
   });
 
-  // UNPAID, INCOMPLETE and INCOMPLETE_EXPIRED are real values of the enum that
-  // the report has no bucket for. They must not be silently folded into one of
-  // the five that are reported.
-  it('leaves statuses it does not report out of every bucket', async () => {
+  // UNPAID, INCOMPLETE and INCOMPLETE_EXPIRED are real values of the enum that used to
+  // belong to none of the reported buckets, so those users were counted nowhere and the
+  // five totals silently did not add up to the user table. They must not be folded into
+  // one of the five either.
+  it('counts the statuses the five named buckets do not cover', async () => {
     await createUser({ subscriptionStatus: 'UNPAID' });
     await createUser({ subscriptionStatus: 'INCOMPLETE' });
     await createUser({ subscriptionStatus: 'INCOMPLETE_EXPIRED' });
@@ -898,6 +902,7 @@ describe('getCachedStripeStats', () => {
       pastDueUsers: 0,
       canceledUsers: 0,
       freeUsers: 0,
+      otherStatusUsers: 3,
       mrrCents: 0,
       currency: 'usd',
     });
@@ -912,6 +917,7 @@ describe('getCachedStripeStats', () => {
       pastDueUsers: 0,
       canceledUsers: 0,
       freeUsers: 0,
+      otherStatusUsers: 0,
       mrrCents: 0,
       currency: 'usd',
     });
@@ -932,6 +938,7 @@ describe('getCachedStripeStats', () => {
       pastDueUsers: 0,
       canceledUsers: 0,
       freeUsers: 0,
+      otherStatusUsers: 0,
       mrrCents: 0,
       currency: 'usd',
     });

tests/api/lib-project-access.test.ts

@@ -41,11 +41,6 @@ import {
   createWorkspace,
 } from '../factories';
 
-type Intent = 'view' | 'manage' | 'delete';
-
-/** `undefined` stands for a caller that passes no options at all. */
-const INTENTS: ReadonlyArray<Intent | undefined> = [undefined, 'view', 'manage', 'delete'];
-
 type Actor =
   | 'an anonymous caller'
   | 'an outsider'
@@ -252,42 +247,6 @@ const SCENARIOS: readonly Scenario[] = [
 ];
 
 // ---------------------------------------------------------------------------
-// The known divergence
-// ---------------------------------------------------------------------------
-
-/**
- * The one place the two functions disagree, spelled out rather than filtered
- * out.
- *
- * `shouldLoadWorkspaceRole` in lib/auth.ts skips the workspace queries for a
- * project owner on `intent: 'view'`, on the reasoning that an owner passes every
- * check on their own. That is true of the three permission booleans, and it is
- * why this is harmless today, but it is not true of the two identity flags:
- * checkProjectAccess() reports `isWorkspaceMember: false, isWorkspaceAdmin:
- * false` for an owner who is in fact the workspace owner, where
- * computeProjectAccess() on the same rows reports true and true.
- *
- * Harmless today rests on one fact and not on the design: the only consumer of
- * `isWorkspaceMember` from checkProjectAccess() is
- * app/api/versions/[versionId]/approvals/route.ts:37, and its
- * `isOwner || isProjectMember || isWorkspaceMember` is already satisfied by
- * `isOwner` for exactly the actor that diverges. Nothing consumes
- * `isWorkspaceAdmin` from checkProjectAccess() at all; `canEdit` folds it in,
- * and `isOwner` covers that too. The next reader of either flag inherits the
- * bug, which is why this is asserted instead of hidden behind a comparison of
- * `hasAccess` alone.
- *
- * If this stops matching, the divergence was closed: delete this function and
- * the test at the bottom of the file rather than widening either of them.
- */
-function knownDivergence(actor: Actor, intent: Intent | undefined): Partial<ExpectedAccess> {
-  const resolvesWorkspaceRole = intent === 'manage' || intent === 'delete';
-  if (actor === 'a project owner who also owns the workspace' && !resolvesWorkspaceRole) {
-    return { isWorkspaceMember: false, isWorkspaceAdmin: false };
-  }
-  return {};
-}
-
 // ---------------------------------------------------------------------------
 // Fixtures
 // ---------------------------------------------------------------------------
@@ -428,7 +387,7 @@ for (const scenario of SCENARIOS) {
     });
 
     for (const actor of ACTORS) {
-      it(`resolves ${actor} exactly as computeProjectAccess does, at every intent`, async () => {
+      it(`resolves ${actor} exactly as computeProjectAccess does`, async () => {
         const userId = seeded.userIdFor(actor);
         const projectId = projectIdFor(actor, seeded);
         const expected = scenario.expected[actor];
@@ -437,20 +396,10 @@ for (const scenario of SCENARIOS) {
         // The pure half first: given the rows, this is the answer.
         expect(computeProjectAccess(enriched, userId), 'computeProjectAccess').toEqual(expected);
 
-        // And the querying half, which has to arrive at the same place from the
-        // same rows, whatever the caller says it intends to do.
-        for (const intent of INTENTS) {
-          const checked = await checkProjectAccess(
-            enriched,
-            userId,
-            intent === undefined ? undefined : { intent }
-          );
-
-          expect(checked, `checkProjectAccess with intent ${intent ?? '(default)'}`).toEqual({
-            ...expected,
-            ...knownDivergence(actor, intent),
-          });
-        }
+        // And the querying half, which has to arrive at the same place from the same rows.
+        // It used to take an `intent` that skipped the workspace queries for an owner at
+        // `view`, which is what made these two disagree; there is one code path now.
+        expect(await checkProjectAccess(enriched, userId), 'checkProjectAccess').toEqual(expected);
       });
     }
   });
@@ -460,8 +409,13 @@ for (const scenario of SCENARIOS) {
 // The divergence, on its own
 // ---------------------------------------------------------------------------
 
-describe('checkProjectAccess and computeProjectAccess disagree in one place', () => {
-  it('hides the workspace role from a project owner who also owns the workspace, on intent view', async () => {
+describe('checkProjectAccess and computeProjectAccess agree about the workspace role', () => {
+  // This was the one cell of the matrix that diverged, and it is the shape every real
+  // signup produces: app/api/projects/route.ts gives a new project the workspace owner's
+  // id. checkProjectAccess() used to skip both workspace queries for an owner at `view`
+  // intent, so the two identity flags read as if this user were a stranger to the
+  // workspace they own.
+  it('reports a project owner who also owns the workspace as a workspace admin', async () => {
     const owner = await createUser();
     const workspace = await createWorkspace({ ownerId: owner.id });
     const project = await createProject({
@@ -471,32 +425,11 @@ describe('checkProjectAccess and computeProjectAccess disagree in one place', ()
     });
 
     const enriched = await fetchEnriched(project.id, owner.id);
-
     const computed = computeProjectAccess(enriched, owner.id);
-    const viewed = await checkProjectAccess(enriched, owner.id, { intent: 'view' });
-    const managed = await checkProjectAccess(enriched, owner.id, { intent: 'manage' });
 
-    // What the rows say: this user owns the workspace the project lives in.
     expect(computed.isWorkspaceMember).toBe(true);
     expect(computed.isWorkspaceAdmin).toBe(true);
 
-    // What checkProjectAccess() says on the intent that pages and GET routes
-    // use. `shouldLoadWorkspaceRole` skipped the workspace queries, so the role
-    // was never resolved and the two flags read as if this user were a stranger
-    // to the workspace.
-    expect(viewed.isWorkspaceMember).toBe(false);
-    expect(viewed.isWorkspaceAdmin).toBe(false);
-
-    // Ask the same question with a mutating intent and the same user, on the
-    // same rows, is a workspace owner again.
-    expect(managed.isWorkspaceMember).toBe(true);
-    expect(managed.isWorkspaceAdmin).toBe(true);
-
-    // Why nobody has noticed: every permission the flags feed is already
-    // granted by isOwner, so the divergence stops at the two identity flags.
-    expect(viewed.hasAccess).toBe(true);
-    expect(viewed.canEdit).toBe(true);
-    expect(viewed.canDelete).toBe(true);
-    expect({ ...viewed, isWorkspaceMember: true, isWorkspaceAdmin: true }).toEqual(computed);
+    expect(await checkProjectAccess(enriched, owner.id)).toEqual(computed);
   });
 });

tests/api/lib-r2-upload-session.test.ts

@@ -180,19 +180,20 @@ describe('cancelR2UploadSession', () => {
     ).toBe('FINALIZED');
   });
 
-  // The `expiresAt: { gt: now }` clause means an expired session cannot be
-  // cancelled at all: the row stays INITIATED and consumedAt stays null. That
-  // is the current contract, and it is why the sweeper rather than the route
-  // has to be the thing that reclaims those reservations. See the report.
-  it('matches nothing once the session has expired, leaving it INITIATED', async () => {
+  // An `expiresAt: { gt: now }` clause used to make this match zero rows once a session
+  // lapsed, so it stayed INITIATED with a null consumedAt for good. The r2-init DELETE
+  // route releases the quota reservation only when the update reports a row, so every
+  // abandoned upload held its reserved bytes against the user's quota permanently.
+  // Cancelling something already expired is the case that most needs to work.
+  it('cancels a session that has already expired', async () => {
     const { session } = await newSession({ expiresAt: new Date(Date.now() - 60_000) });
 
     const result = await cancelR2UploadSession(session.id);
 
-    expect(result.count).toBe(0);
+    expect(result.count).toBe(1);
     const row = await db.videoUploadSession.findUniqueOrThrow({ where: { id: session.id } });
-    expect(row.status).toBe('INITIATED');
-    expect(row.consumedAt).toBeNull();
+    expect(row.status).toBe('CANCELLED');
+    expect(row.consumedAt).not.toBeNull();
   });
 
   it('does nothing for an id that matches no row', async () => {

tests/api/lib-video-assets.test.ts

@@ -18,7 +18,6 @@ import {
   extractVideoFileNameFromProxyUrl,
   extractVideoKeyFromProxyUrl,
   getVideoAssetAccessContext,
-  mediaUrlToR2Key,
   sanitizeAssetDisplayName,
   SAFE_BUNNY_VIDEO_ID,
 } from '@/lib/video-assets';
@@ -132,26 +131,25 @@ describe('proxy URL extraction', () => {
   });
 });
 
-describe('mediaUrlToR2Key', () => {
-  it('derives an image key and a voice key from canonical URLs', () => {
-    expect(mediaUrlToR2Key(IMAGE_URL)).toBe('images/eeeeeeee-eeee-4eee-8eee-eeeeeeeeeee1.png');
-    expect(mediaUrlToR2Key(AUDIO_URL)).toBe('voice/eeeeeeee-eeee-4eee-8eee-eeeeeeeeeee2.webm');
-  });
-
-  it('returns null for a URL that is not an image or audio proxy path', () => {
-    expect(mediaUrlToR2Key(VIDEO_URL)).toBeNull();
-    expect(mediaUrlToR2Key('https://www.youtube.com/watch?v=dQw4w9WgXcQ')).toBeNull();
-  });
-
-  // Documented, not endorsed. Unlike extractImageKeyFromProxyUrl this one
-  // matches on a substring with no shape check, so the key it produces is
-  // attacker-shaped whenever the URL is. The module has no callers today; if
-  // one appears it must use the extract* helpers instead. See the report.
-  it('accepts a substring match that the anchored extractor rejects', () => {
+// mediaUrlToR2Key used to live here. It matched the proxy prefix as a substring with no
+// shape check, so `https://evil.test/api/upload/image/../../videos/live.mp4` produced the
+// key `images/../../videos/live.mp4`. It had no callers, so it was deleted rather than
+// reimplemented; the anchored extract* helpers below are what a caller should use.
+describe('the anchored extractors refuse a substring match', () => {
+  it('rejects a hostile url that only contains the proxy prefix', () => {
     const hostile = 'https://evil.test/api/upload/image/../../videos/live.mp4';
 
     expect(extractImageKeyFromProxyUrl(hostile)).toBeNull();
-    expect(mediaUrlToR2Key(hostile)).toBe('images/../../videos/live.mp4');
+    expect(extractImageFileNameFromProxyUrl(hostile)).toBeNull();
+  });
+
+  it('still derives keys from canonical urls', () => {
+    expect(extractImageKeyFromProxyUrl(IMAGE_URL)).toBe(
+      'images/eeeeeeee-eeee-4eee-8eee-eeeeeeeeeee1.png'
+    );
+    expect(extractAudioKeyFromProxyUrl(AUDIO_URL)).toBe(
+      'voice/eeeeeeee-eeee-4eee-8eee-eeeeeeeeeee2.webm'
+    );
   });
 });
 

tests/api/lib-video-delete.test.ts

@@ -14,7 +14,7 @@
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 import { revalidatePath } from 'next/cache';
 import { db } from '@/lib/db';
-import { deleteProjectVideosWithCleanup } from '@/lib/video-delete';
+import { deleteProjectVideosWithCleanup, VideoStorageCleanupError } from '@/lib/video-delete';
 import {
   createComment,
   createProject,
@@ -372,12 +372,10 @@ describe('deleteProjectVideosWithCleanup and Bunny', () => {
 });
 
 describe('deleteProjectVideosWithCleanup when storage fails', () => {
-  // The rows go first and the objects go second, with no transaction spanning
-  // the two. A refused DELETE therefore leaves the object behind with nothing
-  // in the database still pointing at it: the caller cannot retry, because the
-  // video id it would retry with no longer resolves. The result object is the
-  // only trace, which is why the warnings are asserted here.
-  it('still removes the rows and surfaces the orphaned key as a warning', async () => {
+  // Storage runs first and the rows go second. Deleting the rows first left the object in
+  // the bucket with nothing pointing at it and no way to retry, because the video id no
+  // longer resolved. Keeping the rows makes the delete repeatable.
+  it('keeps the rows and reports the failure when a key cannot be deleted', async () => {
     const scenario = await seedProject();
     const target = await seedDeletableVideo({
       projectId: scenario.project.id,
@@ -387,22 +385,40 @@ describe('deleteProjectVideosWithCleanup when storage fails', () => {
     });
     r2.rejectKeys.add(TARGET_VIDEO_KEY);
 
-    const result = await deleteProjectVideosWithCleanup(scenario.project.id, [target.video.id]);
+    await expect(
+      deleteProjectVideosWithCleanup(scenario.project.id, [target.video.id])
+    ).rejects.toBeInstanceOf(VideoStorageCleanupError);
 
-    expect(result.deletedCount).toBe(1);
-    // The row is gone even though its object is not.
-    expect(await db.video.count()).toBe(0);
-    expect(result.cleanupInput.r2).toEqual({
-      attempted: 2,
-      failed: 1,
-      failedKeys: [TARGET_VIDEO_KEY],
-    });
-    expect(result.cleanupWarnings).toEqual({ r2: { attempted: 2, failed: 1 } });
+    // The row is still there, so the caller can try again.
+    expect(await db.video.count()).toBe(1);
     // The rest of the sweep still ran.
     expect(r2.deletedKeys).toEqual([TARGET_COMMENT_IMAGE_KEY]);
   });
 
-  it('reports a Bunny failure without failing the delete', async () => {
+  it('carries the failed keys on the error so the route can log them', async () => {
+    const scenario = await seedProject();
+    const target = await seedDeletableVideo({
+      projectId: scenario.project.id,
+      ownerId: scenario.owner.id,
+      videoUrl: TARGET_VIDEO_URL,
+      commentImageUrl: TARGET_COMMENT_IMAGE,
+    });
+    r2.rejectKeys.add(TARGET_VIDEO_KEY);
+
+    const error = await deleteProjectVideosWithCleanup(scenario.project.id, [
+      target.video.id,
+    ]).catch((err: unknown) => err as VideoStorageCleanupError);
+
+    expect(error.cleanupInput.r2).toEqual({
+      attempted: 2,
+      failed: 1,
+      failedKeys: [TARGET_VIDEO_KEY],
+    });
+  });
+
+  // A Bunny video that survives the delete is billed and invisible in the app, so it gets
+  // the same treatment as an orphaned R2 object.
+  it('keeps the rows when Bunny refuses the delete', async () => {
     vi.stubEnv('BUNNY_STREAM_API_KEY', 'test-bunny-key');
     vi.stubEnv('BUNNY_STREAM_LIBRARY_ID', '9999');
     vi.stubGlobal(
@@ -417,10 +433,13 @@ describe('deleteProjectVideosWithCleanup when storage fails', () => {
       providerVideoId: 'bunny-version-id-2',
     });
 
-    const result = await deleteProjectVideosWithCleanup(scenario.project.id, [video.id]);
+    const error = await deleteProjectVideosWithCleanup(scenario.project.id, [video.id]).catch(
+      (err: unknown) => err as VideoStorageCleanupError
+    );
 
-    expect(await db.video.count()).toBe(0);
-    expect(result.cleanupWarnings).toEqual({ bunny: { attempted: 1, failed: 1 } });
+    expect(error).toBeInstanceOf(VideoStorageCleanupError);
+    expect(error.cleanupInput.bunny).toMatchObject({ attempted: 1, failed: 1 });
+    expect(await db.video.count()).toBe(1);
   });
 
   it('reports no warnings when both providers succeed', async () => {

tests/api/projects.test.ts

@@ -151,11 +151,10 @@ describe('GET /api/projects', () => {
     expect(projects.projects.map((entry) => entry.id)).toEqual([scenario.project.id]);
   });
 
-  // Documents current behaviour, which looks like a bug. See the note in the
-  // report: the workspace-membership branch of the OR is dropped as soon as
-  // ?workspaceId is supplied, so filtering by workspace hides exactly the
-  // projects the unfiltered call returns.
-  it('stops listing workspace-member projects once ?workspaceId is supplied', async () => {
+  // The workspace-membership branch of the OR used to be dropped as soon as ?workspaceId
+  // was supplied, so filtering by their own workspace showed a member an empty list while
+  // the unfiltered call returned the same project.
+  it('still lists workspace-member projects when ?workspaceId is supplied', async () => {
     const scenario = await seedProject();
     const member = await createUser();
     await addWorkspaceMember({ workspaceId: scenario.workspace.id, userId: member.id });
@@ -166,7 +165,7 @@ describe('GET /api/projects', () => {
     );
 
     expect(unfiltered.projects.map((entry) => entry.id)).toEqual([scenario.project.id]);
-    expect(filtered.projects).toEqual([]);
+    expect(filtered.projects.map((entry) => entry.id)).toEqual([scenario.project.id]);
   });
 
   it('scopes ?workspaceId to that workspace for an owner of several', async () => {

tests/api/rate-limit.test.ts

@@ -165,16 +165,16 @@ describe('checkRateLimit', () => {
     expect(result.remaining).toBe(RATE_LIMIT_CONFIGS.api.maxRequests - 1);
   });
 
-  // Defence in depth against oversized values reaching the query. The call is
-  // allowed but nothing is recorded, so an attacker cannot use a huge key to
-  // bloat the table either.
-  it('allows and records nothing for an over-long key or action', async () => {
+  // An oversized value is hashed to fit its column rather than skipped, so it is written
+  // and counted like any other. A huge key cannot bloat the table either: what lands in
+  // the column is a fixed-width digest.
+  it('records an over-long key and an over-long action', async () => {
     const longKey = await checkRateLimit('x'.repeat(257), 'login', CONFIG);
     const longAction = await checkRateLimit('1.2.3.4', 'y'.repeat(65), CONFIG);
 
     expect(longKey.allowed).toBe(true);
     expect(longAction.allowed).toBe(true);
-    expect(await countRows('rate_limits')).toBe(0);
+    expect(await countRows('rate_limits')).toBe(2);
   });
 
   it('records a key of exactly 255 characters, the column width', async () => {
@@ -184,27 +184,31 @@ describe('checkRateLimit', () => {
     expect(await countRows('rate_limits')).toBe(1);
   });
 
-  // Documents an off-by-one, reported rather than fixed. The guard in
-  // lib/rate-limit.ts rejects `key.length > 256`, but rate_limits.key is
-  // VARCHAR(255), so a 256-character key clears the guard and then fails the
-  // INSERT with P2010. The catch treats any database error as "allow", so such a
-  // key is never counted and the limit silently stops applying to it.
-  //
-  // Not reachable from the product today: every call site builds a key from an
-  // IP, a user id or a 24-character hash. The failure mode is fail-open, so a
-  // future longer key would disable a limit rather than break a page.
-  it('fails open for a 256-character key instead of counting it', async () => {
-    // Kept to four attempts, one past the limit, because each one logs the
-    // swallowed Postgres error and the point is made without ten copies of it.
+  // This is the case that used to fail open twice over: the guard allowed a 256-character
+  // key through, the INSERT then failed with SQLSTATE 22001 against a VARCHAR(255) column,
+  // and the catch answered "allowed" for every attempt. The key is now hashed before it
+  // reaches the query, so the limit applies to it like any other.
+  it('counts a 256-character key and blocks it past the cap', async () => {
     const key = 'x'.repeat(256);
 
-    for (let attempt = 0; attempt < 4; attempt += 1) {
+    for (let attempt = 0; attempt < CONFIG.maxRequests; attempt += 1) {
       const result = await checkRateLimit(key, 'login', CONFIG);
       expect(result.allowed).toBe(true);
-      expect(result.remaining).toBe(CONFIG.maxRequests);
     }
 
-    expect(await countRows('rate_limits')).toBe(0);
+    const blocked = await checkRateLimit(key, 'login', CONFIG);
+    expect(blocked.allowed).toBe(false);
+    expect(blocked.remaining).toBe(0);
+
+    expect(await countRows('rate_limits')).toBe(1);
+    expect((await db.rateLimit.findFirstOrThrow()).count).toBe(CONFIG.maxRequests + 1);
+  });
+
+  it('keeps two different over-long keys in separate buckets', async () => {
+    await checkRateLimit(`a${'x'.repeat(300)}`, 'login', CONFIG);
+    await checkRateLimit(`b${'x'.repeat(300)}`, 'login', CONFIG);
+
+    expect(await countRows('rate_limits')).toBe(2);
   });
 
   it('counts concurrent calls exactly once each', async () => {

tests/api/search-scope.test.ts

@@ -297,14 +297,14 @@ describe('GET /api/search reaches everything the caller is entitled to', () => {
 // ---------------------------------------------------------------------------
 // Billing
 // ---------------------------------------------------------------------------
-// Pins current behaviour, and the behaviour is inconsistent. See the report
-// accompanying this suite: GET /api/projects filters every row through
-// `workspace.owner: buildBillingAccessWhereInput()`, and `checkProjectAccess()`
-// makes `hasAccess` false the moment the workspace owner's billing lapses, so
-// the project itself answers 403. /api/search applies no billing filter at all
-// and keeps returning the names. Changing that means changing this test.
+// Search carries the same billing condition every other read path does. It used to carry
+// none: GET /api/projects filters every row through
+// `workspace.owner: buildBillingAccessWhereInput()`, and `checkProjectAccess()` makes
+// `hasAccess` false the moment the workspace owner's billing lapses, so the project
+// itself answers 403, while search went on returning names, descriptions and video
+// titles for the same tenant.
 describe('GET /api/search and lapsed billing', () => {
-  it('keeps returning a project whose workspace owner has lost billing access', async () => {
+  it('hides a project whose workspace owner has lost billing access', async () => {
     const term = uniqueTerm();
     const expiredOwner = await createExpiredUser();
     await seedProject({ ownerUser: expiredOwner, projectName: `${term} lapsed project` });
@@ -312,11 +312,22 @@ describe('GET /api/search and lapsed billing', () => {
 
     const results = await searchFor(term);
 
-    expect(results.projects.map((entry) => entry.name)).toEqual([`${term} lapsed project`]);
+    expect(results.projects).toEqual([]);
   });
 
-  // The same caller, the same row, through the list endpoint instead. This is
-  // the contrast that makes the case above a finding rather than a preference.
+  // The positive control: the same shape with billing intact still comes back, so the
+  // assertion above is about billing and not about the fixture failing to seed.
+  it('still returns a project whose workspace owner is paying', async () => {
+    const term = uniqueTerm();
+    const scenario = await seedProject({ projectName: `${term} live project` });
+    signedInAs(scenario.owner);
+
+    const results = await searchFor(term);
+
+    expect(results.projects.map((entry) => entry.name)).toEqual([`${term} live project`]);
+  });
+
+  // The same caller, the same row, through the list endpoint instead: the two agree now.
   it('is hidden from GET /api/projects for the same caller and the same row', async () => {
     const expiredOwner = await createExpiredUser();
     await seedProject({ ownerUser: expiredOwner, projectName: 'Lapsed project' });
@@ -329,7 +340,7 @@ describe('GET /api/search and lapsed billing', () => {
     expect(projects).toEqual([]);
   });
 
-  it('keeps returning a video title from a lapsed workspace to a collaborator', async () => {
+  it('hides a video title from a lapsed workspace, even from a collaborator', async () => {
     const term = uniqueTerm();
     const expiredOwner = await createExpiredUser();
     const { project } = await seedProject({ ownerUser: expiredOwner });
@@ -340,6 +351,20 @@ describe('GET /api/search and lapsed billing', () => {
 
     const results = await searchFor(term);
 
-    expect(results.videos.map((entry) => entry.title)).toEqual([`${term} lapsed cut`]);
+    expect(results.videos).toEqual([]);
+  });
+
+  it('still returns a video title to a collaborator while the owner is paying', async () => {
+    const term = uniqueTerm();
+    const { project } = await seedProject();
+
+    await createVideo({ projectId: project.id, title: `${term} live cut` });
+    const collaborator = await createUser();
+    await addProjectMember({ projectId: project.id, userId: collaborator.id });
+    signedInAs(collaborator);
+
+    const results = await searchFor(term);
+
+    expect(results.videos.map((entry) => entry.title)).toEqual([`${term} live cut`]);
   });
 });

tests/component/comment-rich-text.test.tsx

@@ -243,19 +243,19 @@ describe('CommentRichText asset mentions', () => {
     expect(screen.getByRole('button', { name: '@https://evil.test/x' })).toBeInTheDocument();
   });
 
-  // KNOWN BUG, pinned rather than fixed. `renderUrls` keys its fragments by the
-  // index within its own slice, and CommentRichText calls it once per gap
-  // between mentions, so the same key ("txt-0") is emitted for several
-  // siblings. React logs "Encountered two children with the same key" and warns
-  // that children may be duplicated or omitted. The output happens to be
-  // correct today; the text assertion locks that in, and the warning assertion
-  // is the thing to delete once the keys are made unique.
-  it('produces duplicate React keys when text surrounds a mention', () => {
+  // `renderUrls` used to key its fragments by the index within its own slice, and
+  // CommentRichText calls it once per gap between mentions, so the same key ("txt-0") was
+  // emitted for several siblings and React warned that children may be duplicated or
+  // omitted. The keys carry the slice offset now.
+  it('emits no duplicate React keys when text surrounds a mention', () => {
     const { container } = render(
       <CommentRichText text="Before @[One](asset:aaa111) middle @[Two](asset:bbb222) after" />
     );
 
     expect(container).toHaveTextContent('Before @One middle @Two after');
-    expect(consoleError).toHaveBeenCalledWith(expect.stringContaining('same key'), 'txt-0');
+    expect(consoleError).not.toHaveBeenCalledWith(
+      expect.stringContaining('same key'),
+      expect.anything()
+    );
   });
 });

tests/component/hooks/use-comment-actions.test.ts

@@ -733,15 +733,12 @@ describe('useCommentActions editing', () => {
     expect(findComment(harness, 'c1')?.content).toBe('Existing note');
   });
 
-  // KNOWN BUG, pinned rather than fixed. `editTagId` is typed `string | null`
-  // and initialised to `null`, so the `editTagId !== undefined` guard in the
-  // hook can never be false: every edit PATCH carries a `tagId`, and every
-  // successful edit overwrites the comment's tag with whatever `editTagId`
-  // happens to hold. The comment editor in comments-pane.tsx seeds it from the
-  // comment, but the REPLY editor (comments-pane.tsx, "Edit" on a reply) sets
-  // only editingCommentId and editText, so editing a reply's text silently
-  // sends tagId: null.
-  it('always sends a tagId, and clears the tag, even when the caller never set one', async () => {
+  // `editTagId` was initialised to `null`, so the `editTagId !== undefined` guard could
+  // never be false: every edit PATCH carried a `tagId` and every success overwrote the
+  // comment's tag. The comment editor seeds the value from the comment, but the reply
+  // editor sets only editingCommentId and editText, so editing a reply's text silently
+  // cleared its tag or applied a stale one. `undefined` now means "not managed here".
+  it('sends no tagId when the caller never set one, and leaves the tag alone', async () => {
     const harness = renderActions();
 
     act(() => harness.result.current.actions.setEditText('Reworded note'));
@@ -749,6 +746,23 @@ describe('useCommentActions editing', () => {
       await harness.result.current.actions.handleEditComment('c1');
     });
 
+    expect(bodyOf(callsTo('/api/comments/c1', 'PATCH')[0])).toEqual({
+      content: 'Reworded note',
+    });
+    expect(findComment(harness, 'c1')?.tag).toEqual(TAGS[0]);
+  });
+
+  it('sends tagId: null when the editor explicitly clears the tag', async () => {
+    const harness = renderActions();
+
+    act(() => {
+      harness.result.current.actions.setEditText('Reworded note');
+      harness.result.current.actions.setEditTagId(null);
+    });
+    await act(async () => {
+      await harness.result.current.actions.handleEditComment('c1');
+    });
+
     expect(bodyOf(callsTo('/api/comments/c1', 'PATCH')[0])).toEqual({
       content: 'Reworded note',
       tagId: null,

tests/component/hooks/use-download-actions.test.ts

@@ -619,18 +619,19 @@ describe('useDownloadActions repeated clicks', () => {
     });
   });
 
-  // KNOWN FRAGILITY, pinned rather than fixed. The in-flight guard reads
-  // `isDownloadingVideo` out of the closure the callback was created in, so two
-  // calls made from the SAME render (a double click landing before React
-  // commits the state update) both get through and the file is fetched twice.
-  it('lets two calls from the same render both through', async () => {
+  // The in-flight guard used to read `isDownloadingVideo` out of the closure the callback
+  // was created in, so two calls made from the SAME render (a double click landing before
+  // React commits the state update) both got through and the file was fetched twice. It
+  // reads a ref now.
+  it('refuses a second call from the same render', async () => {
     const startDownload = renderDownload().result.current.startDownload;
 
     await act(async () => {
       await Promise.all([startDownload(), startDownload()]);
     });
 
-    expect(urlsFetched().filter((url) => url.includes('prepare=1'))).toHaveLength(2);
+    expect(urlsFetched().filter((url) => url.includes('prepare=1'))).toHaveLength(1);
+    expect(clicked).toHaveLength(1);
   });
 
   it('is ready to download again after a failure', async () => {

tests/component/hooks/use-version-actions.test.ts

@@ -522,18 +522,18 @@ describe('useVersionActions uploading a file to Bunny', () => {
     expect(callsTo(BUNNY_INIT_URL, 'DELETE')).toHaveLength(0);
   });
 
-  // BUG, pinned rather than fixed. bunny-init has already created a video on
-  // Bunny by the time tus runs, but `pendingCleanup` is only assigned after
-  // uploadNewVersionFile returns. A tus failure therefore leaks that video:
-  // nothing ever calls the DELETE branch below it in the catch.
-  it('leaks the Bunny video when the tus upload itself fails', async () => {
+  // bunny-init has already created a video on Bunny by the time tus runs. `pendingCleanup`
+  // used to be assigned only after uploadNewVersionFile returned, so a tus failure threw
+  // past the assignment and left that video behind: billed, and invisible in the app. It
+  // is registered as soon as bunny-init answers now.
+  it('deletes the Bunny video when the tus upload itself fails', async () => {
     tusFailure = 'connection reset';
     const harness = renderVersionActions({ directUploadsEnabled: true });
 
     await createFromFile(harness);
 
     expect(toastError).toHaveBeenCalledWith('Upload failed: connection reset');
-    expect(callsTo(BUNNY_INIT_URL, 'DELETE')).toHaveLength(0);
+    expect(callsTo(BUNNY_INIT_URL, 'DELETE')).toHaveLength(1);
   });
 });
 

tests/component/hooks/use-video-assets.test.ts

@@ -580,7 +580,7 @@ describe('useVideoAssets deleting', () => {
     expect(callsTo(`/api/videos/${VIDEO_ID}/assets/a1`, 'DELETE')).toHaveLength(1);
     expect(deleted).toBe(true);
     expect(assetIds(harness)).toEqual(['a2']);
-    expect(harness.result.current.activeDeleteAssetId).toBeNull();
+    expect(harness.result.current.deletingAssetIds).toEqual([]);
   });
 
   it('marks which row is being deleted while the request runs', async () => {
@@ -592,13 +592,13 @@ describe('useVideoAssets deleting', () => {
     act(() => {
       removal = harness.result.current.deleteAsset('a1');
     });
-    expect(harness.result.current.activeDeleteAssetId).toBe('a1');
+    expect(harness.result.current.deletingAssetIds).toEqual(['a1']);
 
     await act(async () => {
       pending.resolve(jsonResponse(true, {}));
       await removal;
     });
-    expect(harness.result.current.activeDeleteAssetId).toBeNull();
+    expect(harness.result.current.deletingAssetIds).toEqual([]);
   });
 
   it('keeps the row when the server refuses the delete', async () => {
@@ -615,7 +615,7 @@ describe('useVideoAssets deleting', () => {
     expect(deleted).toBe(false);
     expect(assetIds(harness)).toEqual(['a1']);
     expect(toastError).toHaveBeenCalledWith('Only the uploader can delete');
-    expect(harness.result.current.activeDeleteAssetId).toBeNull();
+    expect(harness.result.current.deletingAssetIds).toEqual([]);
   });
 
   it('keeps the row when the delete throws', async () => {
@@ -644,7 +644,35 @@ describe('useVideoAssets deleting', () => {
     });
 
     expect(assetIds(harness)).toEqual([]);
-    expect(harness.result.current.activeDeleteAssetId).toBeNull();
+    expect(harness.result.current.deletingAssetIds).toEqual([]);
+  });
+
+  // A single slot meant the second delete cleared the first one's spinner, so the first
+  // row stopped indicating progress while its request was still in flight.
+  it('marks both rows while two deletes overlap', async () => {
+    listed = listResponse({ assets: [makeAsset(), makeAsset({ id: 'a2' })] });
+    const harness = await renderAssets();
+    const first = deferred<unknown>();
+    const second = deferred<unknown>();
+    fetchMock.mockReturnValueOnce(first.promise).mockReturnValueOnce(second.promise);
+
+    let removals: Promise<boolean[]> | undefined;
+    act(() => {
+      removals = Promise.all([
+        harness.result.current.deleteAsset('a1'),
+        harness.result.current.deleteAsset('a2'),
+      ]);
+    });
+
+    expect(harness.result.current.deletingAssetIds).toEqual(['a1', 'a2']);
+
+    await act(async () => {
+      first.resolve(jsonResponse(true, {}));
+      second.resolve(jsonResponse(true, {}));
+      await removals;
+    });
+
+    expect(harness.result.current.deletingAssetIds).toEqual([]);
   });
 });
 

tests/component/hooks/use-video-page-data.test.ts

@@ -477,14 +477,14 @@ describe('useVideoPageData loading tags', () => {
     expect(harness.result.current.selectedTagId).toBe('tag-colour');
   });
 
-  // KNOWN INEFFICIENCY, pinned rather than fixed. selectedTagId is in the
-  // effect's dependency list purely so the auto-select can read it, so the
-  // moment the first tag is selected the whole effect re-runs and the tag list
-  // is fetched a second time on every page load.
-  it('reads the tag list twice because selecting a tag re-runs the effect', async () => {
-    await renderPage();
+  // selectedTagId used to be in the effect's dependency list purely so the auto-select
+  // could read it, so the moment the first tag was selected the whole effect re-ran and
+  // the tag list was fetched a second time on every page load. It is read from a ref now.
+  it('reads the tag list once even though the auto-select sets a tag', async () => {
+    const harness = await renderPage();
 
-    expect(callsMatching((url) => url === TAGS_URL)).toHaveLength(2);
+    expect(harness.result.current.selectedTagId).toBe('tag-audio');
+    expect(callsMatching((url) => url === TAGS_URL)).toHaveLength(1);
   });
 
   it('selects nothing when the project has no tags', async () => {

tests/component/share-link-unlock.test.tsx

@@ -21,13 +21,13 @@ vi.mock('next/link', () => ({
 let fetchMock: ReturnType<typeof vi.fn>;
 
 /**
- * ACCESSIBILITY FINDING: the password field has no <label>, no aria-label and
- * no aria-labelledby, only a placeholder. A password input has no ARIA role
- * either, so there is no `getByRole` route to it at all. Reported, not papered
- * over: this helper documents that the placeholder is the only handle we have.
+ * A password input has no ARIA role, so `getByRole` cannot reach it whatever the
+ * markup does. `getByLabelText` can, and it only works because the field now has a
+ * visually hidden <label> associated by id: it used to have no label, no aria-label and
+ * no aria-labelledby, which left the placeholder as the only handle anything had.
  */
 function passwordField() {
-  return screen.getByPlaceholderText('Password');
+  return screen.getByLabelText('Password');
 }
 
 beforeEach(() => {
@@ -177,14 +177,13 @@ describe('ShareLinkUnlock', () => {
     render(<ShareLinkUnlock videoId="vid1" />);
 
     await userEvent.type(passwordField(), 'hunter2');
-    // Capture the node first: while submitting, the label is swapped for a
-    // spinner, which leaves the button with no accessible name to query by.
-    // ACCESSIBILITY FINDING, reported rather than worked around.
     const submit = screen.getByRole('button', { name: 'Continue' });
     await userEvent.click(submit);
 
     expect(submit).toBeDisabled();
-    expect(submit).toHaveAccessibleName('');
+    // The spinner that replaces the label carries a visually hidden name, so the button
+    // stays findable and announceable while it submits.
+    expect(submit).toHaveAccessibleName('Unlocking');
 
     release({ ok: true, json: () => Promise.resolve({}) });
     await waitFor(() => expect(replace).toHaveBeenCalledTimes(1));

tests/setup/unit.ts

@@ -0,0 +1,14 @@
+// Per-file setup for the `unit` Vitest project.
+//
+// One job: put the environment back after every test. The unit project had no setup file
+// at all, so each env-stubbing test had to restore its own state, and a forgotten
+// `afterEach` leaves the next test reading a value it never set. That is the failure mode
+// where a test passes for the wrong reason, which is worse than one that fails.
+//
+// Restoring centrally does not stop a test from calling `vi.unstubAllEnvs()` itself; it
+// only makes forgetting harmless.
+import { afterEach, vi } from 'vitest';
+
+afterEach(() => {
+  vi.unstubAllEnvs();
+});

tests/unit/lib/bunny-upload-token.test.ts

@@ -304,13 +304,15 @@ describe('verifyBunnyUploadToken', () => {
     expect(verifyBunnyUploadToken(token, SUBJECT)).toBe(true);
   });
 
-  it('returns false rather than throwing when the server has no secret configured', () => {
+  // A missing signing secret is a configuration fault, not a forgery. Answering "invalid
+  // token" for it turned a self-hosted misconfiguration into a silent, total upload
+  // outage that reads like a client bug.
+  it('throws rather than reporting a forgery when the server has no secret configured', () => {
     const token = createBunnyUploadToken(SUBJECT);
 
     vi.stubEnv('BUNNY_UPLOAD_TOKEN_SECRET', undefined);
     vi.stubEnv('NEXTAUTH_SECRET', undefined);
 
-    // A misconfigured server is indistinguishable from a forged token here.
-    expect(verifyBunnyUploadToken(token, SUBJECT)).toBe(false);
+    expect(() => verifyBunnyUploadToken(token, SUBJECT)).toThrow();
   });
 });

tests/unit/lib/client/upload-chunking.test.ts

@@ -4,6 +4,7 @@ import {
   getPartByteRange,
   getRetryDelayMs,
   getUploadProgressPercent,
+  isRetryableUploadError,
   PART_RETRY_DELAYS_MS,
 } from '@/lib/client/upload-chunking';
 
@@ -184,4 +185,48 @@ describe('getMultipartProgressPercent', () => {
   it('counts progress against the whole file, not the part', () => {
     expect(getMultipartProgressPercent([100, 0, 0], 300)).toBe(33);
   });
+
+  // Dividing by a total of zero produced NaN, which reached the UI as
+  // "Uploading... NaN%". Not reachable from the product today (r2-init rejects
+  // sizeBytes <= 0 and the multipart path only engages above 90 MiB), so the guard is
+  // here to keep an arithmetic accident from becoming a visible one.
+  it.each([
+    ['zero', 0],
+    ['a negative total', -1],
+  ])('reports 0 rather than NaN for %s', (_label, totalBytes) => {
+    expect(getMultipartProgressPercent([0, 0], totalBytes)).toBe(0);
+    expect(getMultipartProgressPercent([50, 50], totalBytes)).toBe(0);
+    expect(getUploadProgressPercent(50, totalBytes)).toBe(0);
+  });
+});
+
+describe('isRetryableUploadError', () => {
+  // The retry loop used to repeat every rejection. Cancelling an upload therefore did
+  // not cancel it: the part sat through the full 2s, 5s and 10s backoff and fired three
+  // more PUTs before the error surfaced.
+  it('refuses to retry the user cancelling the upload', () => {
+    expect(isRetryableUploadError(new Error('Upload aborted'))).toBe(false);
+  });
+
+  // An expired presigned part URL answers 403 every time, so retrying turned one dead
+  // part into four requests and 17 seconds of apparent hanging.
+  it.each([400, 401, 403, 404, 411, 413])('refuses to retry status %s', (status) => {
+    expect(isRetryableUploadError(new Error(`Upload failed with status ${status}`))).toBe(false);
+    expect(isRetryableUploadError(new Error(`Chunk upload failed with status ${status}`))).toBe(
+      false
+    );
+  });
+
+  it.each([408, 429, 500, 502, 503, 504])('retries status %s', (status) => {
+    expect(isRetryableUploadError(new Error(`Upload failed with status ${status}`))).toBe(true);
+  });
+
+  it('retries an error that carries no status at all', () => {
+    expect(isRetryableUploadError(new Error('Network error during upload.'))).toBe(true);
+    expect(isRetryableUploadError(new Error('Upload response missing ETag header.'))).toBe(true);
+  });
+
+  it('retries a non-Error rejection rather than swallowing it', () => {
+    expect(isRetryableUploadError('something went wrong')).toBe(true);
+  });
 });

tests/unit/lib/comment-export.test.ts

@@ -327,9 +327,11 @@ describe('buildCommentsCsv', () => {
     expect(line[17]).toBe('"false"');
   });
 
-  it('neutralises a negative timestamp because it starts with a minus sign', () => {
-    // Documents an interaction between the formula guard and numeric cells.
-    expect(csvRows([row({ timestamp: -1 })])[1][8]).toBe(`"'-1.000"`);
+  // The formula guard prefixes an apostrophe to anything starting with =, +, - or @.
+  // Applying it to a plain negative number stopped the spreadsheet reading the cell as a
+  // number at all, which is what a negative timestamp is.
+  it('leaves a negative number readable as a number', () => {
+    expect(csvRows([row({ timestamp: -1 })])[1][8]).toBe(`"-1.000"`);
   });
 
   it('preserves the flattened thread order in the output', () => {

tests/unit/lib/content-security-policy.test.ts

@@ -133,13 +133,31 @@ describe('buildContentSecurityPolicy', () => {
     expect(mediaSrc).not.toContain('https://public.b-cdn.net');
   });
 
-  it('always allows the local MinIO defaults in connect-src', () => {
+  it('allows the local MinIO defaults in connect-src outside production', () => {
+    vi.stubEnv('NODE_ENV', 'development');
     const connectSrc = directives()['connect-src'];
 
     expect(connectSrc).toContain('http://localhost:9000');
     expect(connectSrc).toContain('http://127.0.0.1:9000');
   });
 
+  // They are a local development convenience, and allowing plaintext loopback object
+  // storage in every deployment weakened the policy for a case production never has.
+  it('drops the local MinIO defaults from connect-src in production', () => {
+    vi.stubEnv('NODE_ENV', 'production');
+    const connectSrc = directives()['connect-src'];
+
+    expect(connectSrc).not.toContain('http://localhost:9000');
+    expect(connectSrc).not.toContain('http://127.0.0.1:9000');
+  });
+
+  it('still allows a loopback R2_ENDPOINT in production when one is configured', () => {
+    vi.stubEnv('NODE_ENV', 'production');
+    vi.stubEnv('R2_ENDPOINT', 'http://127.0.0.1:9000');
+
+    expect(directives()['connect-src']).toContain('http://127.0.0.1:9000');
+  });
+
   it('reduces a custom R2 endpoint to its origin', () => {
     vi.stubEnv('R2_ENDPOINT', 'https://minio.internal:9443/openframe-bucket');
 

tests/unit/lib/email-brand.test.ts

@@ -8,6 +8,7 @@ import {
   emailRow,
   escapeAttr,
   escapeHtml,
+  rawEmailHtml,
 } from '@/lib/email-brand';
 
 describe('escapeHtml', () => {
@@ -30,10 +31,10 @@ describe('escapeHtml', () => {
     expect(escapeHtml('<')).toBe('<');
   });
 
-  it('leaves a single quote unescaped', () => {
-    // Documents the current behaviour: values interpolated into single-quoted
-    // attributes are not protected by this helper.
-    expect(escapeHtml("it's")).toBe("it's");
+  // Single-quoted attributes exist in the templates, so leaving the quote alone left a
+  // value able to close one.
+  it('escapes the single quote', () => {
+    expect(escapeHtml("it's")).toBe('it's');
   });
 
   it('leaves plain text untouched', () => {
@@ -152,12 +153,47 @@ describe('email fragment builders', () => {
     expect(highlighted).not.toContain(EMAIL_COLORS.textSecondary);
   });
 
-  it('emailButton escapes the href but not the label', () => {
+  it('emailButton escapes both the href and the label', () => {
     const html = emailButton('<b>Open</b>', 'https://x.com" onclick="alert(1)');
 
     expect(html).toContain('&quot; onclick=&quot;alert(1)');
-    // Documents that the label is inserted raw, so callers must escape it.
-    expect(html).toContain('<b>Open</b>');
+    expect(html).toContain('&lt;b&gt;Open&lt;/b&gt;');
+    expect(html).not.toContain('<b>Open</b>');
+  });
+
+  // The escaping lives in the helpers rather than in every call site, so a project name
+  // or a display name is safe whether or not the next caller remembers to escape it.
+  it.each([
+    ['emailHeading title', () => emailHeading('*', '<script>alert(1)</script>')],
+    ['emailRow label', () => emailRow('<script>alert(1)</script>', 'value')],
+    ['emailRow value', () => emailRow('label', '<script>alert(1)</script>')],
+    ['emailHighlight text', () => emailHighlight('<script>alert(1)</script>')],
+    ['emailButton label', () => emailButton('<script>alert(1)</script>', 'https://x.test')],
+  ])('%s is escaped', (_label, build) => {
+    const html = build();
+
+    expect(html).not.toContain('<script>');
+    expect(html).toContain('&lt;script&gt;alert(1)&lt;/script&gt;');
+  });
+
+  it('rawEmailHtml opts a value out of escaping', () => {
+    const html = emailRow('From', rawEmailHtml('<span>Alice</span>'));
+
+    expect(html).toContain('<span>Alice</span>');
+  });
+
+  it('escapes the footer text', () => {
+    const html = brandedEmailTemplate('<tr><td>body</td></tr>', {
+      footerText: '<script>alert(1)</script>',
+    });
+
+    expect(html).not.toContain('<script>');
+  });
+
+  it('inserts the body markup verbatim', () => {
+    const html = brandedEmailTemplate('<tr><td>body</td></tr>');
+
+    expect(html).toContain('<tr><td>body</td></tr>');
   });
 
   it('emailHighlight wraps the text in a bordered block', () => {

tests/unit/lib/logger.test.ts

@@ -179,17 +179,44 @@ describe('logError', () => {
       });
     });
 
-    // Documents a real limitation rather than an intended behaviour: the branch
-    // keys on the constructor name, so an error that only claims to be a Prisma
-    // error through `err.name` (a re-thrown, deserialised or minified one) falls
-    // through to the generic branch and its message is logged verbatim.
-    it('does not redact an error that is Prisma only by its `name` property', () => {
+    // An Error instance always has a constructor, so keying on `constructor.name` alone
+    // would stop redacting the moment an error identifies itself as Prisma only through
+    // `name`: one that was re-thrown or deserialised and lost its prototype, or a
+    // production build whose minifier renamed the class.
+    it('redacts an error that is Prisma only by its `name` property', () => {
       const err = new Error(LEAKY_PRISMA_MESSAGE);
       err.name = 'PrismaClientKnownRequestError';
+      (err as unknown as Record<string, unknown>).code = 'P2002';
 
       logError('user lookup failed', err);
 
-      expect(loggedPayload()).toEqual({ type: 'Error', message: LEAKY_PRISMA_MESSAGE });
+      expect(loggedPayload()).toEqual({
+        type: 'PrismaError',
+        code: 'P2002',
+        message: 'Database error [P2002]',
+      });
+    });
+
+    it('redacts a name-only Prisma error that carries no code', () => {
+      const err = new Error(LEAKY_PRISMA_MESSAGE);
+      err.name = 'PrismaClientValidationError';
+
+      logError('user lookup failed', err);
+
+      expect(loggedPayload()).toEqual({
+        type: 'PrismaError',
+        code: 'UNKNOWN',
+        message: 'Database error [UNKNOWN]',
+      });
+    });
+
+    it('leaves a non-Prisma error alone', () => {
+      const err = new Error('plain failure');
+      err.name = 'ValidationError';
+
+      logError('lookup failed', err);
+
+      expect(loggedPayload()).toEqual({ type: 'Error', message: 'plain failure' });
     });
   });
 

tests/unit/lib/project-download.test.ts

@@ -346,14 +346,21 @@ describe('validateProjectDownloadManifest', () => {
     );
   });
 
-  // KNOWN BUG in lib/project-download.ts, asserted as-is rather than fixed here:
-  // `BigInt(manifest.totalBytes)` is unguarded, so a non-numeric total throws a
-  // SyntaxError out of a function whose contract is to return a message string.
-  // The route wraps this in a try/catch and turns it into a 500 rather than the
-  // 400 that every other rejection produces.
-  it('throws instead of returning a message when totalBytes is not numeric', () => {
-    expect(() => validateProjectDownloadManifest(manifestOf({ totalBytes: 'lots' }))).toThrow(
-      SyntaxError
+  // The contract is to return a message, never to throw: a SyntaxError out of here
+  // reaches the route as a 500 rather than the 400 every other rejection produces.
+  it('returns a message rather than throwing when totalBytes is not numeric', () => {
+    expect(validateProjectDownloadManifest(manifestOf({ totalBytes: 'lots' }))).toBe(
+      'Could not determine the size of this download'
+    );
+  });
+
+  it.each([
+    ['a negative total', '-1'],
+    ['a decimal total', '1.5'],
+    ['a hex total', '0x10'],
+  ])('rejects %s without throwing', (_label, totalBytes) => {
+    expect(validateProjectDownloadManifest(manifestOf({ totalBytes }))).toBe(
+      'Could not determine the size of this download'
     );
   });
 });
@@ -707,26 +714,38 @@ describe('buildProjectDownloadManifest provider routing', () => {
     ]);
   });
 
-  // KNOWN BUG in lib/project-download.ts, asserted as-is rather than fixed here:
-  // the r2 branch returns `originalUrl` verbatim after a `startsWith` check on
-  // the proxy prefix, while the sibling branch below it validates the same shape
-  // against a strict UUID pattern. A stored url with dot segments is handed back
-  // untouched, and the extension the file name is built from is taken from the
-  // raw url too, so the resulting `fileName` escapes the archive root.
-  it('passes an r2 traversal path through and lets it leak into the file name', () => {
+  // The r2 branch validates against the strict proxy-path pattern rather than a
+  // `startsWith` on the prefix, so dot segments never reach the manifest as a url
+  // and never leak a path separator into the file name either.
+  it.each([
+    ['dot segments after a valid-looking name', '/api/upload/video/clip.mp4/../../../etc/passwd'],
+    ['a non-uuid basename', '/api/upload/video/clip.mp4'],
+    ['an encoded traversal', '/api/upload/video/..%2F..%2Fetc%2Fpasswd'],
+    ['a nested path', '/api/upload/video/nested/dir/file.mp4'],
+  ])('drops an r2 version whose stored url has %s', (_label, originalUrl) => {
+    const manifest = buildProjectDownloadManifest('Project', [
+      video({ versions: [version({ providerId: 'r2', originalUrl })] }),
+    ]);
+
+    expect(manifest.files).toEqual([]);
+  });
+
+  it('keeps a well-formed r2 proxy path', () => {
     const manifest = buildProjectDownloadManifest('Project', [
       video({
         versions: [
           version({
             providerId: 'r2',
-            originalUrl: '/api/upload/video/clip.mp4/../../../../etc/passwd',
+            originalUrl: '/api/upload/video/bbbbbbbb-1111-2222-3333-444444444444.mp4',
           }),
         ],
       }),
     ]);
 
-    expect(manifest.files[0]?.url).toBe('/api/upload/video/clip.mp4/../../../../etc/passwd');
-    expect(manifest.files[0]?.fileName).toBe('01-Intro-v1./etc/passwd');
+    expect(manifest.files[0]?.url).toBe(
+      '/api/upload/video/bbbbbbbb-1111-2222-3333-444444444444.mp4'
+    );
+    expect(manifest.files[0]?.fileName).toBe('01-Intro-v1.mp4');
   });
 });
 
@@ -872,10 +891,10 @@ describe('buildProjectDownloadManifest file naming', () => {
     ).toEqual(['01-Intro-v1.mov']);
   });
 
-  it('keeps the case of the extension', () => {
+  it('lowercases the extension', () => {
     expect(
       namesOf([video({ versions: [version({ originalUrl: 'https://cdn.example/master.MP4' })] })])
-    ).toEqual(['01-Intro-v1.MP4']);
+    ).toEqual(['01-Intro-v1.mp4']);
   });
 
   it('strips the query string before reading the extension', () => {
@@ -888,13 +907,11 @@ describe('buildProjectDownloadManifest file naming', () => {
     ).toEqual(['01-Intro-v1.webm']);
   });
 
-  // KNOWN BUG in lib/project-download.ts, asserted as-is rather than fixed here:
-  // `extensionFromUrl` slices from the last dot anywhere in the url, including a
-  // dot in the host, and the result is appended after the sanitiser has already
-  // run. An allowlisted direct url with no file extension therefore produces a
-  // file name containing a path separator, which a zip writer turns into a
-  // directory rather than a file.
-  it('lets a dot in the host leak a path separator into the file name', () => {
+  // The extension is appended after the sanitiser has run, so it is derived from the
+  // last path segment only and has to be a short alphanumeric run. A dot in the host
+  // of an extensionless url must not contribute a path separator: a zip writer would
+  // turn that into a directory rather than a file.
+  it('falls back rather than letting a dot in the host leak a path separator', () => {
     vi.stubEnv('NEXT_PUBLIC_DIRECT_DOWNLOAD_ALLOWED_HOSTS', 'example.com');
 
     expect(
@@ -905,7 +922,20 @@ describe('buildProjectDownloadManifest file naming', () => {
           ],
         }),
       ])
-    ).toEqual(['01-Intro-v1.com/download']);
+    ).toEqual(['01-Intro-v1.mp4']);
+  });
+
+  it.each([
+    ['a path segment after the extension', 'https://example.com/a.mp4/../../etc/passwd'],
+    ['an extension longer than ten characters', 'https://example.com/clip.verylongextension'],
+    ['a non-alphanumeric extension', 'https://example.com/clip.mp4%2f..'],
+    ['a dotfile with no extension', 'https://example.com/.hidden'],
+  ])('falls back to .mp4 for %s', (_label, originalUrl) => {
+    vi.stubEnv('NEXT_PUBLIC_DIRECT_DOWNLOAD_ALLOWED_HOSTS', 'example.com');
+
+    expect(
+      namesOf([video({ versions: [version({ providerId: 'direct', originalUrl })] })])
+    ).toEqual(['01-Intro-v1.mp4']);
   });
 
   it('falls back to .mp4 when the url contains no dot at all', () => {