Refactor icon handling to properly use RPC and signed URLs #1274
Conversation
WalkthroughConsolidates image handling into a new ImageUploads backend service, brands image fields as ImageUpload types/keys, centralizes user/organization image update RPCs, migrates frontend RPC usage to a hook-based Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Frontend (rpc client)
participant Rpc as RPC layer
participant ImageSvc as ImageUploads
participant S3 as S3
participant DB as Database
UI->>Rpc: rpc.OrganisationUpdate({ id, image })
Rpc->>ImageSvc: applyUpdate(payload, existing?, keyPrefix, updateCallback)
alt payload is Some (new image)
ImageSvc->>S3: sanitize bytes & upload -> returns key
S3-->>ImageSvc: key
ImageSvc->>DB: update(org.iconUrl = key) via updateCallback
ImageSvc->>S3: delete previous key (if any)
S3-->>ImageSvc: deleted
else payload is None (remove)
ImageSvc->>DB: update(org.iconUrl = null) via updateCallback
ImageSvc->>S3: delete previous key (if any)
end
ImageSvc-->>Rpc: updated org
Rpc-->>UI: RPC response
sequenceDiagram
participant Page as Server page
participant Layer as makeCurrentUserLayer
participant Effect as Effect.runPromise
participant ImageUploads as ImageUploads.resolveImageUrl
participant S3 as S3
Page->>Effect: Effect.gen { provide(makeCurrentUserLayer(user)), db.use(...), ... }
Effect->>ImageUploads: resolveImageUrl(keyOrUrl)
alt key resolves to S3 key
ImageUploads->>S3: getSignedUrl(key)
S3-->>ImageUploads: signed URL
ImageUploads-->>Effect: ImageUrl
else passthrough
ImageUploads-->>Effect: original URL/key as ImageUrl
end
Effect-->>Page: resolved data for render
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (2)**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (1)📚 Learning: 2025-09-24T07:24:21.449ZApplied to files:
🧬 Code graph analysis (1)packages/web-backend/src/S3Buckets/index.ts (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
apps/web/components/pages/_components/ComparePlans.tsx (1)
363-372: Fix double action on “Free” click (navigates then also subscribes)The else applies when plan.name is "Free", so planCheckout() runs after setting window.location. Use if/else-if/else.
- if (plan.name === "Free") { - window.location.href = "/download"; - } - if (plan.name === "Desktop License") { - openCommercialCheckout(); - } else { - planCheckout(); - } + if (plan.name === "Free") { + window.location.href = "/download"; + } else if (plan.name === "Desktop License") { + openCommercialCheckout(); + } else { + planCheckout(); + }packages/web-backend/src/Users/UsersRpcs.ts (1)
46-61: Authorize signed-URL requests; ‘type’ is unused.GetSignedImageUrl signs arbitrary keys without verifying ownership/membership. Enforce allowed prefixes based on payload.type and CurrentUser (and org membership), or route through a DB lookup + policy before signing. Also use the ‘type’ to select/check the prefix; otherwise remove it.
Minimal guard example:
- const [bucket] = yield* s3Buckets.getBucketAccess(Option.none()); - const url = yield* bucket.getSignedObjectUrl(payload.key); + const [bucket] = yield* s3Buckets.getBucketAccess(Option.none()); + const user = yield* CurrentUser; + const allowed = payload.type === "user" + ? payload.key.startsWith(`users/${user.id}/`) + : /* verify org membership before allowing */ false; // replace with real policy check + if (!allowed) return yield* Effect.fail(new InternalError({ type: "unknown" })); + const url = yield* bucket.getSignedObjectUrl(payload.key);Alternatively, accept an entity id and derive the key from DB, not from client-provided input.
packages/web-backend/src/Folders/index.ts (3)
134-141: Incorrect Effect usage: double yield then pipe on a value causes runtime errorYou’re yielding the Effect, then calling
.pipe(...)on the resolved value. Pipe the catch on the Effect before yielding.- const folder = yield* (yield* repo - .getById(data.id) - .pipe(Policy.withPolicy(policy.canEdit(data.id)))).pipe( - Effect.catchTag( - "NoSuchElementException", - () => new Folder.NotFoundError(), - ), - ); + const folder = yield* repo + .getById(data.id) + .pipe( + Policy.withPolicy(policy.canEdit(data.id)), + Effect.catchTag( + "NoSuchElementException", + () => new Folder.NotFoundError(), + ), + );
156-164: Misuse of Effect.flatMap with catchTag
Effect.flatMap(Effect.catchTag(...))is invalid. Catch on the same pipeline instead.- .pipe( - Policy.withPolicy(policy.canEdit(parentId)), - Effect.flatMap( - Effect.catchTag( - "NoSuchElementException", - () => new Folder.ParentNotFoundError(), - ), - ), - ); + .pipe( + Policy.withPolicy(policy.canEdit(parentId)), + Effect.catchTag( + "NoSuchElementException", + () => new Folder.ParentNotFoundError(), + ), + );
167-181: Ancestor traversal treats value as Option; reconcile getById contract
repo.getByIdappears to fail with NoSuchElementException, but the code checksOption.isNone(nextParent). Convert the effect to Option (or catch) before testing.- const nextParent = yield* repo.getById(parentId, { - organizationId: Organisation.OrganisationId.make( - folder.organizationId, - ), - }); - - if (Option.isNone(nextParent)) break; - currentParentId = nextParent.value.parentId; + const nextParent = yield* repo + .getById(parentId, { + organizationId: Organisation.OrganisationId.make(folder.organizationId), + }) + .pipe(Effect.option); + if (Option.isNone(nextParent)) break; + currentParentId = nextParent.value.parentId;apps/web/app/Layout/AuthContext.tsx (1)
32-34: Incorrect error message in hookUse the correct provider/context names.
- throw new Error("useSiteContext must be used within a SiteContextProvider"); + throw new Error("useAuthContext must be used within an AuthContextProvider");apps/web/app/s/[videoId]/page.tsx (2)
642-712: Pipe provideOptionalAuth before runPromise; fix typo; minor nits.
- Missing provideOptionalAuth violates app/server Effect runtime guideline; services may lack auth context.
- Typo: toplLevelCommentId → topLevelCommentId.
- Minor: rename lambda variable to avoid shadowing comments table; optional.
- let toplLevelCommentId = Option.none<Comment.CommentId>(); + let topLevelCommentId = Option.none<Comment.CommentId>(); if (Option.isSome(replyId)) { const [parentComment] = yield* db.use((db) => db .select({ parentCommentId: comments.parentCommentId }) .from(comments) .where(eq(comments.id, replyId.value)) .limit(1), ); - toplLevelCommentId = Option.fromNullable(parentComment?.parentCommentId); + topLevelCommentId = Option.fromNullable(parentComment?.parentCommentId); } - const commentToBringToTheTop = Option.orElse( - toplLevelCommentId, + const commentToBringToTheTop = Option.orElse( + topLevelCommentId, () => commentId, );- .pipe( - Effect.map((comments) => - comments.map( + .pipe( + Effect.map((rows) => + rows.map( Effect.fn(function* (c) { return Object.assign(c, { authorImage: yield* Option.fromNullable(c.authorImage).pipe( Option.map((v) => imageUploads.resolveImageUrl( - v as ImageUpload.ImageUrlOrKey, + v as ImageUpload.ImageUrlOrKey, ), ), Effect.transposeOption, Effect.map(Option.getOrNull), ), }); }), ), ), Effect.flatMap(Effect.all), ); -}).pipe(EffectRuntime.runPromise); +}).pipe(provideOptionalAuth, EffectRuntime.runPromise);Optional:
- The cast to ImageUpload.ImageUrlOrKey is likely unnecessary given users.image is typed as that in schema; can remove after a quick compile check.
- Consider wrapping resolveImageUrl with a safe fallback to original value to avoid a hard failure if signing throws.
52-107: Resolve organization/space icon URLs before returning fromgetSharedSpacesForVideo.The
iconUrlfield in the database is typed asImageUpload.ImageUrlOrKey, confirming it can store S3 keys. Your function returns these raw values directly, exposing them to the client. Other parts of the codebase (e.g.,apps/web/app/(org)/dashboard/dashboard-data.ts) correctly resolve keys to signed URLs usingimageUploads.resolveImageUrl()before returning to the client. UpdategetSharedSpacesForVideoto follow this same pattern within the Effect system.
🧹 Nitpick comments (34)
infra/sst.config.ts (1)
155-158: Good security improvement with explicit OIDC subjects.The conditional logic correctly restricts production to only the production environment subject, while allowing both preview and staging for non-production stages. This is more secure than the previous wildcard-like pattern.
Note:
StringLikeis used with exact string matches (no wildcards). While functionally correct,StringEqualswould be more semantically appropriate for exact matches.If you prefer semantic precision, consider using
StringEqualsinstead:- StringLike: { + StringEquals: { [`${oidc.url}:sub`]:packages/web-domain/src/Rpcs.ts (1)
1-13: Filename styleConsider renaming this module to rpcs.ts to follow the kebab-case filename guideline.
packages/web-domain/src/Authentication.ts (1)
1-41: Filename styleConsider renaming this module to authentication.ts to follow the kebab-case filename guideline.
packages/web-backend/src/Database.ts (1)
10-15: Allow sync callbacks and normalize to Promise.Broadens utility and avoids forcing async lambdas when unnecessary.
- use: <T>(cb: (_: DbClient) => Promise<T>) => + use: <T>(cb: (_: DbClient) => Promise<T> | T) => Effect.tryPromise({ - try: () => cb(db()), + try: () => Promise.resolve(cb(db())), catch: (cause) => new DatabaseError({ cause }), }),apps/web/app/(org)/dashboard/folder/[id]/components/SubfolderDialog.tsx (2)
113-119: Trim folder name before sending.Prevents accidental whitespace-only or trailing-space names.
- rpc.FolderCreate({ - name: data.name, + rpc.FolderCreate({ + name: data.name.trim(), color: data.color, spaceId: Option.fromNullable(activeSpace?.id), parentId: Option.some(parentFolderId), }),
1-215: Filename styleConsider renaming this component file to subfolder-dialog.tsx per the kebab-case filename guideline for TSX modules. Component name can remain
SubfolderDialog.apps/web/components/forms/server.ts (1)
64-82: Sanitize uploaded icon and use sanitized content type; add extension fallback.Aligns with the space-icon flow and reduces risk from malformed files.
- const fileExtension = iconFile.name.split(".").pop(); - const fileKey = ImageUpload.ImageKey.make( - `organizations/${organizationId}/icon-${Date.now()}.${fileExtension}`, - ); + const fileExtension = + iconFile.name.includes(".") ? iconFile.name.split(".").pop()! : "png"; + const fileKey = ImageUpload.ImageKey.make( + `organizations/${organizationId}/icon-${Date.now()}.${fileExtension}`, + ); @@ - yield* bucket.putObject( - fileKey, - yield* Effect.promise(() => iconFile.bytes()), - { contentType: iconFile.type }, - ); + const sanitizedFile = yield* Effect.promise(() => sanitizeFile(iconFile)); + yield* bucket.putObject( + fileKey, + yield* Effect.promise(() => sanitizedFile.bytes()), + { contentType: sanitizedFile.type }, + );Add missing import at top if not present:
+import { sanitizeFile } from "@/lib/sanitizeFile";apps/web/app/(org)/dashboard/folder/[id]/components/FolderVideosSection.tsx (1)
1-173: Filename styleConsider renaming this component file to folder-videos-section.tsx per the kebab-case filename guideline. Component symbol can stay
FolderVideosSection.apps/web/actions/organization/upload-space-icon.ts (1)
54-61: Type the deletion key as ImageKey and use sanitized content type; add extension fallback.Ensures correct typing for S3 operations, consistent sanitization, and robustness when filenames lack extensions.
-const fileExtension = file.name.split(".").pop(); +const fileExtension = file.name.includes(".") ? file.name.split(".").pop()! : "png"; const fileKey = ImageUpload.ImageKey.make( `organizations/${ space.organizationId }/spaces/${spaceId}/icon-${Date.now()}.${fileExtension}`, ); @@ - if (space.iconUrl) { + if (space.iconUrl) { // Extract the S3 key (it might already be a key or could be a legacy URL) - const key = space.iconUrl.startsWith("organizations/") - ? space.iconUrl - : space.iconUrl.match(/organizations\/.+/)?.[0]; - if (key) { + const keyStr = space.iconUrl.startsWith("organizations/") + ? space.iconUrl + : space.iconUrl.match(/organizations\/.+/)?.[0]; + const key = keyStr ? ImageUpload.ImageKey.make(keyStr) : undefined; + if (key) { try { await bucket.deleteObject(key).pipe(runPromise); } catch (e) { // Log and continue console.warn("Failed to delete old space icon from S3", e); } } } @@ - await bucket + await bucket .putObject( fileKey, Effect.promise(() => sanitizedFile.bytes()), - { contentType: file.type }, + { contentType: sanitizedFile.type }, ) .pipe(runPromise);Also applies to: 66-81, 85-93
apps/web/app/(org)/dashboard/caps/page.tsx (1)
247-249: Consider providing optional auth to Effect pipelines in server components.Per prior guidance for apps/web/app/*, wrap Effect with provideOptionalAuth before runPromise, even if the current pipeline doesn’t require CurrentUser. This keeps future additions safe. Based on learnings.
-import { runPromise } from "@/lib/server"; +import { runPromise } from "@/lib/server"; +import { provideOptionalAuth } from "@cap/web-backend"; … -const sharedSpacesMap = - await getSharedSpacesForVideos(videoIds).pipe(runPromise); +const sharedSpacesMap = + await getSharedSpacesForVideos(videoIds).pipe(provideOptionalAuth, runPromise);packages/web-domain/src/ImageUpload.ts (1)
22-23: Broaden googleusercontent filter.Some avatars come from subdomains (e.g., lh4.googleusercontent.com). Match by hostname endsWith("googleusercontent.com") to avoid false negatives.
- const { pathname, origin } = new URL(iconKeyOrURL); - - if (origin === "https://lh3.googleusercontent.com") return Option.none(); + const { pathname, hostname } = new URL(iconKeyOrURL); + + if (hostname.endsWith("googleusercontent.com")) return Option.none();apps/web/app/(org)/dashboard/settings/organization/components/OrganizationIcon.tsx (5)
11-11: Remove unused importwithRpcNot referenced after the refactor.
-import { withRpc } from "@/lib/Rpcs";
42-45: Prefer targeted cache updates overrouter.refresh()Guideline: client mutations should perform focused cache updates with
setQueryData/setQueriesData(TanStack v5) instead of full refreshes.Update success handlers to update the relevant organization/icon query cache and avoid
router.refresh(). Based on coding guidelines.Also applies to: 60-62
89-95: Disable UI while either mutation is pendingCurrently tied only to
uploadIcon. Prevent conflicting actions during remove.- disabled={uploadIcon.isPending} - isLoading={uploadIcon.isPending} + disabled={uploadIcon.isPending || removeIcon.isPending} + isLoading={uploadIcon.isPending || removeIcon.isPending}
46-50: Align error logging behavior
removeIcon.onErrorlogs to console;uploadIcon.onErrordoes not. Keep consistent (either both log or neither).Also applies to: 63-68
1-1: Filename should be kebab-casePer repo convention, module filenames use kebab-case. Consider renaming to
organization-icon.tsx. Component can remainOrganizationIcon.Based on coding guidelines.
packages/web-domain/src/User.ts (1)
19-38: Optional: reuseImageUpdatePayloadin onboarding icon schemaFor consistency with image updates elsewhere and transport encoding, consider
ImageUpdatePayloadinstead of rawUint8Array.apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx (1)
45-45: Remove unused import.The
withRpcimport is no longer used after migrating to theuseRpcClient()pattern.Apply this diff:
-import { withRpc } from "@/lib/Rpcs";apps/web/app/(org)/onboarding/components/OrganizationSetupPage.tsx (1)
12-12: Remove unused import.The
withRpcimport is no longer used after migrating to theuseRpcClient()pattern.Apply this diff:
-import { withRpc } from "@/lib/Rpcs";apps/web/app/s/[videoId]/_components/tabs/Activity/Comment.tsx (1)
2-2: Remove unused importAvatar is imported but unused. Safe to drop.
-import { Avatar, Button } from "@cap/ui"; +import { Button } from "@cap/ui";apps/web/app/(org)/dashboard/caps/components/NewFolderDialog.tsx (2)
117-119: Prefer targeted cache updates over full router refreshInstead of router.refresh(), invalidate or update the relevant TanStack Query keys (e.g., space folders listing) to avoid full-page refresh and re-render cost.
Example sketch:
// after success: queryClient.setQueriesData( { queryKey: ["folders", spaceId], exact: false }, (prev: Folder[]) => (prev ? [...prev, { id, name: folderName, color: selectedColor, parentId: null, spaceId }] : prev) ); // or queryClient.invalidateQueries({ queryKey: ["folders", spaceId] });Also applies to: 126-201
105-113: Align with apps/web mutation guideline (server actions vs RPC)Guideline: “Mutations should call Server Actions directly and perform targeted cache updates.” You’re invoking rpc.FolderCreate from the client. If possible, wrap FolderCreate in a server action and call that here, keeping the RPC on the server side. If the team decided to standardize on client RPC for now, please note the deviation.
apps/web/app/(org)/onboarding/components/InviteTeamPage.tsx (1)
14-15: Remove unused import.The
withRpcimport on line 15 is no longer used after migrating to the useRpcClient pattern.Apply this diff:
-import { useEffectMutation, useRpcClient } from "@/lib/EffectRuntime"; -import { withRpc } from "@/lib/Rpcs"; +import { useEffectMutation, useRpcClient } from "@/lib/EffectRuntime";packages/web-backend/src/Users/UsersRpcs.ts (2)
62-68: Normalize error surface and consider returning updated data.
- UserUpdate maps only DatabaseError/S3Error; unknown errors will leak. Add a catchAll to map to InternalError({ type: "unknown" }) for parity with GetSignedImageUrl.
- Consider returning the updated user (or at least the resolved image URL) to let clients update cache without a full refresh.
Apply:
- UserUpdate: (data) => - users.update(data).pipe( - Effect.catchTags({ - DatabaseError: () => new InternalError({ type: "database" }), - S3Error: () => new InternalError({ type: "s3" }), - }), - ), + UserUpdate: (data) => + users.update(data).pipe( + Effect.catchTags({ + DatabaseError: () => new InternalError({ type: "database" }), + S3Error: () => new InternalError({ type: "s3" }), + }), + Effect.catchAll(() => new InternalError({ type: "unknown" })), + ),
46-49: Remove or use ‘type’.The ‘type’ field is currently unused. Use it for prefix/policy selection or drop it from the API to avoid confusion.
packages/web-backend/src/Organisations/index.ts (3)
8-9: Drop unused S3Buckets import.S3Buckets isn’t used in this module; keep dependencies minimal.
-import { S3Buckets } from "../S3Buckets";
56-61: Remove unused S3Buckets.Default from dependencies.dependencies: [ ImageUploads.Default, - S3Buckets.Default, Database.Default, OrganisationsPolicy.Default, ],
38-51: Optional: return updated icon URL.Consider returning the resolved iconUrl after update to enable clients to update caches without a reload.
apps/web/app/(org)/dashboard/settings/account/Settings.tsx (3)
142-147: Add client-side guard for file size/type before reading.Prevent large/invalid uploads early to save memory and avoid stalled UX.
const handleProfileImageChange = (file: File | null) => { if (!file || isProfileImageMutating) { return; } + const maxBytes = 5 * 1024 * 1024; // 5MB + const allowed = ["image/png", "image/jpeg", "image/webp", "image/svg+xml"]; + if (file.size > maxBytes || !allowed.includes(file.type)) { + toast.error("Please upload a PNG, JPG, WEBP, or SVG under 5MB."); + return; + } uploadProfileImageMutation.mutate(file); };
19-21: Remove unused withRpc import.-import { withRpc } from "@/lib/Rpcs";
179-180: Minor: user is required; drop optional chaining.- userName={user?.name} + userName={user.name}apps/web/app/s/[videoId]/_components/Toolbar.tsx (1)
36-44: Prompt auth on emoji click when logged outCurrently returns silently. Align UX with comment button by opening the auth overlay.
- if (!user) return; + if (!user) { + setShowAuthOverlay(true); + return; + }packages/web-backend/src/ImageUploads/index.ts (1)
16-49: Solid abstraction for upload/update/delete and URL resolutionThe sequencing and key extraction look correct. Nice consolidation.
Optional: consider validating
image.contentTypeagainst an allowlist (e.g., image/*) and capping payload size before upload.Also applies to: 52-61
apps/web/app/s/[videoId]/page.tsx (1)
17-23: Avoid deep import; import VideosPolicy from the public entrypoint.Deep-importing from src breaks package encapsulation and bundling. Pull VideosPolicy from @cap/web-backend alongside the other services.
Apply:
-import { - Database, - ImageUploads, - provideOptionalAuth, - Videos, -} from "@cap/web-backend"; -import { VideosPolicy } from "@cap/web-backend/src/Videos/VideosPolicy"; +import { + Database, + ImageUploads, + provideOptionalAuth, + Videos, + VideosPolicy, +} from "@cap/web-backend";
apps/web/app/(org)/dashboard/settings/organization/components/OrganizationIcon.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/app/(org)/dashboard/caps/page.tsx (1)
147-165:sharedOrganizationsis built from the wrong join; duplicates and wrong data.You aggregate from
organizationswithout joiningshared_videos, so you get the owner org repeated (and multiplied by comment rows). Build it fromshared_videos → organizationsinstead and de‑dupe.Apply this replacement for the
sharedOrganizationsprojection:- sharedOrganizations: sql< - { - id: string; - name: string; - iconUrl: ImageUpload.ImageUrlOrKey | null; - }[] - >` - COALESCE( - JSON_ARRAYAGG( - JSON_OBJECT( - 'id', ${organizations.id}, - 'name', ${organizations.name}, - 'iconUrl', ${organizations.iconUrl} - ) - ), - JSON_ARRAY() - ) - `, + sharedOrganizations: sql< + { + id: string; + name: string; + iconUrl: ImageUpload.ImageUrlOrKey | null; + }[] + >` + COALESCE( + ( + SELECT JSON_ARRAYAGG(DISTINCT JSON_OBJECT( + 'id', o2.id, + 'name', o2.name, + 'iconUrl', o2.iconUrl + )) + FROM shared_videos sv + JOIN organizations o2 ON sv.organizationId = o2.id + WHERE sv.videoId = ${videos.id} + ), + JSON_ARRAY() + ) + `,This avoids the comments join fan‑out and returns actual share recipients.
apps/web/lib/folder.ts (1)
182-199: JSON aggregation type/serialization mismatch and duplicate rows risk.
- Drizzle/MySQL JSON aggregation commonly returns a JSON string at runtime; the code later treats
video.sharedOrganizationsas an array (calls.filter), which will throw if it’s actually a string.- Left joins to
sharedVideosandspaceVideoscan multiply rows, producing duplicate org objects inside the JSON array.Normalize and dedupe in TS before mapping, instead of trusting the
sql<…>type parameter.Apply this diff together with the next comment’s diff (lines 251-292):
- COALESCE( + COALESCE( JSON_ARRAYAGG( JSON_OBJECT( 'id', ${organizations.id}, 'name', ${organizations.name}, 'iconUrl', ${organizations.iconUrl} ) ), JSON_ARRAY() )And in the mapping (below), parse if string, then dedupe by
idbefore resolving URLs.
♻️ Duplicate comments (1)
apps/web/app/(org)/dashboard/caps/page.tsx (1)
18-18: Stop importingtype Arrayfrom "effect" — it shadows the built‑in and breaks typing.This masks the global
Array<T>in type space, causingArray<...>annotations below to fail. Use the built‑in array type and import Effect only.Apply:
-import { type Array, Effect } from "effect"; +import { Effect } from "effect";If you need helpers, import the module namespace instead:
import * as Arr from "effect/Array";
🧹 Nitpick comments (5)
apps/web/app/(org)/dashboard/caps/page.tsx (5)
37-49: Unnecessary join toorganizationsin space sharing query.You don't use any
organizationsfields in this select. Dropping the join reduces work.- .innerJoin(organizations, eq(spaces.organizationId, organizations.id))
52-67:orgSharingselectsiconUrlbut you discard it; either resolve and return it or drop the column.Currently
iconUrlis selected but not used when buildingsharedSpacesMap. If shared space cards need icons, resolve here viaImageUploads.resolveImageUrl; otherwise remove it from the select.Option A (keep and resolve later): leave as is but propagate
iconUrlintosharedSpacesMapand resolve withImageUploads.Option B (no icons for sharedSpaces): remove from select:
- iconUrl: organizations.iconUrl,Please confirm which path matches Caps/CapCard expectations. Based on PR goals.
233-270: Icon URL resolution flow looks good; consider de‑duping per video to avoid repeated S3 signing.If multiple org entries share the same
iconUrl, resolve once per unique value.- sharedOrganizations: yield* Effect.all( - (video.sharedOrganizations ?? []) - .filter((o) => o.id !== null) - .map(Effect.fn(function* (org) { - return { ...org, iconUrl: org.iconUrl ? yield* imageUploads.resolveImageUrl(org.iconUrl) : null }; - })), -), + sharedOrganizations: yield* (video.sharedOrganizations ?? []) + .filter((o) => o.id !== null) + .reduce((acc, o) => acc.set(o.iconUrl ?? "__null__", o), new Map<string, typeof (video.sharedOrganizations)[number]>()) + |> Array.from + |> ((entries) => + Effect.all( + entries.map(([, org]) => + Effect.fn(function* () { + return { + ...org, + iconUrl: org.iconUrl ? yield* imageUploads.resolveImageUrl(org.iconUrl) : null, + }; + })(), + ), + ) + ),Keeps output identical while minimizing repeated signing calls.
262-266: Avoidanyin metadata; preferunknown.Tighten the cast to keep strict typing.
- metadata: video.metadata as - | { - customCreatedAt?: string; - [key: string]: any; - } - | undefined, + metadata: video.metadata as + | { + customCreatedAt?: string; + [key: string]: unknown; + } + | undefined,As per coding guidelines.
123-133: Unify DB access via theDatabaseservice for consistency and error mapping.You use
db()directly here butDatabase.useelsewhere. Consider wrapping these queries inDatabase.useto standardize error handling and DI.Also applies to: 136-206, 208-226
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web/app/(org)/dashboard/caps/Caps.tsx(4 hunks)apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx(5 hunks)apps/web/app/(org)/dashboard/caps/page.tsx(4 hunks)apps/web/lib/folder.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/app/(org)/dashboard/caps/Caps.tsx
- apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.Use strict TypeScript and avoid any; leverage shared types
Files:
apps/web/lib/folder.tsapps/web/app/(org)/dashboard/caps/page.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/web/lib/folder.tsapps/web/app/(org)/dashboard/caps/page.tsx
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/lib/folder.tsapps/web/app/(org)/dashboard/caps/page.tsx
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for all client-side server state and fetching in the web app
Mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData
Run server-side effects via the ManagedRuntime from apps/web/lib/server.ts using EffectRuntime.runPromise/runPromiseExit; do not create runtimes ad hoc
Client code should use helpers from apps/web/lib/EffectRuntime.ts (useEffectQuery, useEffectMutation, useRpcClient); never call ManagedRuntime.make inside components
Files:
apps/web/lib/folder.tsapps/web/app/(org)/dashboard/caps/page.tsx
apps/web/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Server components needing Effect services must call EffectRuntime.runPromise(effect.pipe(provideOptionalAuth))
Files:
apps/web/app/(org)/dashboard/caps/page.tsx
🧬 Code graph analysis (2)
apps/web/lib/folder.ts (2)
packages/web-backend/src/index.ts (1)
ImageUploads(6-6)packages/web-domain/src/ImageUpload.ts (1)
ImageUrlOrKey(9-9)
apps/web/app/(org)/dashboard/caps/page.tsx (8)
packages/web-domain/src/Video.ts (3)
Video(16-59)VideoId(12-12)VideoId(13-13)packages/database/index.ts (1)
db(18-25)packages/web-backend/src/Database.ts (1)
Database(7-17)packages/database/schema.ts (4)
spaceVideos(648-668)spaces(591-619)organizations(172-206)sharedVideos(345-373)packages/web-domain/src/ImageUpload.ts (1)
ImageUrlOrKey(9-9)apps/web/lib/server.ts (1)
runPromise(130-142)packages/web-backend/src/ImageUploads/index.ts (1)
ImageUploads(7-67)packages/web-backend/src/index.ts (1)
ImageUploads(6-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (5)
apps/web/app/(org)/dashboard/caps/page.tsx (2)
231-232: Confirm Effect auth provisioning for server components.Guidelines say server components needing Effect services should run with
provideOptionalAuth. VerifyrunPromisealready applies it; if not, pipe it:await getSharedSpacesForVideos(videoIds) .pipe(provideOptionalAuth, runPromise);Same for the
Effect.all(...)pipeline. As per coding guidelines.Also applies to: 270-270
29-36: LGTM on the Effect‑based helper.Generator style +
Database.useis clean and composable.Also applies to: 106-107
apps/web/lib/folder.ts (3)
14-15: ImageUploads layer is correctly provided in the server runtime.Verification confirms that
ImageUploads.Defaultis included inapps/web/lib/server.tsline 114 within theDependencieslayer, which is composed into theManagedRuntimeon line 128. The folder.ts effects will correctly access the ImageUploads layer viayield* ImageUploadswithout runtime failures.
20-20: Remove unusedrunPromiseimport on line 20.The import is not referenced anywhere in the file and should be deleted.
⛔ Skipped due to learnings
Learnt from: CR PR: CapSoftware/Cap#0 File: CLAUDE.md:0-0 Timestamp: 2025-10-14T10:15:44.019Z Learning: Applies to apps/web/**/*.{ts,tsx} : Run server-side effects via the ManagedRuntime from apps/web/lib/server.ts using EffectRuntime.runPromise/runPromiseExit; do not create runtimes ad hoc
251-292: Use TypeScript-side deduplication (MySQL 8 doesn't support DISTINCT in JSON_ARRAYAGG); verify JSON parsing is necessary before applying the full refactor.MySQL 8 does not support DISTINCT inside JSON_ARRAYAGG, confirming that the proposed TypeScript-side deduplication approach is correct.
Effect.forEach supports a
concurrencyoption (numeric, "unbounded", or "inherit"), so the proposed{ concurrency: 8 }syntax is valid.However, before applying the full refactor:
- JSON parsing concern: The diff assumes
sharedOrganizationsmay be a JSON string, but your current code treats it as an array. Verify whether Drizzle ORM returns aggregated organization data as a JSON string or native array in your actual deployment. Without this confirmation, JSON parsing may be unnecessary overhead.- Deduplication necessity: Confirm whether the LEFT JOIN + GROUP BY pattern actually produces duplicate organizations or spaces. If the current data is already deduplicated, this step may not be needed.
- sharedSpaces icon resolution: This is a new feature (currently icons are not resolved). Decide if this enhancement should be included in the same change.
Once verified, the refactor is technically sound. If you proceed, the proposed diff correctly uses Effect.forEach with concurrency bounding and resolves both organization and space icons for consistency.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
apps/web/app/(org)/dashboard/settings/organization/components/CapSettingsCard.tsx (1)
4-4: Remove unused import.The
userIsProimport is no longer used after refactoring to directuser.isProaccess on line 180.Apply this diff to remove the unused import:
-import { userIsPro } from "@cap/utils";apps/web/app/(org)/dashboard/spaces/[spaceId]/page.tsx (1)
321-330: Bug: count query filters on videos.folderId instead of sharedVideos.folderIdThis makes the count diverge from the list query, miscounting items in folders.
- .where( - and( - eq(sharedVideos.organizationId, orgId), - isNull(videos.folderId), - ), - ), + .where( + and( + eq(sharedVideos.organizationId, orgId), + isNull(sharedVideos.folderId), + ), + ),apps/web/app/(org)/dashboard/caps/components/SharingDialog.tsx (1)
32-41: Type inconsistency detected.There's a mismatch between the
SharingDialogProps.sharedSpaces.iconUrltype (line 35:string | null) and theSpaceCardparameter type (line 391:ImageUpload.ImageUrl | null). WhenfilteredSpaces(derived fromallShareableItemsandspacesData) is passed toSpaceCard, there may be a type error if the source data hasiconUrlas a plain string.Consider updating the
sharedSpacesprop type to useImageUpload.ImageUrl | nullfor consistency:sharedSpaces: { id: string; name: string; - iconUrl?: string | null; + iconUrl?: ImageUpload.ImageUrl | null; organizationId: string; }[];Also applies to: 388-397
apps/web/app/s/[videoId]/page.tsx (1)
642-708: Pipe provideOptionalAuth when running server Effects.Per guidelines, server-side Effect runs should include provideOptionalAuth.
As per coding guidelines
- }).pipe(EffectRuntime.runPromise); + }).pipe(provideOptionalAuth, EffectRuntime.runPromise);apps/web/app/Layout/AuthContext.tsx (1)
33-36: Fix incorrect error message.Message references SiteContext; should reference AuthContext.
Apply:
- throw new Error("useSiteContext must be used within a SiteContextProvider"); + throw new Error("useAuthContext must be used within an AuthContextProvider");
♻️ Duplicate comments (3)
apps/web/app/layout.tsx (1)
134-136: Inline HTML injection: add CSP/nonce or use next/scriptSame concern as before: add a CSP nonce or switch to next/script beforeInteractive to harden against XSS.
- <script - dangerouslySetInnerHTML={{ __html: `(${script.toString()})()` }} - /> + {/* Prefer Next Script with CSP nonce */} + {/* <Script id="theme" strategy="beforeInteractive" nonce={serverEnv().CSP_NONCE}> + {`(${script.toString()})()`} + </Script> */}apps/web/app/(org)/dashboard/settings/organization/components/OrganizationIcon.tsx (1)
4-4: Fix type: use OrganisationId (not Organisation.OrganisationId).This won’t type-check; the branded ID is exported at module scope. Import and use OrganisationId directly.
-import type { Organisation } from "@cap/web-domain"; +import type { OrganisationId } from "@cap/web-domain"; @@ - organizationId: Organisation.OrganisationId; + organizationId: OrganisationId; @@ - mutationFn: (organizationId: Organisation.OrganisationId) => + mutationFn: (organizationId: OrganisationId) =>Also applies to: 28-30, 54-55
apps/web/app/(org)/dashboard/dashboard-data.ts (1)
131-171: Duplicate space rows due to joins + wide DISTINCT; remove joins and use EXISTS.Left‑joining space_members/users and selecting memberImage yields multiple rows per space; selectDistinct won’t dedupe when any selected column differs. Also memberImage isn’t used downstream.
Apply:
- return yield* db - .use((db) => - db - .selectDistinct({ + return yield* db + .use((db) => + db + .select({ id: spaces.id, primary: spaces.primary, privacy: spaces.privacy, name: spaces.name, description: spaces.description, organizationId: spaces.organizationId, createdById: spaces.createdById, iconUrl: spaces.iconUrl, - memberImage: users.image, memberCount: sql<number>`( - SELECT COUNT(*) FROM space_members WHERE space_members.spaceId = spaces.id + SELECT COUNT(*) FROM space_members WHERE space_members.spaceId = ${spaces.id} )`, videoCount: sql<number>`( - SELECT COUNT(*) FROM space_videos WHERE space_videos.spaceId = spaces.id + SELECT COUNT(*) FROM space_videos WHERE space_videos.spaceId = ${spaces.id} )`, }) .from(spaces) - .leftJoin(spaceMembers, eq(spaces.id, spaceMembers.spaceId)) - .leftJoin(users, eq(spaceMembers.userId, users.id)) .where( and( - eq(spaces.organizationId, activeOrganizationId), - or( - // User is the space creator - eq(spaces.createdById, user.id), - // User is a member of the space - eq(spaceMembers.userId, user.id), - // Space is public within the organization - eq(spaces.privacy, "Public"), - ), + eq(spaces.organizationId, activeOrganizationId), + or( + eq(spaces.createdById, user.id), + sql<boolean>`exists( + select 1 from space_members + where space_members.spaceId = ${spaces.id} + and space_members.userId = ${user.id} + )`, + eq(spaces.privacy, "Public") + ) ), ), )This removes duplication and unnecessary joins/columns.
🧹 Nitpick comments (21)
apps/web/app/(org)/dashboard/caps/components/SettingsDialog.tsx (1)
10-10: Remove unused import.The
userIsProimport is no longer used after the refactoring to directly accessuser.isPro.Apply this diff to remove the unused import:
-import { userIsPro } from "@cap/utils";apps/web/app/(org)/dashboard/_components/Navbar/SpaceDialog.tsx (2)
201-201: Replaceanywithunknownfor stricter type safety.The
anytype violates the TypeScript strictness guideline. Useunknowninstead and narrow the type when accessing properties.As per coding guidelines.
- } catch (error: any) { + } catch (error: unknown) { console.error( edit ? "Error updating space:" : "Error creating space:", error, ); toast.error( - error?.message || - error?.error || + (error instanceof Error ? error.message : null) || + (typeof error === 'object' && error !== null && 'error' in error ? String(error.error) : null) || (edit ? "Failed to update space" : "Failed to create space"), );
1-294: Consider renaming file to follow kebab-case convention.The filename
SpaceDialog.tsxuses PascalCase, but the coding guidelines specify kebab-case for TypeScript/JavaScript module filenames. Consider renaming tospace-dialog.tsx.As per coding guidelines.
apps/web/app/(org)/dashboard/spaces/[spaceId]/components/MemberSelect.tsx (2)
67-69: Remove unused triggerWidth ref/effecttriggerWidth is written but never read; keep width from triggerRef only.
- const triggerWidth = useRef<number>(0); const triggerRef = useRef<HTMLDivElement>(null); @@ - useEffect(() => { - if (triggerRef.current) { - triggerWidth.current = triggerRef.current.offsetWidth; - } - }, []); + // triggerRef is sufficient; DropdownMenuContent reads offsetWidth directly.Also applies to: 93-98
112-120: A11y: disable focus when disabledContainer remains focusable while aria-disabled; align tabIndex with disabled.
- tabIndex={0} - aria-disabled={disabled} + tabIndex={disabled ? -1 : 0} + aria-disabled={disabled || undefined}apps/web/app/layout.tsx (1)
83-106: Server Effect: provide optional auth per guidelineServer components using Effect services should pipe provideOptionalAuth before runPromise. Please verify the correct provider is applied here.
If available:
- }).pipe(runPromise); + }).pipe(provideOptionalAuth, runPromise);And:
import { runPromise, provideOptionalAuth } from "@/lib/server";apps/web/app/(org)/dashboard/spaces/[spaceId]/page.tsx (2)
207-214: Group By consistency: include videos.durationThe SELECT includes videos.duration but it’s not grouped here (unlike the org query). Add it to avoid ONLY_FULL_GROUP_BY issues.
.groupBy( videos.id, videos.ownerId, videos.name, videos.createdAt, videos.metadata, - users.name, + users.name, + videos.duration, )
39-45: Simplify SpaceMemberData.image typeMake image non-optional with null to reduce branching.
-export type SpaceMemberData = { +export type SpaceMemberData = { id: string; userId: string; role: string; - image?: ImageUpload.ImageUrl | null; + image: ImageUpload.ImageUrl | null; name: string | null; email: string; };apps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersDialog.tsx (1)
3-3: Unused import detected.The
ImageUpdatePayloadimport appears to be unused in this file.Apply this diff to remove the unused import:
-import { ImageUpdatePayload } from "@cap/web-domain/src/ImageUpload";apps/web/app/(org)/dashboard/settings/organization/components/OrganizationIcon.tsx (1)
11-11: Remove unused import.withRpc is unused here; drop it.
-import { withRpc } from "@/lib/Rpcs";apps/web/components/FileInput.tsx (1)
18-21: Consider allowing WebP.If backend supports it, include image/webp for smaller uploads.
-const ALLOWED_IMAGE_TYPES = new Set(["image/jpeg", "image/png"]); +const ALLOWED_IMAGE_TYPES = new Set(["image/jpeg", "image/png", "image/webp"]);apps/web/app/(org)/dashboard/settings/account/Settings.tsx (1)
3-3: Drop unused imports.users (type) and withRpc aren’t used.
-import type { users } from "@cap/database/schema"; @@ -import { withRpc } from "@/lib/Rpcs";Also applies to: 20-20
apps/web/app/s/[videoId]/page.tsx (2)
684-689: Refine orderBy: pass separate expressions instead of a single SQL literal with a comma.Clearer typing; avoids embedding multiple order terms in one sql``.
- .orderBy( - Option.match(commentToBringToTheTop, { - onSome: (commentId) => - sql`CASE WHEN ${comments.id} = ${commentId} THEN 0 ELSE 1 END, ${comments.createdAt}`, - onNone: () => comments.createdAt, - }), - ), + .orderBy( + ...Option.match(commentToBringToTheTop, { + onSome: (commentId) => [ + sql`CASE WHEN ${comments.id} = ${commentId} THEN 0 ELSE 1 END`, + comments.createdAt, + ], + onNone: () => [comments.createdAt], + }), + ),
26-26: Remove unused type import.type ImageUpload isn’t referenced.
- Comment, - type ImageUpload, + Comment,apps/web/app/Layout/AuthContext.tsx (3)
6-14: Avoid type name collision with domain CurrentUser.This file exports a client‑shape CurrentUser while packages/web-domain also defines CurrentUser (Context.Tag). Rename this type (e.g., UiCurrentUser) or re‑export with an alias to prevent confusion and drift.
40-42: Annotate hook return type for clarity.Explicit return type helps callers and avoids accidental widening during refactors.
Apply:
-export function useCurrentUser() { - return use(useAuthContext().user); -} +export function useCurrentUser(): CurrentUser | null { + return use(useAuthContext().user); +}
1-1: Filename style.Guidelines prefer kebab‑case for TS/TSX module filenames. Consider renaming to auth-context.tsx. Imports will need updating.
As per coding guidelines
apps/web/app/(org)/dashboard/dashboard-data.ts (4)
173-186: Cap concurrency when resolving signed URLs for spaces.Avoid bursty S3 URL generation under large result sets.
Apply:
- ).pipe( - Effect.map((rows) => - rows.map( - Effect.fn(function* (row) { - return { - ...row, - iconUrl: row.iconUrl - ? yield* imageUploads.resolveImageUrl(row.iconUrl) - : null, - }; - }), - ), - ), - Effect.flatMap(Effect.all), - ); + ).pipe( + Effect.map((rows) => + rows.map( + Effect.fn(function* (row) { + return { + ...row, + iconUrl: row.iconUrl + ? yield* imageUploads.resolveImageUrl(row.iconUrl) + : null, + }; + }), + ), + ), + Effect.flatMap((effects) => Effect.all(effects, { concurrency: 20 })), + );
337-350: Limit concurrency for member image URL resolution.Prevents spikes if organizations have many members.
Apply:
- members: yield* Effect.all(allMembers.map( + members: yield* Effect.all(allMembers.map( Effect.fn(function* (m) { const imageUploads = yield* ImageUploads; return { ...m.member, user: { ...m.user!, image: m.user!.image ? yield* imageUploads.resolveImageUrl(m.user!.image) : null, }, }; }), - )), + ), { concurrency: 20 }),
20-33: User.image marked optional but code always sets it.Consider making image non‑optional (ImageUpload.ImageUrl | null) for consistency.
52-79: Drop unused columns/joins in organizationsWithMembers.You fetch users.inviteQuota/defaultOrgId here but later fetch owner.inviteQuota separately; if not used, remove the users join/fields to reduce result size.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
apps/web/app/(org)/dashboard/Contexts.tsx(3 hunks)apps/web/app/(org)/dashboard/_components/MobileTab.tsx(0 hunks)apps/web/app/(org)/dashboard/_components/Navbar/Items.tsx(0 hunks)apps/web/app/(org)/dashboard/_components/Navbar/SpaceDialog.tsx(3 hunks)apps/web/app/(org)/dashboard/_components/Navbar/SpacesList.tsx(0 hunks)apps/web/app/(org)/dashboard/_components/Navbar/Top.tsx(1 hunks)apps/web/app/(org)/dashboard/caps/components/SettingsDialog.tsx(1 hunks)apps/web/app/(org)/dashboard/caps/components/SharingDialog.tsx(2 hunks)apps/web/app/(org)/dashboard/caps/components/UploadCapButton.tsx(1 hunks)apps/web/app/(org)/dashboard/dashboard-data.ts(6 hunks)apps/web/app/(org)/dashboard/folder/[id]/components/ClientMyCapsLink.tsx(0 hunks)apps/web/app/(org)/dashboard/layout.tsx(1 hunks)apps/web/app/(org)/dashboard/settings/account/Settings.tsx(3 hunks)apps/web/app/(org)/dashboard/settings/account/components/ProfileImage.tsx(4 hunks)apps/web/app/(org)/dashboard/settings/account/page.tsx(1 hunks)apps/web/app/(org)/dashboard/settings/organization/components/CapSettingsCard.tsx(1 hunks)apps/web/app/(org)/dashboard/settings/organization/components/OrganizationIcon.tsx(3 hunks)apps/web/app/(org)/dashboard/spaces/[spaceId]/components/MemberSelect.tsx(3 hunks)apps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersDialog.tsx(2 hunks)apps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersIndicator.tsx(0 hunks)apps/web/app/(org)/dashboard/spaces/[spaceId]/components/OrganizationIndicator.tsx(3 hunks)apps/web/app/(org)/dashboard/spaces/[spaceId]/page.tsx(6 hunks)apps/web/app/(org)/dashboard/spaces/browse/page.tsx(0 hunks)apps/web/app/Layout/AuthContext.tsx(2 hunks)apps/web/app/layout.tsx(2 hunks)apps/web/app/s/[videoId]/_components/CommentStamp.tsx(3 hunks)apps/web/app/s/[videoId]/_components/ShareHeader.tsx(4 hunks)apps/web/app/s/[videoId]/_components/tabs/Activity/Comment.tsx(3 hunks)apps/web/app/s/[videoId]/page.tsx(3 hunks)apps/web/components/FileInput.tsx(4 hunks)apps/web/components/SignedImageUrl.tsx(1 hunks)apps/web/components/forms/NewOrganization.tsx(0 hunks)apps/web/lib/use-signed-image-url.ts(0 hunks)packages/web-backend/src/Auth.ts(4 hunks)packages/web-backend/src/Organisations/index.ts(1 hunks)
💤 Files with no reviewable changes (8)
- apps/web/app/(org)/dashboard/_components/MobileTab.tsx
- apps/web/app/(org)/dashboard/folder/[id]/components/ClientMyCapsLink.tsx
- apps/web/app/(org)/dashboard/spaces/browse/page.tsx
- apps/web/app/(org)/dashboard/_components/Navbar/Items.tsx
- apps/web/app/(org)/dashboard/_components/Navbar/SpacesList.tsx
- apps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersIndicator.tsx
- apps/web/components/forms/NewOrganization.tsx
- apps/web/lib/use-signed-image-url.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/web-backend/src/Organisations/index.ts
- apps/web/app/s/[videoId]/_components/tabs/Activity/Comment.tsx
- packages/web-backend/src/Auth.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.Use strict TypeScript and avoid any; leverage shared types
Files:
apps/web/app/(org)/dashboard/_components/Navbar/Top.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/OrganizationIndicator.tsxapps/web/app/(org)/dashboard/caps/components/SharingDialog.tsxapps/web/app/(org)/dashboard/settings/organization/components/OrganizationIcon.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/MemberSelect.tsxapps/web/app/(org)/dashboard/settings/organization/components/CapSettingsCard.tsxapps/web/app/(org)/dashboard/settings/account/Settings.tsxapps/web/app/(org)/dashboard/layout.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersDialog.tsxapps/web/app/s/[videoId]/page.tsxapps/web/app/(org)/dashboard/settings/account/components/ProfileImage.tsxapps/web/app/layout.tsxapps/web/app/(org)/dashboard/_components/Navbar/SpaceDialog.tsxapps/web/app/s/[videoId]/_components/ShareHeader.tsxapps/web/components/SignedImageUrl.tsxapps/web/app/s/[videoId]/_components/CommentStamp.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/page.tsxapps/web/app/(org)/dashboard/caps/components/SettingsDialog.tsxapps/web/components/FileInput.tsxapps/web/app/(org)/dashboard/caps/components/UploadCapButton.tsxapps/web/app/Layout/AuthContext.tsxapps/web/app/(org)/dashboard/Contexts.tsxapps/web/app/(org)/dashboard/settings/account/page.tsxapps/web/app/(org)/dashboard/dashboard-data.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/web/app/(org)/dashboard/_components/Navbar/Top.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/OrganizationIndicator.tsxapps/web/app/(org)/dashboard/caps/components/SharingDialog.tsxapps/web/app/(org)/dashboard/settings/organization/components/OrganizationIcon.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/MemberSelect.tsxapps/web/app/(org)/dashboard/settings/organization/components/CapSettingsCard.tsxapps/web/app/(org)/dashboard/settings/account/Settings.tsxapps/web/app/(org)/dashboard/layout.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersDialog.tsxapps/web/app/s/[videoId]/page.tsxapps/web/app/(org)/dashboard/settings/account/components/ProfileImage.tsxapps/web/app/layout.tsxapps/web/app/(org)/dashboard/_components/Navbar/SpaceDialog.tsxapps/web/app/s/[videoId]/_components/ShareHeader.tsxapps/web/components/SignedImageUrl.tsxapps/web/app/s/[videoId]/_components/CommentStamp.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/page.tsxapps/web/app/(org)/dashboard/caps/components/SettingsDialog.tsxapps/web/components/FileInput.tsxapps/web/app/(org)/dashboard/caps/components/UploadCapButton.tsxapps/web/app/Layout/AuthContext.tsxapps/web/app/(org)/dashboard/Contexts.tsxapps/web/app/(org)/dashboard/settings/account/page.tsxapps/web/app/(org)/dashboard/dashboard-data.ts
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/app/(org)/dashboard/_components/Navbar/Top.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/OrganizationIndicator.tsxapps/web/app/(org)/dashboard/caps/components/SharingDialog.tsxapps/web/app/(org)/dashboard/settings/organization/components/OrganizationIcon.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/MemberSelect.tsxapps/web/app/(org)/dashboard/settings/organization/components/CapSettingsCard.tsxapps/web/app/(org)/dashboard/settings/account/Settings.tsxapps/web/app/(org)/dashboard/layout.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersDialog.tsxapps/web/app/s/[videoId]/page.tsxapps/web/app/(org)/dashboard/settings/account/components/ProfileImage.tsxapps/web/app/layout.tsxapps/web/app/(org)/dashboard/_components/Navbar/SpaceDialog.tsxapps/web/app/s/[videoId]/_components/ShareHeader.tsxapps/web/components/SignedImageUrl.tsxapps/web/app/s/[videoId]/_components/CommentStamp.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/page.tsxapps/web/app/(org)/dashboard/caps/components/SettingsDialog.tsxapps/web/components/FileInput.tsxapps/web/app/(org)/dashboard/caps/components/UploadCapButton.tsxapps/web/app/Layout/AuthContext.tsxapps/web/app/(org)/dashboard/Contexts.tsxapps/web/app/(org)/dashboard/settings/account/page.tsxapps/web/app/(org)/dashboard/dashboard-data.ts
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for all client-side server state and fetching in the web app
Mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData
Run server-side effects via the ManagedRuntime from apps/web/lib/server.ts using EffectRuntime.runPromise/runPromiseExit; do not create runtimes ad hoc
Client code should use helpers from apps/web/lib/EffectRuntime.ts (useEffectQuery, useEffectMutation, useRpcClient); never call ManagedRuntime.make inside components
Files:
apps/web/app/(org)/dashboard/_components/Navbar/Top.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/OrganizationIndicator.tsxapps/web/app/(org)/dashboard/caps/components/SharingDialog.tsxapps/web/app/(org)/dashboard/settings/organization/components/OrganizationIcon.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/MemberSelect.tsxapps/web/app/(org)/dashboard/settings/organization/components/CapSettingsCard.tsxapps/web/app/(org)/dashboard/settings/account/Settings.tsxapps/web/app/(org)/dashboard/layout.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersDialog.tsxapps/web/app/s/[videoId]/page.tsxapps/web/app/(org)/dashboard/settings/account/components/ProfileImage.tsxapps/web/app/layout.tsxapps/web/app/(org)/dashboard/_components/Navbar/SpaceDialog.tsxapps/web/app/s/[videoId]/_components/ShareHeader.tsxapps/web/components/SignedImageUrl.tsxapps/web/app/s/[videoId]/_components/CommentStamp.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/page.tsxapps/web/app/(org)/dashboard/caps/components/SettingsDialog.tsxapps/web/components/FileInput.tsxapps/web/app/(org)/dashboard/caps/components/UploadCapButton.tsxapps/web/app/Layout/AuthContext.tsxapps/web/app/(org)/dashboard/Contexts.tsxapps/web/app/(org)/dashboard/settings/account/page.tsxapps/web/app/(org)/dashboard/dashboard-data.ts
apps/web/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Server components needing Effect services must call EffectRuntime.runPromise(effect.pipe(provideOptionalAuth))
Files:
apps/web/app/(org)/dashboard/_components/Navbar/Top.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/OrganizationIndicator.tsxapps/web/app/(org)/dashboard/caps/components/SharingDialog.tsxapps/web/app/(org)/dashboard/settings/organization/components/OrganizationIcon.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/MemberSelect.tsxapps/web/app/(org)/dashboard/settings/organization/components/CapSettingsCard.tsxapps/web/app/(org)/dashboard/settings/account/Settings.tsxapps/web/app/(org)/dashboard/layout.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersDialog.tsxapps/web/app/s/[videoId]/page.tsxapps/web/app/(org)/dashboard/settings/account/components/ProfileImage.tsxapps/web/app/layout.tsxapps/web/app/(org)/dashboard/_components/Navbar/SpaceDialog.tsxapps/web/app/s/[videoId]/_components/ShareHeader.tsxapps/web/app/s/[videoId]/_components/CommentStamp.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/page.tsxapps/web/app/(org)/dashboard/caps/components/SettingsDialog.tsxapps/web/app/(org)/dashboard/caps/components/UploadCapButton.tsxapps/web/app/Layout/AuthContext.tsxapps/web/app/(org)/dashboard/Contexts.tsxapps/web/app/(org)/dashboard/settings/account/page.tsxapps/web/app/(org)/dashboard/dashboard-data.ts
🧠 Learnings (1)
📚 Learning: 2025-10-14T10:15:44.019Z
Learnt from: CR
PR: CapSoftware/Cap#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-14T10:15:44.019Z
Learning: Applies to apps/web/**/*.{tsx} : Styling uses Tailwind CSS only; use Next/Image for remote assets; prefer memoization and code-splitting for performance
Applied to files:
apps/web/app/layout.tsx
🧬 Code graph analysis (11)
apps/web/app/(org)/dashboard/settings/organization/components/OrganizationIcon.tsx (2)
apps/web/lib/EffectRuntime.ts (2)
useRpcClient(25-25)useEffectMutation(23-23)packages/web-domain/src/Organisation.ts (3)
Organisation(21-24)OrganisationId(16-18)OrganisationId(19-19)
apps/web/app/(org)/dashboard/settings/account/Settings.tsx (2)
apps/web/app/(org)/dashboard/Contexts.tsx (1)
useDashboardContext(51-51)apps/web/lib/EffectRuntime.ts (2)
useRpcClient(25-25)useEffectMutation(23-23)
apps/web/app/s/[videoId]/page.tsx (5)
packages/database/index.ts (1)
db(18-25)packages/web-backend/src/Database.ts (1)
Database(7-17)packages/web-backend/src/ImageUploads/index.ts (1)
ImageUploads(7-67)packages/database/schema.ts (2)
comments(375-396)users(60-119)apps/web/lib/EffectRuntime.ts (1)
EffectRuntime(20-20)
apps/web/app/layout.tsx (5)
packages/web-backend/src/ImageUploads/index.ts (1)
ImageUploads(7-67)apps/web/utils/getBootstrapData.ts (1)
getBootstrapData(17-56)packages/database/auth/session.ts (1)
getCurrentUser(15-28)packages/utils/src/constants/plans.ts (1)
userIsPro(14-38)apps/web/lib/server.ts (1)
runPromise(130-142)
apps/web/app/s/[videoId]/_components/ShareHeader.tsx (2)
packages/web-domain/src/ImageUpload.ts (2)
ImageUrl(6-6)ImageUrl(7-7)apps/web/app/Layout/AuthContext.tsx (1)
useCurrentUser(40-42)
apps/web/components/SignedImageUrl.tsx (1)
packages/ui/src/components/Avatar.tsx (1)
Avatar(73-109)
apps/web/app/(org)/dashboard/spaces/[spaceId]/page.tsx (6)
packages/web-domain/src/Space.ts (2)
SpaceIdOrOrganisationId(7-7)SpaceIdOrOrganisationId(8-8)packages/web-backend/src/Database.ts (1)
Database(7-17)packages/web-backend/src/index.ts (1)
ImageUploads(6-6)packages/database/schema.ts (1)
users(60-119)packages/web-backend/src/Auth.ts (1)
makeCurrentUserLayer(48-50)apps/web/lib/server.ts (1)
runPromise(130-142)
apps/web/app/Layout/AuthContext.tsx (4)
packages/web-domain/src/Authentication.ts (1)
CurrentUser(8-16)packages/web-domain/src/User.ts (2)
UserId(10-10)UserId(11-11)packages/web-domain/src/ImageUpload.ts (2)
ImageUrl(6-6)ImageUrl(7-7)packages/web-domain/src/Organisation.ts (3)
Organisation(21-24)OrganisationId(16-18)OrganisationId(19-19)
apps/web/app/(org)/dashboard/Contexts.tsx (1)
apps/web/app/Layout/AuthContext.tsx (2)
CurrentUser(6-14)useCurrentUser(40-42)
apps/web/app/(org)/dashboard/settings/account/page.tsx (1)
apps/web/app/(org)/dashboard/settings/account/Settings.tsx (1)
Settings(25-271)
apps/web/app/(org)/dashboard/dashboard-data.ts (5)
packages/database/schema.ts (6)
organizations(172-206)organizationMembers(209-231)users(60-119)spaces(591-619)spaceMembers(621-646)organizationInvites(233-258)packages/web-domain/src/ImageUpload.ts (2)
ImageUrl(6-6)ImageUrl(7-7)packages/database/index.ts (1)
db(18-25)packages/web-backend/src/ImageUploads/index.ts (1)
ImageUploads(7-67)apps/web/lib/server.ts (1)
runPromise(130-142)
🪛 ast-grep (0.39.6)
apps/web/app/layout.tsx
[warning] 134-134: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (rust)
🔇 Additional comments (22)
apps/web/app/(org)/dashboard/caps/components/SettingsDialog.tsx (1)
194-194: LGTM! Simplified pro-status check.The direct access to
user.isProsimplifies the code by removing an unnecessary intermediary constant. The logic remains functionally correct for disabling pro features.apps/web/app/(org)/dashboard/caps/components/UploadCapButton.tsx (1)
41-41: LGTM! Simplified pro status check.The change from
userIsPro(user)to direct property accessuser.isProis cleaner and removes unnecessary indirection while maintaining the same behavior.apps/web/app/(org)/dashboard/_components/Navbar/SpaceDialog.tsx (1)
17-17: Type migration verified—FileInput component properly supportsImageUpload.ImageUrltypes.The
FileInputcomponent'sinitialPreviewUrlprop acceptsImageUpload.ImageUrl | null, confirming compatibility with your changes at lines 17, 40, and 121. The type migration is correct and requires no adjustments.apps/web/app/(org)/dashboard/settings/organization/components/CapSettingsCard.tsx (1)
180-180: Verified:user.isProproperty exists and is type-safe.The refactor from
userIsPro(user)touser.isProis valid. TheuserfromuseDashboardContext()has typeCurrentUser, which includes theisPro: booleanproperty defined inapps/web/app/Layout/AuthContext.tsx. The change maintains type safety and the disabled logic remains correct.apps/web/app/(org)/dashboard/spaces/[spaceId]/components/MemberSelect.tsx (3)
24-25: Type alignment for user image looks goodUserObject.image moved to ImageUpload.ImageUrl | null; matches signed URL flow.
35-36: TagOption.image typing is correctOptional ImageUpload.ImageUrl keeps option objects lightweight and consistent with SignedImageUrl.
199-205: Review comment is based on incorrect codebase understandingThe
typeprop never existed in theSignedImageUrlcomponent interface. The component only accepts:image,name,className, andletterClass. Thetypeparameter exists only in the backend RPC layer (GetSignedImageUrlinpackages/web-backend/src/Users/UsersRpcs.ts), which is a different abstraction layer from the React component.All 30+ usages across the codebase consistently follow the same pattern—passing no
typeprop toSignedImageUrl. The MemberSelect.tsx usage at lines 199-205 and 222-227 matches every other call site and requires no changes.Likely an incorrect or invalid review comment.
apps/web/app/(org)/dashboard/settings/account/page.tsx (1)
8-10: LGTMSettings now sources user via context; page is clean and minimal.
apps/web/app/(org)/dashboard/layout.tsx (2)
27-28: Auth guard is correctRedirecting unauthenticated users early is good.
75-101: Verification complete: user prop removal confirmedThe DashboardContexts component correctly no longer expects a
userprop. The function destructures exactly the props passed by layout.tsx, and internally sources the user viauseCurrentUser()at line 76. The user is then provided to the context value for consumers, maintaining access throughuseDashboardContext().apps/web/app/(org)/dashboard/_components/Navbar/Top.tsx (2)
104-109: LGTM!The SignedImageUrl usage for the active space icon is correctly updated to use the new ImageUpload pattern without the type prop.
263-268: LGTM!The user image handling correctly uses
user.imageUrlwith the updated SignedImageUrl component.apps/web/app/s/[videoId]/_components/ShareHeader.tsx (2)
56-56: LGTM!The introduction of
useCurrentUser()hook correctly centralizes user state management, removing the need for prop drilling.
35-35: LGTM!The ownerImage type update to
ImageUpload.ImageUrl | nulland its usage with SignedImageUrl are consistent with the refactored image handling pattern.Also applies to: 241-246
apps/web/components/SignedImageUrl.tsx (1)
1-25: LGTM! Excellent simplification.The component has been successfully simplified by removing RPC logic and S3 key handling. The image resolution is now handled upstream, and the component serves as a clean wrapper around Avatar. This aligns with the PR's goal of consolidating image handling into a centralized service.
apps/web/app/(org)/dashboard/spaces/[spaceId]/components/OrganizationIndicator.tsx (1)
24-24: LGTM!The type update for the image field to
ImageUpload.ImageUrl | nulland its straightforward usage with SignedImageUrl are consistent with the refactor pattern.Also applies to: 76-81
apps/web/app/(org)/dashboard/settings/account/components/ProfileImage.tsx (2)
58-58: LGTM!Wrapping the object URL with
ImageUpload.ImageUrl.make()correctly brands it for use with the typed image handling system.
54-59: No action required—type compatibility is correct.The branded type
ImageUrlcreated viaSchema.String.pipe(Schema.brand("ImageUrl"))is fully compatible with string parameters. This is confirmed by the codebase pattern atpackages/web-domain/src/ImageUpload.ts:20, where the identical branded type is passed directly to theURLconstructor without type assertions. TypeScript strict mode is enabled and the code compiles without errors.apps/web/app/s/[videoId]/_components/CommentStamp.tsx (1)
13-13: LGTM!The type update for
authorImagetoImageUpload.ImageUrl | nulland its usage with SignedImageUrl are consistent with the refactored image handling pattern.Also applies to: 67-72
apps/web/components/FileInput.tsx (1)
30-36: LGTM: Branded ImageUrl + blob preview and cleanup are correct.State typing, revokeObjectURL guards, and SignedImageUrl usage look sound.
Also applies to: 55-58, 170-176, 247-253
apps/web/app/(org)/dashboard/settings/account/Settings.tsx (1)
90-119: LGTM: RPC-based upload/remove flow is correct and aligned with EffectRuntime.Good Option.some/none usage, UI feedback, and router.refresh handling.
Also applies to: 120-137
apps/web/app/Layout/AuthContext.tsx (1)
16-18: Review comment verified and resolved.React 19.1.1 is installed, which fully supports the use() hook in Client Components. The AuthContext.tsx implementation correctly uses this pattern: the Promise is passed through props to the provider, the context stores the Promise, and both useAuthContext() and useCurrentUser() properly call use() to unwrap the context and Promise respectively. This is the intended pattern for handling server-side Promises in Next.js App Router Client Components with React 19+.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/app/s/[videoId]/_components/Sidebar.tsx (1)
185-185: Migrate Transcript to useuseCurrentUser()hook for consistency with Activity and Summary components.The Transcript component currently accepts and uses the
userprop to determine edit permissions (canEdit = user?.id === data.ownerIdat line 382), which is then used to conditionally render edit buttons at lines 462 and 563. However, Activity and Summary components have already migrated away from accepting the user prop and instead use theuseCurrentUser()hook internally from@/app/Layout/AuthContext. Transcript should follow the same pattern by usinguseCurrentUser()directly rather than relying on prop injection.packages/web-backend/src/Users/UsersRpcs.ts (1)
39-44: Map S3 errors in onboarding RPC to InternalError to satisfy the RPC contract.If onboarding emits S3Error, it will leak past the declared error schema (InternalError). Catch and map it like the update path does.
Apply:
- }).pipe( - Effect.catchTag( - "DatabaseError", - () => new InternalError({ type: "database" }), - ), - ), + }).pipe( + Effect.catchTags({ + DatabaseError: () => new InternalError({ type: "database" }), + S3Error: () => new InternalError({ type: "s3" }), + }), + ),
♻️ Duplicate comments (4)
apps/web/app/(org)/dashboard/dashboard-data.ts (2)
131-187: Duplicate space rows due to joins; remove unused memberImage selection.The query selects
memberImage(line 147) and joinsspaceMembersandusers(lines 156-157), which can produce multiple rows per space when a space has multiple members. The wideselectDistinctcannot dedupe these rows becausememberImagevalues differ across rows. SincememberImageis not used in the returned object (line 178 only mapsiconUrl), remove the unnecessary join and column.Apply this diff to fix:
.selectDistinct({ id: spaces.id, primary: spaces.primary, privacy: spaces.privacy, name: spaces.name, description: spaces.description, organizationId: spaces.organizationId, createdById: spaces.createdById, iconUrl: spaces.iconUrl, - memberImage: users.image, memberCount: sql<number>`( SELECT COUNT(*) FROM space_members WHERE space_members.spaceId = spaces.id )`, videoCount: sql<number>`( SELECT COUNT(*) FROM space_videos WHERE space_videos.spaceId = spaces.id )`, }) .from(spaces) - .leftJoin(spaceMembers, eq(spaces.id, spaceMembers.spaceId)) - .leftJoin(users, eq(spaceMembers.userId, users.id)) .where( and( eq(spaces.organizationId, activeOrganizationId), or( - // User is the space creator eq(spaces.createdById, user.id), - // User is a member of the space - eq(spaceMembers.userId, user.id), - // Space is public within the organization + sql`EXISTS (SELECT 1 FROM space_members WHERE space_members.spaceId = spaces.id AND space_members.userId = ${user.id})`, eq(spaces.privacy, "Public"), ), ), ),
307-326: totalInvites double-counts due to cross-joined aggregates; use scalar subqueries.The query joins
organizationMembersandorganizationInvites(lines 317-324), then counts both in a single SQL expression (lines 310-314). When an organization has both members and invites, the LEFT JOINs create a cross product, causing row multiplication and inflated counts. Use separate scalar subqueries instead.Apply this diff:
- const totalInvitesResult = yield* db.use((db) => - db - .select({ - value: sql<number>` - ${count(organizationMembers.id)} + ${count( - organizationInvites.id, - )} - `, - }) - .from(organizations) - .leftJoin( - organizationMembers, - eq(organizations.id, organizationMembers.organizationId), - ) - .leftJoin( - organizationInvites, - eq(organizations.id, organizationInvites.organizationId), - ) - .where(eq(organizations.ownerId, organization.ownerId)), - ); + const totalInvitesResult = yield* db.use((db) => + db + .select({ + value: sql<number>` + (SELECT COUNT(*) FROM organization_members + WHERE organization_members.organizationId = ${organization.id}) + + + (SELECT COUNT(*) FROM organization_invites + WHERE organization_invites.organizationId = ${organization.id}) + `, + }) + .from(organizations) + .where(eq(organizations.id, organization.id)), + );packages/web-domain/src/User.ts (2)
17-18: FixUserUpdatetype alias to match project pattern.Use
typeof X.Type, notSchema.Schema.Type<...>.Apply:
-export type UserUpdate = Schema.Schema.Type<typeof UserUpdate>; +export type UserUpdate = typeof UserUpdate.Type;
85-88: Addsuccessschema toRpc.make("UserUpdate").Required by @effect/rpc; without it, typegen/contracts break.
Apply:
- Rpc.make("UserUpdate", { - payload: UserUpdate, - error: Schema.Union(InternalError, PolicyDeniedError), - }).middleware(RpcAuthMiddleware), + Rpc.make("UserUpdate", { + payload: UserUpdate, + success: Schema.Void, + error: Schema.Union(InternalError, PolicyDeniedError), + }).middleware(RpcAuthMiddleware),
🧹 Nitpick comments (2)
apps/web/app/s/[videoId]/page.tsx (1)
642-707: Effect pipeline is correct; consider simplifying for efficiency.The Effect-based comments retrieval correctly resolves image URLs using the ImageUploads service. However, the nested structure at lines 692-705 creates intermediate Effect wrappers that could be simplified.
Consider this more direct pattern:
- .pipe( - Effect.map((comments) => - comments.map( - Effect.fn(function* (c) { - return Object.assign(c, { - authorImage: yield* Option.fromNullable(c.authorImage).pipe( - Option.map(imageUploads.resolveImageUrl), - Effect.transposeOption, - Effect.map(Option.getOrNull), - ), - }); - }), - ), - ), - Effect.flatMap(Effect.all), - ); + .pipe( + Effect.flatMap((comments) => + Effect.all(comments.map((c) => + Option.fromNullable(c.authorImage).pipe( + Option.map(imageUploads.resolveImageUrl), + Effect.transposeOption, + Effect.map(Option.getOrNull), + Effect.map((authorImage) => Object.assign(c, { authorImage })), + ) + )) + ) + );This reduces the number of Effect allocations and simplifies the pipeline by using
flatMapdirectly instead ofmap+flatMap(Effect.all).packages/web-domain/src/User.ts (1)
13-16: Clarify purpose ofidin UserUpdate.Backend Users.update ignores payload.id and operates on CurrentUser. Either:
- enforce
payload.id === currentUser.idand throw PolicyDeniedError otherwise, or- drop
idfrom the schema to avoid confusion.Reference: packages/web-backend/src/Users/index.ts shows update using CurrentUser only. (Based on relevant snippet)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/web/app/(org)/dashboard/dashboard-data.ts(6 hunks)apps/web/app/s/[videoId]/Share.tsx(2 hunks)apps/web/app/s/[videoId]/_components/CapVideoPlayer.tsx(0 hunks)apps/web/app/s/[videoId]/_components/ShareHeader.tsx(4 hunks)apps/web/app/s/[videoId]/_components/Sidebar.tsx(4 hunks)apps/web/app/s/[videoId]/page.tsx(3 hunks)packages/web-backend/src/Users/UsersRpcs.ts(2 hunks)packages/web-domain/src/User.ts(2 hunks)
💤 Files with no reviewable changes (1)
- apps/web/app/s/[videoId]/_components/CapVideoPlayer.tsx
✅ Files skipped from review due to trivial changes (1)
- apps/web/app/s/[videoId]/_components/ShareHeader.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.Use strict TypeScript and avoid any; leverage shared types
Files:
apps/web/app/s/[videoId]/_components/Sidebar.tsxapps/web/app/s/[videoId]/Share.tsxpackages/web-domain/src/User.tsapps/web/app/s/[videoId]/page.tsxpackages/web-backend/src/Users/UsersRpcs.tsapps/web/app/(org)/dashboard/dashboard-data.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/web/app/s/[videoId]/_components/Sidebar.tsxapps/web/app/s/[videoId]/Share.tsxpackages/web-domain/src/User.tsapps/web/app/s/[videoId]/page.tsxpackages/web-backend/src/Users/UsersRpcs.tsapps/web/app/(org)/dashboard/dashboard-data.ts
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/app/s/[videoId]/_components/Sidebar.tsxapps/web/app/s/[videoId]/Share.tsxapps/web/app/s/[videoId]/page.tsxapps/web/app/(org)/dashboard/dashboard-data.ts
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for all client-side server state and fetching in the web app
Mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData
Run server-side effects via the ManagedRuntime from apps/web/lib/server.ts using EffectRuntime.runPromise/runPromiseExit; do not create runtimes ad hoc
Client code should use helpers from apps/web/lib/EffectRuntime.ts (useEffectQuery, useEffectMutation, useRpcClient); never call ManagedRuntime.make inside components
Files:
apps/web/app/s/[videoId]/_components/Sidebar.tsxapps/web/app/s/[videoId]/Share.tsxapps/web/app/s/[videoId]/page.tsxapps/web/app/(org)/dashboard/dashboard-data.ts
apps/web/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Server components needing Effect services must call EffectRuntime.runPromise(effect.pipe(provideOptionalAuth))
Files:
apps/web/app/s/[videoId]/_components/Sidebar.tsxapps/web/app/s/[videoId]/Share.tsxapps/web/app/s/[videoId]/page.tsxapps/web/app/(org)/dashboard/dashboard-data.ts
🧬 Code graph analysis (6)
apps/web/app/s/[videoId]/_components/Sidebar.tsx (3)
packages/web-domain/src/ImageUpload.ts (2)
ImageUrl(6-6)ImageUrl(7-7)apps/web/app/Layout/AuthContext.tsx (1)
useCurrentUser(40-42)apps/web/app/s/[videoId]/_components/tabs/Activity/index.tsx (1)
Activity(26-109)
apps/web/app/s/[videoId]/Share.tsx (1)
packages/web-domain/src/ImageUpload.ts (2)
ImageUrl(6-6)ImageUrl(7-7)
packages/web-domain/src/User.ts (1)
packages/web-domain/src/ImageUpload.ts (2)
ImageUpdatePayload(38-44)ImageUpdatePayload(45-45)
apps/web/app/s/[videoId]/page.tsx (5)
packages/database/index.ts (1)
db(18-25)packages/web-backend/src/Database.ts (1)
Database(7-17)packages/web-backend/src/ImageUploads/index.ts (1)
ImageUploads(7-67)packages/database/schema.ts (2)
comments(375-396)users(60-119)apps/web/lib/EffectRuntime.ts (1)
EffectRuntime(20-20)
packages/web-backend/src/Users/UsersRpcs.ts (1)
packages/web-backend/src/Users/index.ts (1)
Users(8-34)
apps/web/app/(org)/dashboard/dashboard-data.ts (5)
packages/database/schema.ts (6)
organizations(172-206)organizationMembers(209-231)users(60-119)spaces(591-619)spaceMembers(621-646)organizationInvites(233-258)packages/web-domain/src/ImageUpload.ts (2)
ImageUrl(6-6)ImageUrl(7-7)packages/database/index.ts (1)
db(18-25)packages/web-backend/src/ImageUploads/index.ts (1)
ImageUploads(7-67)apps/web/lib/server.ts (1)
runPromise(130-142)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (4)
apps/web/app/s/[videoId]/Share.tsx (1)
5-5: LGTM! Consistent type migration to ImageUpload.ImageUrl.The type changes from
string | nulltoImageUpload.ImageUrl | nullfor author images align with the broader refactor to use branded image types and centralized image URL resolution.Also applies to: 28-28, 33-33
apps/web/app/s/[videoId]/_components/Sidebar.tsx (1)
92-98: LGTM! Correct migration to useCurrentUser().The component now correctly retrieves the current user via
useCurrentUser()and safely computesisOwnerOrMemberusing optional chaining. This aligns with the broader refactor to centralize user access via hooks rather than prop drilling.packages/web-backend/src/Users/UsersRpcs.ts (2)
11-12: Unified update flow + error mapping looks good.Using Users.update and mapping Database/S3 to InternalError aligns with the new ImageUploads path and the domain error union.
Please confirm Users.update returns void to match a Void success schema in the domain RPC.
Also applies to: 45-51
54-54: The review comment is incorrect—Users.Default is properly wired upstream.UsersRpcsLive (line 8 of UsersRpcs.ts) consumes
Usersat line 12 but intentionally does not provide it locally. Instead,Users.Defaultis supplied at the application level through theDependencieslayer (apps/web/lib/server.ts, line 111), which is merged into the composition viaLayer.provideMergewhenRpcsLiveis used in erpc/route.ts. The dependency chain is complete and correct.Additionally, the line number cited in the review comment (54) does not match the actual code location (line 71).
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/web-backend/src/S3Buckets/index.ts (1)
171-181: LGTM: Parameter type simplification improves API clarity.The change from nested
Option<Option<S3Bucket.S3BucketId>>to optionalbucketId?: Option.Option<S3Bucket.S3BucketId>with nullish coalescing:
- Simplifies the function signature
- Makes the API easier to use
- Maintains the same logical behavior
- Properly handles the absence of bucketId via
Option.none()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/web-backend/src/S3Buckets/index.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.Use strict TypeScript and avoid any; leverage shared types
Files:
packages/web-backend/src/S3Buckets/index.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
packages/web-backend/src/S3Buckets/index.ts
🧠 Learnings (1)
📚 Learning: 2025-09-24T07:24:21.449Z
Learnt from: Brendonovich
PR: CapSoftware/Cap#1060
File: apps/web/components/forms/server.ts:85-97
Timestamp: 2025-09-24T07:24:21.449Z
Learning: In the S3BucketAccess service, bucketName is an Effect that must be yielded (using yield*) to get the actual string value before use in string interpolation or other operations.
Applied to files:
packages/web-backend/src/S3Buckets/index.ts
🧬 Code graph analysis (1)
packages/web-backend/src/S3Buckets/index.ts (2)
packages/web-domain/src/S3Bucket.ts (3)
S3Bucket(7-15)S3BucketId(4-4)S3BucketId(5-5)packages/web-backend/src/S3Buckets/S3BucketClientProvider.ts (1)
S3BucketClientProvider(4-14)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (3)
packages/web-backend/src/S3Buckets/index.ts (3)
58-79: LGTM: Correct usage of endpointIsPathStyle with proper fallback.The function correctly:
- Decrypts custom bucket credentials
- Determines path-style from endpoint when present
- Falls back to path-style (
true) when endpoint is absent- Properly handles the bucket name parameter
135-140: LGTM: Proper path-style flag propagation.The
isPathStyleproperty is correctly added to the default provider configuration using the existingdefaultConfigs.forcePathStylevalue.
153-161: Client reuse for custom buckets is intentional and correct—no changes needed.The
S3Buckettype inpackages/web-domain/src/S3Bucket.tsdefines a singleendpointfield, not separate internal/public endpoints. This architectural constraint means custom buckets cannot configure different endpoints like default buckets do. The client reuse at lines 157–158 is therefore by design, where the internal/public distinction is logical (based on operation type inS3BucketAccess), not network-based. Default buckets can distinguish endpoints only because environment config provides separateinternalEndpointandpublicEndpointvalues.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor