Move createFolder action to effect rpc #893
Conversation
WalkthroughThis PR removes legacy server-actions for caps and folders, adds a Folder.create service and FolderCreate RPC, updates frontend dialogs to use RPC-based mutations with Option-wrapped fields, and extends the authenticated user context to include activeOrgId across backend and domain layers. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Dialog (New/Subfolder)
participant RPC as withRpc(FolderCreate)
participant Auth as RpcAuthMiddleware
participant BE as FolderRpcsLive
participant SVC as Folders.create
participant DB as Database
UI->>RPC: mutate({ name, color, spaceId?: Option, parentId?: Option })
RPC->>Auth: authenticate request
Auth->>BE: invoke FolderCreate (with currentUser.activeOrgId)
BE->>SVC: create(data)
SVC->>DB: validate parent in org, insert folder
DB-->>SVC: created row
SVC-->>BE: Folder
BE-->>RPC: Folder
RPC-->>UI: success
UI->>UI: reset form, close, router.refresh(), toast
sequenceDiagram
participant Req as HTTP Request
participant AuthMW as HttpAuthMiddlewareLive
participant Ctx as CurrentUser.context
Req->>AuthMW: incoming request (with user)
AuthMW->>Ctx: set { id, email, activeOrgId }
Ctx-->>AuthMW: context available downstream
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
apps/web/app/(org)/dashboard/caps/components/NewFolderDialog.tsx (1)
150-153: Optional UX: notify when color is not selectedCurrently, clicking “Create” with no color silently no-ops. Consider warning the user (e.g., toast) to clarify why nothing happened.
onClick={() => { - if (selectedColor === null) return; + if (selectedColor === null) { + toast.error("Please select a folder color"); + return; + } createFolder.mutate({ name: folderName, color: selectedColor }); }}packages/web-backend/src/Folders/index.ts (1)
65-70: Optional: consider authorization policy for creationYou already apply policy checks on delete. If creation requires specific permissions (e.g., per-space or org role), consider enforcing
policy.canCreate(...)similarly.apps/web/app/(org)/dashboard/folder/[id]/components/SubfolderDialog.tsx (1)
79-89: Trim folder name before sending to RPC to avoid accidental leading/trailing spacesSmall UX/validation improvement: you already disable the button when the trimmed name is empty; trim the value before sending it to the backend as well.
mutationFn: (data: { name: string; color: FolderColor }) => withRpc((r) => r.FolderCreate({ - name: data.name, + name: data.name.trim(), color: data.color, spaceId: Option.fromNullable(activeSpace?.id), parentId: Option.some(parentFolderId), }), ),onClick={() => { if (selectedColor === null) return; createSubfolder.mutate({ - name: folderName, + name: folderName.trim(), color: selectedColor, }); }}Also applies to: 165-167
packages/web-domain/src/Folder.ts (1)
33-43: Verify PolicyDeniedError for FolderCreate RPCI wasn’t able to find any existing policy‐denial checks around folder creation in the codebase (no throws of
PolicyDeniedError, and the Delete RPC already includes it). Please double-check whether the Folders service enforces authorization on create—if so, callers will need to distinguish a policy denial fromNotFoundError/InternalError.• Manually inspect the backend Folders service’s create handler for any
PolicyDeniedErrorthrows or authorization logic.
• If policy checks exist (or are planned), update the RPC definition inpackages/web-domain/src/Folder.ts:Rpc.make("FolderCreate", { payload: Schema.Struct({ name: Schema.String, color: FolderColor, spaceId: Schema.OptionFromUndefinedOr(Schema.String), parentId: Schema.OptionFromUndefinedOr(FolderId), }), success: Folder, - error: Schema.Union(NotFoundError, InternalError), + error: Schema.Union(NotFoundError, InternalError, PolicyDeniedError), }).middleware(RpcAuthMiddleware),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
apps/web/actions/caps/get-shared-spaces.ts(0 hunks)apps/web/actions/folders/createFolder.ts(0 hunks)apps/web/app/(org)/dashboard/caps/components/NewFolderDialog.tsx(4 hunks)apps/web/app/(org)/dashboard/folder/[id]/components/SubfolderDialog.tsx(4 hunks)packages/web-backend/src/Auth.ts(2 hunks)packages/web-backend/src/Folders/FoldersRpcs.ts(1 hunks)packages/web-backend/src/Folders/index.ts(3 hunks)packages/web-domain/src/Authentication.ts(1 hunks)packages/web-domain/src/Folder.ts(1 hunks)
💤 Files with no reviewable changes (2)
- apps/web/actions/caps/get-shared-spaces.ts
- apps/web/actions/folders/createFolder.ts
🧰 Additional context used
🧬 Code Graph Analysis (5)
packages/web-backend/src/Folders/FoldersRpcs.ts (2)
packages/database/schema.ts (1)
folders(205-229)packages/web-domain/src/Errors.ts (1)
InternalError(3-6)
packages/web-backend/src/Folders/index.ts (4)
packages/web-domain/src/Folder.ts (5)
Folder(18-26)FolderColor(10-10)FolderColor(11-11)FolderId(7-7)FolderId(8-8)packages/web-domain/src/Authentication.ts (1)
CurrentUser(7-10)packages/database/index.ts (1)
db(28-33)packages/database/helpers.ts (1)
nanoId(6-9)
apps/web/app/(org)/dashboard/caps/components/NewFolderDialog.tsx (3)
apps/web/lib/EffectRuntime.ts (1)
useEffectMutation(19-19)packages/web-domain/src/Folder.ts (3)
Folder(18-26)FolderColor(10-10)FolderColor(11-11)apps/web/lib/Rpcs.ts (1)
withRpc(16-17)
apps/web/app/(org)/dashboard/folder/[id]/components/SubfolderDialog.tsx (3)
packages/web-domain/src/Folder.ts (5)
Folder(18-26)FolderId(7-7)FolderId(8-8)FolderColor(10-10)FolderColor(11-11)apps/web/lib/EffectRuntime.ts (1)
useEffectMutation(19-19)apps/web/lib/Rpcs.ts (1)
withRpc(16-17)
packages/web-domain/src/Folder.ts (5)
packages/web-domain/src/Video.ts (1)
NotFoundError(75-78)apps/web/lib/Rpcs.ts (1)
Rpc(11-14)packages/web-domain/src/Errors.ts (1)
InternalError(3-6)packages/web-domain/src/Policy.ts (1)
PolicyDeniedError(18-21)packages/web-domain/src/Authentication.ts (1)
RpcAuthMiddleware(28-34)
⏰ 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). (4)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (10)
packages/web-domain/src/Authentication.ts (1)
7-10: ConfirmactiveOrgIdis guaranteed non-null in all auth contextsExpanding
CurrentUserto includeactiveOrgId: stringlooks aligned with the rest of the PR. Please verify thatactiveOrganizationIdin the DB/session is always present for authenticated users; if it can be null/undefined (e.g., users not yet joined to any org), callers will receive an invalidCurrentUsercontext. If nullability is possible, consider modeling this asOption<string>or guarding at the middleware level.packages/web-backend/src/Folders/FoldersRpcs.ts (1)
20-25: LGTM:FolderCreateRPC mirrors existing error-handling patternThe implementation correctly delegates to
folders.createand mapsDatabaseErrortoInternalError({ type: "database" }), consistent withFolderDelete.packages/web-backend/src/Auth.ts (2)
48-52: Guard against nullableactiveOrganizationIdbefore populatingCurrentUserIf
user.activeOrganizationIdcan be null/undefined, this will violate theCurrentUsercontract (activeOrgId: string) and may leak an invalid context into request handling. Consider:
- Rejecting with
UnauthorizedifactiveOrganizationIdis missing, or- Modeling
activeOrgIdas optional/Option at the domain level.Please confirm DB/session guarantees here.
74-78: Mirror the same nullability guard inprovideOptionalAuthThe optional auth path should enforce the same invariant for
activeOrgId. Today it passes through potentially nullish values intoCurrentUser.context.packages/web-backend/src/Folders/index.ts (1)
91-92: VerifyFolder.FolderId.makeexists (or switch to a supported constructor)If
Folder.FolderIdis a brandedSchema.String, a.makehelper may not exist. Consider switching to a supported constructor, e.g., a brand assertion or a schema decode.Example alternatives:
- Using a brand assertion:
id: nanoId() as Folder.FolderId- Or using schema decode (if available in your helpers):
id: Schema.decodeSync(Folder.FolderId)(nanoId())apps/web/app/(org)/dashboard/folder/[id]/components/SubfolderDialog.tsx (2)
70-71: LGTM: Router refresh after successUsing useRouter().refresh() after a successful mutation is consistent with Next.js app router patterns to revalidate server components.
170-176: LGTM: Loading and disabled states are wired correctlySpinner and disabled conditions correctly depend on isPending and basic input validation.
packages/web-domain/src/Folder.ts (3)
10-12: LGTM: Public FolderColor type looks goodExplicit literal union for folder colors is clear and enforces valid values across the stack.
13-17: LGTM: Specific NotFoundErrorDedicated FolderNotFoundError aligns with existing patterns (e.g., VideoNotFoundError) and improves error specificity.
18-26: LGTM: Folder model with Option fieldsUsing OptionFromNullOr for spaceId and parentId is appropriate for persistence layers that store null while exposing Option to callers.
| DialogTitle, | ||
| Input, | ||
| } from "@cap/ui"; | ||
| import type { Folder } from "@cap/web-domain"; |
There was a problem hiding this comment.
Fix type import: Folder.FolderColor is not a named type import
Using Folder.FolderColor with import type { Folder } is likely a type error. Import the color type directly and use it in the mutation payload.
Apply this diff:
-import type { Folder } from "@cap/web-domain";
+import type { FolderColor } from "@cap/web-domain";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import type { Folder } from "@cap/web-domain"; | |
| import type { FolderColor } from "@cap/web-domain"; |
🤖 Prompt for AI Agents
In apps/web/app/(org)/dashboard/caps/components/NewFolderDialog.tsx around line
12, the code imports only "Folder" and then uses "Folder.FolderColor" which is
not a valid named type import; update the import to also pull the color type
directly (e.g. import type { Folder, FolderColor } from "@cap/web-domain") and
change any usage of Folder.FolderColor in the mutation payload and related type
annotations to use FolderColor instead.
| mutationFn: (data: { name: string; color: Folder.FolderColor }) => | ||
| withRpc((r) => | ||
| r.FolderCreate({ | ||
| name: data.name, | ||
| color: data.color, | ||
| spaceId: Option.fromNullable(spaceId), | ||
| parentId: Option.none(), | ||
| }), | ||
| ), |
There was a problem hiding this comment.
Align the mutation payload type with the corrected import
Update the payload type to reference FolderColor (imported directly) instead of Folder.FolderColor.
Apply this diff:
- mutationFn: (data: { name: string; color: Folder.FolderColor }) =>
+ mutationFn: (data: { name: string; color: FolderColor }) =>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| mutationFn: (data: { name: string; color: Folder.FolderColor }) => | |
| withRpc((r) => | |
| r.FolderCreate({ | |
| name: data.name, | |
| color: data.color, | |
| spaceId: Option.fromNullable(spaceId), | |
| parentId: Option.none(), | |
| }), | |
| ), | |
| mutationFn: (data: { name: string; color: FolderColor }) => | |
| withRpc((r) => | |
| r.FolderCreate({ | |
| name: data.name, | |
| color: data.color, | |
| spaceId: Option.fromNullable(spaceId), | |
| parentId: Option.none(), | |
| }), | |
| ), |
🤖 Prompt for AI Agents
In apps/web/app/(org)/dashboard/caps/components/NewFolderDialog.tsx around lines
68 to 76, the mutation payload type currently uses Folder.FolderColor but the
module import provides FolderColor directly; update the type annotation to use
FolderColor instead of Folder.FolderColor (i.e., change (data: { name: string;
color: Folder.FolderColor }) to (data: { name: string; color: FolderColor }) ),
and ensure the FolderColor import is present at the top of the file.
| import { Folder } from "@cap/web-domain"; | ||
| import { Option } from "effect"; | ||
| import { withRpc } from "@/lib/Rpcs"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix incorrect type references: use top-level FolderId/FolderColor instead of Folder.FolderId/Folder.FolderColor
The domain exports FolderId and FolderColor as top-level types (see packages/web-domain/src/Folder.ts). Referencing them via Folder.* will fail. Prefer type-only imports to avoid bundling.
-import { Folder } from "@cap/web-domain";
+import type { FolderId, FolderColor } from "@cap/web-domain";-interface Props {
+interface Props {
open: boolean;
onOpenChange: (open: boolean) => void;
- parentFolderId: Folder.FolderId;
+ parentFolderId: FolderId;
}- mutationFn: (data: { name: string; color: Folder.FolderColor }) =>
+ mutationFn: (data: { name: string; color: FolderColor }) =>Also applies to: 33-33, 80-80
🤖 Prompt for AI Agents
In apps/web/app/(org)/dashboard/folder/[id]/components/SubfolderDialog.tsx
around lines 25-27 (and also affecting lines 33 and 80), the code incorrectly
references Folder.FolderId and Folder.FolderColor; import and use the top-level
types instead by adding type-only imports like: import type { FolderId,
FolderColor } from "@cap/web-domain"; then replace any Folder.FolderId and
Folder.FolderColor usages with FolderId and FolderColor respectively; use type
imports to avoid bundling runtime code.
| if (Option.isSome(data.parentId)) { | ||
| const parentId = data.parentId.value; | ||
| const [parentFolder] = yield* db.execute((db) => | ||
| db | ||
| .select() | ||
| .from(Db.folders) | ||
| .where( | ||
| Dz.and( | ||
| Dz.eq(Db.folders.id, parentId), | ||
| Dz.eq(Db.folders.organizationId, user.activeOrgId), | ||
| ), | ||
| ), | ||
| ); | ||
|
|
||
| if (!parentFolder) return yield* new Folder.NotFoundError(); | ||
| } | ||
|
|
There was a problem hiding this comment.
Enforce parent/space consistency to preserve invariants
If a parentId is provided, the new folder must inherit the parent’s spaceId. Allowing a mismatch (or keeping the caller-provided spaceId) can create invalid trees (e.g., a child in root with a parent inside a space). Override the incoming spaceId with the parent’s spaceId when parentId is Some.
Apply this diff:
- const user = yield* CurrentUser;
+ const user = yield* CurrentUser;
+ // Use parent’s spaceId if creating a subfolder
+ let effectiveSpaceId = data.spaceId;
if (Option.isSome(data.parentId)) {
const parentId = data.parentId.value;
const [parentFolder] = yield* db.execute((db) =>
db
.select()
.from(Db.folders)
.where(
Dz.and(
Dz.eq(Db.folders.id, parentId),
Dz.eq(Db.folders.organizationId, user.activeOrgId),
),
),
);
if (!parentFolder) return yield* new Folder.NotFoundError();
+ // Inherit space from parent to avoid space/root mismatches
+ effectiveSpaceId = Option.fromNullable(parentFolder.spaceId);
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/web-backend/src/Folders/index.ts around lines 73 to 89, when a
parentId is provided the code checks the parent exists but does not enforce that
the new folder inherits the parent's spaceId; update the logic so after fetching
parentFolder (and verifying it exists) you override the incoming data.spaceId
with parentFolder.spaceId (i.e., set data.spaceId = parentFolder.spaceId or
construct the new folder payload using parentFolder.spaceId) to ensure child
folders always inherit their parent’s space and preserve tree invariants.
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 (1)
apps/web/app/(org)/dashboard/caps/components/NewFolderDialog.tsx (1)
106-139: Do not call Hooks inside loops; replace per-item useRef with a callback refuseRef is a Hook and cannot be called inside map. This violates Rules of Hooks and can break rendering.
Apply this diff:
- const folderRef = useRef<any>(null); - folderRefs.current[option.value] = folderRef; + // Store element refs in a map via callback refs (no hooks in loops) ... - onMouseEnter={() => { - const folderRef = folderRefs.current[option.value]?.current; - if (!folderRef) return; - folderRef.stop(); - folderRef.play("folder-open"); - }} + onMouseEnter={() => { + const refEl = folderRefs.current[option.value]; + if (!refEl) return; + refEl.stop(); + refEl.play("folder-open"); + }} onMouseLeave={() => { - const folderRef = folderRefs.current[option.value]?.current; - if (!folderRef) return; - folderRef.stop(); - folderRef.play("folder-close"); + const refEl = folderRefs.current[option.value]; + if (!refEl) return; + refEl.stop(); + refEl.play("folder-close"); }} > - {React.cloneElement(option.component, { ref: folderRef })} + {React.cloneElement(option.component, { + ref: (el: any) => { + folderRefs.current[option.value] = el; + }, + })}If the folder components don’t forward refs to the underlying animation API, we can swap to an explicit onMouseEnter/Leave prop or a small wrapper that exposes stop/play.
♻️ Duplicate comments (3)
packages/web-backend/src/Folders/index.ts (2)
72-87: Inherit parent’s spaceId when creating subfolders to avoid invalid treesIf parentId is provided, the new folder must inherit the parent’s spaceId. Otherwise, callers can create a child in a different “root” (space vs root), violating invariants and breaking delete/move logic.
Apply this diff to compute and enforce an effectiveSpaceId derived from the parent:
- const user = yield* CurrentUser; + const user = yield* CurrentUser; + // Use parent’s spaceId if creating a subfolder + let effectiveSpaceId = data.spaceId; if (Option.isSome(data.parentId)) { const parentId = data.parentId.value; const [parentFolder] = yield* db.execute((db) => db .select() .from(Db.folders) .where( Dz.and( Dz.eq(Db.folders.id, parentId), Dz.eq(Db.folders.organizationId, user.activeOrgId), ), ), ); if (!parentFolder) return yield* new Folder.NotFoundError(); + // Inherit space from parent to avoid space/root mismatches + effectiveSpaceId = Option.fromNullable(parentFolder.spaceId); }
89-97: Use the computed effectiveSpaceId for persistence and return valueAfter deriving effectiveSpaceId above, ensure both the stored row and returned DTO use it instead of the caller-provided spaceId.
Apply this diff:
const folder = { id: Folder.FolderId.make(nanoId()), name: data.name, color: data.color, organizationId: user.activeOrgId, createdById: user.id, - spaceId: data.spaceId, + spaceId: effectiveSpaceId, parentId: data.parentId, }; yield* db.execute((db) => db.insert(Db.folders).values({ ...folder, - spaceId: Option.getOrNull(folder.spaceId), + spaceId: Option.getOrNull(effectiveSpaceId), parentId: Option.getOrNull(folder.parentId), }), );Also applies to: 99-105
apps/web/app/(org)/dashboard/caps/components/NewFolderDialog.tsx (1)
10-10: Fix type import: use FolderColor instead of Folder.FolderColorFolder.FolderColor isn’t a valid type when importing type-only Folder. Import FolderColor directly.
Apply this diff:
-import type { Folder } from "@cap/web-domain"; +import type { FolderColor } from "@cap/web-domain";
🧹 Nitpick comments (2)
packages/web-backend/src/Folders/index.ts (1)
89-97: Server-side normalize/validate nameUI ensures a non-empty trimmed name, but the backend accepts any string. Consider trimming and enforcing non-empty (and perhaps a reasonable max length) server-side.
Example:
- const folder = { + const name = data.name.trim(); + if (name.length === 0) return yield* new Folder.NotFoundError(); // or a dedicated validation error + const folder = { id: Folder.FolderId.make(nanoId()), - name: data.name, + name,If you prefer a typed validation, we can strengthen the RPC payload schema (minLength) instead.
apps/web/app/(org)/dashboard/caps/components/NewFolderDialog.tsx (1)
56-58: Optional: type selectedColor as FolderColor | nullThis avoids inference through FolderOptions and keeps the type aligned with the domain.
Apply this diff:
- const [selectedColor, setSelectedColor] = useState< - (typeof FolderOptions)[number]["value"] | null - >(null); + const [selectedColor, setSelectedColor] = useState<FolderColor | null>(null);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
apps/web/app/(org)/dashboard/caps/components/NewFolderDialog.tsx(3 hunks)apps/web/app/(org)/dashboard/folder/[id]/components/SubfolderDialog.tsx(5 hunks)packages/web-backend/src/Folders/FoldersRpcs.ts(1 hunks)packages/web-backend/src/Folders/index.ts(2 hunks)packages/web-domain/src/Folder.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/web-backend/src/Folders/FoldersRpcs.ts
- apps/web/app/(org)/dashboard/folder/[id]/components/SubfolderDialog.tsx
🧰 Additional context used
🧬 Code Graph Analysis (3)
packages/web-domain/src/Folder.ts (3)
packages/web-domain/src/Errors.ts (1)
InternalError(3-6)packages/web-domain/src/Policy.ts (1)
PolicyDeniedError(18-21)packages/web-domain/src/Authentication.ts (1)
RpcAuthMiddleware(28-34)
packages/web-backend/src/Folders/index.ts (3)
packages/web-domain/src/Folder.ts (5)
Folder(18-26)FolderColor(10-10)FolderColor(11-11)FolderId(7-7)FolderId(8-8)packages/web-domain/src/Authentication.ts (1)
CurrentUser(7-10)packages/database/helpers.ts (1)
nanoId(6-9)
apps/web/app/(org)/dashboard/caps/components/NewFolderDialog.tsx (3)
apps/web/lib/EffectRuntime.ts (1)
useEffectMutation(19-19)packages/web-domain/src/Folder.ts (3)
Folder(18-26)FolderColor(10-10)FolderColor(11-11)apps/web/lib/Rpcs.ts (1)
withRpc(16-17)
⏰ 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). (4)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (rust)
🔇 Additional comments (5)
packages/web-backend/src/Folders/index.ts (1)
64-87: Consider enforcing policy on create (and align RPC error union)You’re initializing policy but not using it in create. If creation should be restricted (e.g., space-level constraints or org roles), add a policy guard and include PolicyDeniedError in FolderCreate’s error union.
Do you intend folder creation to be unrestricted for any authenticated user in the active org? If not, I can wire a policy check and update the RPC contract.
packages/web-domain/src/Folder.ts (3)
10-12: LGTM: FolderColor surface looks correctString-literal union matches UI options and backend usage.
18-26: LGTM: Folder model shape aligns with DB and service layerOptionFromNullOr matches nullable DB columns; fields align with backend insert/return shape.
33-43: Confirm create policy stance; include PolicyDeniedError if applicableFolderDelete includes PolicyDeniedError, but FolderCreate doesn’t. If you add creation policy checks in the backend (recommended), include PolicyDeniedError here as well to keep contracts consistent.
Apply this diff if you decide to enforce policy on create:
- error: Schema.Union(NotFoundError, InternalError), + error: Schema.Union(NotFoundError, InternalError, PolicyDeniedError),Also consider tightening name validation at the schema level (e.g., min length).
apps/web/app/(org)/dashboard/caps/components/NewFolderDialog.tsx (1)
150-164: Guard + types are fine; minor UX polishThe pre-check for selectedColor is correct. With the trim above, users won’t accidentally create “spacey” names. No further changes needed here.
| const createFolder = useEffectMutation({ | ||
| mutationFn: (data: { name: string; color: Folder.FolderColor }) => | ||
| withRpc((r) => | ||
| r.FolderCreate({ | ||
| name: data.name, | ||
| color: data.color, | ||
| spaceId: Option.fromNullable(spaceId), | ||
| parentId: Option.none(), | ||
| }), | ||
| ), | ||
| onSuccess: () => { |
There was a problem hiding this comment.
Align mutation payload type with FolderColor and trim the name
Use FolderColor for the color type and send a trimmed name to avoid leading/trailing spaces.
Apply this diff:
- const createFolder = useEffectMutation({
- mutationFn: (data: { name: string; color: Folder.FolderColor }) =>
+ const createFolder = useEffectMutation({
+ mutationFn: (data: { name: string; color: FolderColor }) =>
withRpc((r) =>
r.FolderCreate({
- name: data.name,
+ name: data.name.trim(),
color: data.color,
spaceId: Option.fromNullable(spaceId),
parentId: Option.none(),
}),
),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const createFolder = useEffectMutation({ | |
| mutationFn: (data: { name: string; color: Folder.FolderColor }) => | |
| withRpc((r) => | |
| r.FolderCreate({ | |
| name: data.name, | |
| color: data.color, | |
| spaceId: Option.fromNullable(spaceId), | |
| parentId: Option.none(), | |
| }), | |
| ), | |
| onSuccess: () => { | |
| const createFolder = useEffectMutation({ | |
| mutationFn: (data: { name: string; color: FolderColor }) => | |
| withRpc((r) => | |
| r.FolderCreate({ | |
| name: data.name.trim(), | |
| color: data.color, | |
| spaceId: Option.fromNullable(spaceId), | |
| parentId: Option.none(), | |
| }), | |
| ), | |
| onSuccess: () => { |
🤖 Prompt for AI Agents
In apps/web/app/(org)/dashboard/caps/components/NewFolderDialog.tsx around lines
67 to 77, the mutation payload uses an incorrect/loose color type and sends the
raw name; update the mutationFn signature to accept color as Folder.FolderColor
and send a trimmed name by calling data.name.trim() when building the
FolderCreate RPC payload (leave spaceId and parentId handling unchanged).
| create: Effect.fn("Folders.create")(function* (data: { | ||
| name: string; | ||
| color: Folder.FolderColor; | ||
| spaceId: Option.Option<string>; | ||
| parentId: Option.Option<Folder.FolderId>; | ||
| }) { | ||
| const user = yield* CurrentUser; | ||
|
|
||
| if (Option.isSome(data.parentId)) { | ||
| const parentId = data.parentId.value; | ||
| const [parentFolder] = yield* db.execute((db) => | ||
| db | ||
| .select() | ||
| .from(Db.folders) | ||
| .where( | ||
| Dz.and( | ||
| Dz.eq(Db.folders.id, parentId), | ||
| Dz.eq(Db.folders.organizationId, user.activeOrgId), | ||
| ), | ||
| ), | ||
| ); | ||
|
|
||
| if (!parentFolder) return yield* new Folder.NotFoundError(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate spaceId belongs to the active org (prevent cross-org references)
When creating a top-level folder in a space (no parentId), confirm the provided spaceId exists and is owned by user.activeOrgId. Otherwise, a folder could reference a space from another org.
Apply this diff after computing effectiveSpaceId and before insert:
+ // If creating a top-level folder in a space, ensure the space exists in the active org
+ if (Option.isNone(data.parentId) && Option.isSome(effectiveSpaceId)) {
+ const [space] = yield* db.execute((db) =>
+ db
+ .select({ id: Db.spaces.id })
+ .from(Db.spaces)
+ .where(
+ Dz.and(
+ Dz.eq(Db.spaces.id, effectiveSpaceId.value),
+ Dz.eq(Db.spaces.organizationId, user.activeOrgId),
+ ),
+ ),
+ );
+ if (!space) return yield* new Folder.NotFoundError();
+ }If the schema uses a different table identifier than Db.spaces, adjust accordingly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| create: Effect.fn("Folders.create")(function* (data: { | |
| name: string; | |
| color: Folder.FolderColor; | |
| spaceId: Option.Option<string>; | |
| parentId: Option.Option<Folder.FolderId>; | |
| }) { | |
| const user = yield* CurrentUser; | |
| if (Option.isSome(data.parentId)) { | |
| const parentId = data.parentId.value; | |
| const [parentFolder] = yield* db.execute((db) => | |
| db | |
| .select() | |
| .from(Db.folders) | |
| .where( | |
| Dz.and( | |
| Dz.eq(Db.folders.id, parentId), | |
| Dz.eq(Db.folders.organizationId, user.activeOrgId), | |
| ), | |
| ), | |
| ); | |
| if (!parentFolder) return yield* new Folder.NotFoundError(); | |
| } | |
| const user = yield* CurrentUser; | |
| // Use parent’s spaceId if creating a subfolder | |
| let effectiveSpaceId = data.spaceId; | |
| if (Option.isSome(data.parentId)) { | |
| const parentId = data.parentId.value; | |
| const [parentFolder] = yield* db.execute((db) => | |
| db | |
| .select() | |
| .from(Db.folders) | |
| .where( | |
| Dz.and( | |
| Dz.eq(Db.folders.id, parentId), | |
| Dz.eq(Db.folders.organizationId, user.activeOrgId), | |
| ), | |
| ), | |
| ); | |
| if (!parentFolder) return yield* new Folder.NotFoundError(); | |
| effectiveSpaceId = Option.fromNullable(parentFolder.spaceId); | |
| } | |
| // If creating a top-level folder in a space, ensure the space exists in the active org | |
| if (Option.isNone(data.parentId) && Option.isSome(effectiveSpaceId)) { | |
| const [space] = yield* db.execute((db) => | |
| db | |
| .select({ id: Db.spaces.id }) | |
| .from(Db.spaces) | |
| .where( | |
| Dz.and( | |
| Dz.eq(Db.spaces.id, effectiveSpaceId.value), | |
| Dz.eq(Db.spaces.organizationId, user.activeOrgId), | |
| ), | |
| ), | |
| ); | |
| if (!space) return yield* new Folder.NotFoundError(); | |
| } | |
| const folder = { | |
| id: Folder.FolderId.make(nanoId()), | |
| name: data.name, | |
| color: data.color, | |
| organizationId: user.activeOrgId, | |
| createdById: user.id, | |
| spaceId: effectiveSpaceId, | |
| parentId: data.parentId, | |
| }; | |
| yield* db.execute((db) => | |
| db.insert(Db.folders).values({ | |
| ...folder, | |
| spaceId: Option.getOrNull(effectiveSpaceId), | |
| parentId: Option.getOrNull(folder.parentId), | |
| }), | |
| ); | |
| return new Folder.Folder(folder); |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/web-backend/src/Rpcs.ts (2)
25-34: Fix Biome formatting to unblock CI (arrow param parentheses and layout)The CI is failing with a Biome formatting error on this block. Wrap the arrow function parameter in parentheses and let Biome reflow the object for consistency.
Apply this diff and run the formatter:
- Option.match({ - onNone: () => new UnauthenticatedError(), - onSome: user => Effect.succeed({ - id: user.id, - email: user.email, - activeOrgId: user.activeOrganizationId, - }), - }), + Option.match({ + onNone: () => new UnauthenticatedError(), + onSome: (user) => + Effect.succeed({ + id: user.id, + email: user.email, + activeOrgId: user.activeOrganizationId, + }), + }),Alternatively, run: pnpm biome format to apply house style across the repo.
27-31: Guard against missing activeOrganizationId (if not guaranteed non-null)If
user.activeOrganizationIdcan be null/undefined, this will violate the declared typeactiveOrgId: stringand propagate undefined to consumers. Consider failing the auth middleware when the user lacks an active org, or represent it explicitly as an Option in the auth context.Example (fail when missing):
- onSome: (user) => - Effect.succeed({ - id: user.id, - email: user.email, - activeOrgId: user.activeOrganizationId, - }), + onSome: (user) => + user.activeOrganizationId + ? Effect.succeed({ + id: user.id, + email: user.email, + activeOrgId: user.activeOrganizationId, + }) + : Effect.fail(new UnauthenticatedError()),If the domain guarantees
activeOrganizationIdis always set for authenticated users, please ignore and confirm that invariant.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/web-backend/src/Rpcs.ts(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI
packages/web-backend/src/Rpcs.ts
[error] 25-34: Biome formatting check failed: File content differs from formatting output.
⏰ 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 (1)
packages/web-backend/src/Rpcs.ts (1)
27-31: Addition of activeOrgId in auth context looks goodMapping
activeOrgIdfromuser.activeOrganizationIdaligns with the updatedCurrentUserpayload and enables org-scoped RPCs like FolderCreate. No functional concerns with this change.
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 (1)
packages/web-backend/src/Rpcs.ts (1)
21-24: catchAll should re-fail; currently returns a plain error value.
Effect.catchAllexpects the handler to return an Effect. Returningnew InternalError(...)makes the error a success value.Minimal fix:
- Effect.catchAll(() => new InternalError({ type: "database" })), + Effect.catchAll(() => Effect.fail(new InternalError({ type: "database" }))),Optional: Prefer mapping the error instead of catching-and-failing:
- Effect.catchAll(() => Effect.fail(new InternalError({ type: "database" }))), + Effect.mapError(() => new InternalError({ type: "database" })),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/web-backend/src/Rpcs.ts(1 hunks)
⏰ 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). (4)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
packages/web-backend/src/Rpcs.ts (1)
13-13: LGTM: clean RPC composition.Merging
VideosRpcsLiveandFolderRpcsLiveviaLayer.mergeAllis straightforward and keeps RPC wiring centralized.
| Effect.succeed({ | ||
| id: user.id, | ||
| email: user.email, | ||
| activeOrgId: user.activeOrganizationId, | ||
| }), |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify downstream usage of the renamed activeOrgId field.
You’re now exposing activeOrgId (derived from user.activeOrganizationId). Ensure all callers/types consume activeOrgId and there are no lingering references to activeOrganizationId.
Run this script from repo root to find stale usages and confirm type expectations:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for stale references to 'activeOrganizationId'..."
rg -nP -C2 '\bactiveOrganizationId\b' --type=ts --type=tsx
echo
echo "Consumers of 'activeOrgId'..."
rg -nP -C2 '\bactiveOrgId\b' --type=ts --type=tsx
echo
echo "Locate CurrentUser/RpcAuthMiddleware types to confirm shape..."
rg -nP -C3 '\b(CurrentUser|RpcAuthMiddleware)\b' --type=ts --type=tsxLength of output: 243
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "Searching for stale references to 'activeOrganizationId' in .ts/.tsx files..."
rg -nP -C2 '\bactiveOrganizationId\b' -g '*.ts' -g '*.tsx' || true
echo
echo "Searching for usages of 'activeOrgId' in .ts/.tsx files..."
rg -nP -C2 '\bactiveOrgId\b' -g '*.ts' -g '*.tsx' || true
echo
echo "Locating 'CurrentUser' type definitions in .ts/.tsx files..."
rg -nP -C3 '\b(interface|type)\s+CurrentUser\b' -g '*.ts' -g '*.tsx' || true
echo
echo "Locating 'RpcAuthMiddleware' references in .ts/.tsx files..."
rg -nP -C3 '\bRpcAuthMiddleware\b' -g '*.ts' -g '*.tsx' || trueLength of output: 23099
Update front-end references from activeOrganizationId to activeOrgId
The RPC layer now exposes activeOrgId (not activeOrganizationId) on the CurrentUser object. We still have numerous call sites in the front-end that will break or type-error. Please refactor all of these from user.activeOrganizationId (or currentUser.activeOrganizationId) to user.activeOrgId (or currentUser.activeOrgId):
• apps/web/app/s/[videoId]/page.tsx (lines ~525–528)
• apps/web/lib/folder.ts (lines ~229, 247)
• apps/web/lib/Notification.ts (lines ~39, 68, 102, 172–182)
• apps/web/app/api/notifications/route.ts (lines 56, 73)
• apps/web/actions/organization/* (upload-space-icon.ts, delete-space.ts, create-space.ts: lines 34, 75, 89)
• apps/web/actions/folders/* (updateFolder.ts, moveVideoToFolder.ts: lines ~21, 40)
• apps/web/components/forms/server.ts (line 110)
• apps/web/app/(org)/onboarding/page.tsx (line 19)
• apps/web/app/(org)/dashboard/**/* (Navbar/server.ts:35, layout.tsx:53, dashboard-data.ts:84–100, caps/page.tsx:132, 216)
Example replacements:
- if (!user || !user.activeOrganizationId) throw …
+ if (!user || !user.activeOrgId) throw …- eq(folders.organizationId, user.activeOrganizationId)
+ eq(folders.organizationId, user.activeOrgId)- eq(notifications.orgId, currentUser.activeOrganizationId)
+ eq(notifications.orgId, currentUser.activeOrgId)Use a global search (e.g. rg -t ts -t tsx '\bactiveOrganizationId\b') to catch all instances.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/web-backend/src/Rpcs.ts around lines 28–32 the RPC now returns
activeOrgId instead of activeOrganizationId; update the front-end to use
activeOrgId everywhere. Search the codebase (e.g. rg -t ts -t tsx
'\bactiveOrganizationId\b') and replace property accesses, destructurings and
type annotations from activeOrganizationId to activeOrgId in the listed files
(and any others found), update any client/server DTOs or types referencing the
old name, run the TypeScript build to catch remaining spots, and adjust tests
and imports accordingly so all references use activeOrgId consistently.
Summary by CodeRabbit
New Features
Refactor
Chores
Revert