Effect: Port UpdateFolder
#1142
Conversation
WalkthroughRemoves a legacy server action for folder updates and replaces direct DB calls with an Effect-based backend: adds FoldersRepo, Spaces service, new FolderUpdate RPC and Folders.update logic; tightens domain ID types; migrates client rename to RPC mutation; adds video RPCs and rewires policies and dependency wiring. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant UI as Folder.tsx (Client)
participant RPC as FolderRpcs.FolderUpdate
participant SVC as Folders Service
participant REPO as FoldersRepo
participant DB as Database
U->>UI: edit folder name (blur / Enter)
UI->>RPC: FolderUpdate.mutate({ id, name })
RPC->>SVC: update(id, data)
SVC->>REPO: getById(id) + policy check
alt parent changed
SVC->>REPO: getById(parentId, org scope)
SVC->>REPO: traverse parents → detect cycles
end
SVC->>REPO: update(id, data)
REPO->>DB: UPDATE folders ...
DB-->>REPO: updated row
REPO-->>SVC: Folder
SVC-->>RPC: Folder | DomainError
RPC-->>UI: success → toast + refresh / error → toast
sequenceDiagram
autonumber
actor U as User
participant RPC as VideosRpcs
participant VID as Videos Service
participant S3 as S3
participant DB as Database
U->>RPC: VideoDuplicate(videoId)
RPC->>VID: duplicate(videoId)
VID->>DB: read/write
DB-->>VID: result
VID-->>RPC: video | error
U->>RPC: GetUploadProgress(videoId)
RPC->>VID: getUploadProgress(videoId)
VID->>S3: head/status
S3-->>VID: progress
VID-->>RPC: progress | error
U->>RPC: VideoGetDownloadInfo(videoId)
RPC->>VID: getDownloadInfo(videoId)
VID->>S3: signed URL
S3-->>VID: url
VID-->>RPC: info | error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 (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)
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 |
| @@ -0,0 +1,4 @@ | |||
| import { Schema } from "effect"; | |||
|
|
|||
| export const SpaceId = Schema.String; // TODO: .pipe(Schema.brand("SpaceId")); | |||
There was a problem hiding this comment.
This is not branded yet for better compatibility with existing code.
I will enforce it in a follow up PR which goes through and enforces the branded types everywhere in the database schema.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
apps/web/actions/folders/updateFolder.ts(0 hunks)apps/web/app/(org)/dashboard/caps/components/Folder.tsx(2 hunks)apps/web/app/Layout/devtoolsServer.ts(0 hunks)apps/web/app/embed/page.tsx(0 hunks)apps/web/app/s/page.tsx(0 hunks)packages/web-backend/src/Folders/FoldersRepo.ts(1 hunks)packages/web-backend/src/Folders/FoldersRpcs.ts(2 hunks)packages/web-backend/src/Folders/index.ts(3 hunks)packages/web-backend/src/Loom/ImportVideo.ts(2 hunks)packages/web-backend/src/Videos/VideosRpcs.ts(2 hunks)packages/web-domain/src/Comment.ts(1 hunks)packages/web-domain/src/Folder.ts(3 hunks)packages/web-domain/src/Loom.ts(2 hunks)packages/web-domain/src/Organisation.ts(1 hunks)packages/web-domain/src/S3Bucket.ts(1 hunks)packages/web-domain/src/Space.ts(1 hunks)packages/web-domain/src/User.ts(1 hunks)packages/web-domain/src/Video.ts(1 hunks)packages/web-domain/src/index.ts(1 hunks)
💤 Files with no reviewable changes (4)
- apps/web/app/Layout/devtoolsServer.ts
- apps/web/app/embed/page.tsx
- apps/web/app/s/page.tsx
- apps/web/actions/folders/updateFolder.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
packages/web-domain/src/Organisation.tspackages/web-domain/src/Comment.tspackages/web-backend/src/Videos/VideosRpcs.tspackages/web-domain/src/Video.tsapps/web/app/(org)/dashboard/caps/components/Folder.tsxpackages/web-domain/src/User.tspackages/web-domain/src/Space.tspackages/web-backend/src/Folders/FoldersRpcs.tspackages/web-domain/src/index.tspackages/web-backend/src/Folders/FoldersRepo.tspackages/web-domain/src/Loom.tspackages/web-domain/src/S3Bucket.tspackages/web-backend/src/Loom/ImportVideo.tspackages/web-domain/src/Folder.tspackages/web-backend/src/Folders/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
packages/web-domain/src/Organisation.tspackages/web-domain/src/Comment.tspackages/web-backend/src/Videos/VideosRpcs.tspackages/web-domain/src/Video.tsapps/web/app/(org)/dashboard/caps/components/Folder.tsxpackages/web-domain/src/User.tspackages/web-domain/src/Space.tspackages/web-backend/src/Folders/FoldersRpcs.tspackages/web-domain/src/index.tspackages/web-backend/src/Folders/FoldersRepo.tspackages/web-domain/src/Loom.tspackages/web-domain/src/S3Bucket.tspackages/web-backend/src/Loom/ImportVideo.tspackages/web-domain/src/Folder.tspackages/web-backend/src/Folders/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-domain/src/Organisation.tspackages/web-domain/src/Comment.tspackages/web-backend/src/Videos/VideosRpcs.tspackages/web-domain/src/Video.tsapps/web/app/(org)/dashboard/caps/components/Folder.tsxpackages/web-domain/src/User.tspackages/web-domain/src/Space.tspackages/web-backend/src/Folders/FoldersRpcs.tspackages/web-domain/src/index.tspackages/web-backend/src/Folders/FoldersRepo.tspackages/web-domain/src/Loom.tspackages/web-domain/src/S3Bucket.tspackages/web-backend/src/Loom/ImportVideo.tspackages/web-domain/src/Folder.tspackages/web-backend/src/Folders/index.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 data fetching in the web app
Web mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData rather than broad invalidations
Client code should use useEffectQuery/useEffectMutation and useRpcClient from apps/web/lib/EffectRuntime.ts; do not create ManagedRuntime inside components
Files:
apps/web/app/(org)/dashboard/caps/components/Folder.tsx
apps/web/app/**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer Server Components for initial data in the Next.js App Router and pass initialData to client components
Files:
apps/web/app/(org)/dashboard/caps/components/Folder.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/app/(org)/dashboard/caps/components/Folder.tsx
🧬 Code graph analysis (11)
packages/web-backend/src/Videos/VideosRpcs.ts (2)
packages/database/schema.ts (1)
videos(248-295)packages/web-domain/src/Errors.ts (1)
InternalError(3-6)
packages/web-domain/src/Video.ts (2)
packages/web-domain/src/User.ts (2)
UserId(3-3)UserId(4-4)packages/web-domain/src/Organisation.ts (2)
OrganisationId(3-5)OrganisationId(6-6)
apps/web/app/(org)/dashboard/caps/components/Folder.tsx (3)
apps/web/lib/EffectRuntime.ts (1)
useEffectMutation(24-24)packages/web-domain/src/Folder.ts (2)
Folder(37-45)FolderUpdate(47-52)apps/web/lib/Rpcs.ts (1)
withRpc(16-17)
packages/web-backend/src/Folders/FoldersRpcs.ts (2)
packages/database/schema.ts (1)
folders(222-246)packages/web-domain/src/Errors.ts (1)
InternalError(3-6)
packages/web-domain/src/index.ts (2)
packages/web-domain/src/Folder.ts (1)
Folder(37-45)packages/web-domain/src/Organisation.ts (1)
Organisation(8-11)
packages/web-backend/src/Folders/FoldersRepo.ts (4)
packages/web-domain/src/Folder.ts (3)
Folder(37-45)FolderId(11-11)FolderId(12-12)packages/web-domain/src/Organisation.ts (3)
Organisation(8-11)OrganisationId(3-5)OrganisationId(6-6)packages/web-domain/src/User.ts (2)
UserId(3-3)UserId(4-4)packages/database/helpers.ts (1)
nanoId(6-9)
packages/web-domain/src/Loom.ts (2)
packages/web-domain/src/User.ts (2)
UserId(3-3)UserId(4-4)packages/web-domain/src/Organisation.ts (2)
OrganisationId(3-5)OrganisationId(6-6)
packages/web-domain/src/S3Bucket.ts (1)
packages/web-domain/src/User.ts (2)
UserId(3-3)UserId(4-4)
packages/web-backend/src/Loom/ImportVideo.ts (2)
packages/web-domain/src/Organisation.ts (1)
Organisation(8-11)packages/web-domain/src/Video.ts (1)
Video(16-59)
packages/web-domain/src/Folder.ts (6)
packages/web-domain/src/Organisation.ts (2)
OrganisationId(3-5)OrganisationId(6-6)packages/web-domain/src/User.ts (2)
UserId(3-3)UserId(4-4)packages/web-domain/src/Space.ts (2)
SpaceId(3-3)SpaceId(4-4)packages/web-domain/src/Errors.ts (1)
InternalError(3-6)packages/web-domain/src/Authentication.ts (1)
RpcAuthMiddleware(29-35)packages/web-domain/src/Policy.ts (1)
PolicyDeniedError(19-22)
packages/web-backend/src/Folders/index.ts (3)
packages/web-domain/src/Folder.ts (4)
Folder(37-45)FolderId(11-11)FolderId(12-12)FolderUpdate(47-52)packages/web-domain/src/Organisation.ts (1)
Organisation(8-11)packages/web-domain/src/Policy.ts (2)
Policy(7-11)policy(27-39)
⏰ 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 (2)
packages/web-backend/src/Videos/VideosRpcs.ts (2)
20-26: LGTM: Consistent error handling pattern.The
VideoDuplicatehandler follows the same error-handling structure as the existingVideoDeletehandler, mappingDatabaseErrorandS3ErrortoInternalErrorwith appropriate types.
28-35: Error tags and auth pipeline are valid.UnknownException,DatabaseError, andS3Errorexist and cover all errors fromgetUploadProgressandgetDownloadInfo, andprovideOptionalAuthis correctly applied beforecatchTags.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/web-domain/src/Folder.ts (1)
43-43: Fix type inconsistency: Use branded SpaceId.Line 43 still uses
Schema.Stringfor spaceId, but line 63 (FolderCreate RPC) now uses the brandedSpaceIdtype. This creates a type mismatch where the RPC payload is more restrictive than the domain class it creates.Apply this diff to align with the branded type:
- spaceId: Schema.OptionFromNullOr(Schema.String), + spaceId: Schema.OptionFromNullOr(SpaceId),packages/web-backend/src/Folders/index.ts (1)
129-196: Critical: Early return prevents name/color updates.Line 146 exits immediately when
data.parentIdis falsy, making the update a no-op for any request that omits parentId (valid for name/color-only edits). The DB update at lines 186-195 becomes unreachable.Restructure the control flow so parent validation runs only when parentId is supplied:
- // If parentId is provided and not null, verify it exists and belongs to the same organization - if (!data.parentId) return; - const parentId = data.parentId; - - // Check that we're not creating an immediate circular reference - if (parentId === folderId) - return yield* new Folder.RecursiveDefinitionError(); - - const parentFolder = yield* repo - .getById(parentId, { - organizationId: Organisation.OrganisationId.make( - folder.organizationId, - ), - }) - .pipe( - Policy.withPolicy(policy.canEdit(parentId)), - Effect.flatMap( - Effect.catchTag( - "NoSuchElementException", - () => new Folder.ParentNotFoundError(), - ), - ), - ); - - // Check for circular references in the folder hierarchy - let currentParentId = parentFolder.parentId; - while (currentParentId) { - if (currentParentId === folderId) - return yield* new Folder.RecursiveDefinitionError(); - - const parentId = currentParentId; - const nextParent = yield* repo.getById(parentId, { - organizationId: Organisation.OrganisationId.make( - folder.organizationId, - ), - }); - - if (Option.isNone(nextParent)) break; - currentParentId = nextParent.value.parentId; - } + // If parentId is provided, verify it exists and belongs to the same organization + if (data.parentId) { + const parentId = data.parentId; + + // Check that we're not creating an immediate circular reference + if (parentId === folderId) + return yield* new Folder.RecursiveDefinitionError(); + + const parentFolder = yield* repo + .getById(parentId, { + organizationId: Organisation.OrganisationId.make( + folder.organizationId, + ), + }) + .pipe( + Policy.withPolicy(policy.canEdit(parentId)), + Effect.flatMap( + Effect.catchTag( + "NoSuchElementException", + () => new Folder.ParentNotFoundError(), + ), + ), + ); + + // Check for circular references in the folder hierarchy + let currentParentId = parentFolder.parentId; + while (currentParentId) { + if (currentParentId === folderId) + return yield* new Folder.RecursiveDefinitionError(); + + const parentId = currentParentId; + const nextParent = yield* repo.getById(parentId, { + organizationId: Organisation.OrganisationId.make( + folder.organizationId, + ), + }); + + if (Option.isNone(nextParent)) break; + currentParentId = nextParent.value.parentId; + } + } yield* db.execute((db) => db .update(Db.folders) .set({ ...(data.name ? { name: data.name } : {}), ...(data.color ? { color: data.color } : {}), ...(data.parentId ? { parentId: data.parentId } : {}), }) .where(Dz.eq(Db.folders.id, folderId)), );
🧹 Nitpick comments (1)
packages/web-backend/src/Folders/index.ts (1)
186-195: Update logic should use repository method.The update currently bypasses the repository layer and directly executes a DB update, which is inconsistent with the create/read operations that use the repo. Additionally, the conditional spread operators create an empty object when fields are falsy, making the update a no-op.
After fixing the early return issue, consider using
repo.updateinstead of direct DB access:- yield* db.execute((db) => - db - .update(Db.folders) - .set({ - ...(data.name ? { name: data.name } : {}), - ...(data.color ? { color: data.color } : {}), - ...(data.parentId ? { parentId: data.parentId } : {}), - }) - .where(Dz.eq(Db.folders.id, folderId)), - ); + yield* repo.update(folderId, { + ...(data.name !== undefined && { name: data.name }), + ...(data.color !== undefined && { color: data.color }), + ...(data.parentId !== undefined && { + parentId: Option.some(data.parentId) + }), + });Note: This assumes
repo.updateaccepts the same shape. Verify the signature in FoldersRepo.ts before applying.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/web-backend/src/Folders/FoldersRepo.ts(1 hunks)packages/web-backend/src/Folders/index.ts(3 hunks)packages/web-domain/src/Folder.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/web-backend/src/Folders/FoldersRepo.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
packages/web-domain/src/Folder.tspackages/web-backend/src/Folders/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
packages/web-domain/src/Folder.tspackages/web-backend/src/Folders/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-domain/src/Folder.tspackages/web-backend/src/Folders/index.ts
🧬 Code graph analysis (2)
packages/web-domain/src/Folder.ts (6)
packages/web-domain/src/Organisation.ts (2)
OrganisationId(3-5)OrganisationId(6-6)packages/web-domain/src/User.ts (2)
UserId(3-3)UserId(4-4)packages/web-domain/src/Space.ts (2)
SpaceId(3-3)SpaceId(4-4)packages/web-domain/src/Errors.ts (1)
InternalError(3-6)packages/web-domain/src/Policy.ts (1)
PolicyDeniedError(19-22)packages/web-domain/src/Authentication.ts (1)
RpcAuthMiddleware(29-35)
packages/web-backend/src/Folders/index.ts (5)
packages/web-backend/src/Database.ts (1)
Database(9-19)packages/web-backend/src/Folders/FoldersPolicy.ts (1)
FoldersPolicy(8-47)packages/web-backend/src/Folders/FoldersRepo.ts (1)
FoldersRepo(17-88)packages/web-domain/src/Organisation.ts (1)
Organisation(8-11)packages/web-domain/src/Folder.ts (4)
Folder(37-45)FolderId(11-11)FolderId(12-12)FolderUpdate(47-52)
⏰ 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 (6)
packages/web-domain/src/Folder.ts (4)
1-9: Imports are correctly structured.The addition of HttpApiSchema for status annotations and branded ID types (OrganisationId, SpaceId, UserId) aligns with the domain modeling goals and enables type-safe ID handling across the codebase.
17-35: HTTP annotations on errors are properly applied.The NotFoundError, RecursiveDefinitionError, and ParentNotFoundError classes correctly use HttpApiSchema.annotations to specify HTTP status codes (404, 409, 404 respectively), which will enable proper error-to-HTTP-status mapping in the RPC layer.
47-52: FolderUpdate class correctly models partial updates.The class uses
Schema.optionalfor mutable fields (name, color, parentId) while keepingidrequired, which is the correct pattern for partial updates. The class name "FolderPatch" in the Schema.Class declaration accurately represents the patch semantics.
54-78: RPC definitions are well-structured with comprehensive error handling.The FolderUpdate RPC correctly declares all possible error types (RecursiveDefinitionError, ParentNotFoundError, PolicyDeniedError, NotFoundError, InternalError), uses the FolderUpdate payload class, and applies the authentication middleware. The error union covers all failure modes from the implementation.
packages/web-backend/src/Folders/index.ts (2)
3-21: Dependencies correctly updated for repository-based flow.The imports now include Organisation and User for branded ID construction, and the FoldersRepo dependency is properly injected. This aligns with the shift to repository-based operations and type-safe ID handling.
73-111: Folder creation correctly uses repository layer.The refactored create method properly:
- Validates parent folder existence using
repo.getByIdwith organization scoping- Applies policy checks before creation
- Uses branded IDs (
Organisation.OrganisationId.make,User.UserId.make)- Delegates persistence to
repo.create
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/web-backend/src/Folders/index.ts (1)
146-146: Critical: Early return blocks all updates whenparentIdis omitted.Line 146 returns immediately if
data.parentIdis falsy, preventing name/color updates when the client doesn't supplyparentId. This makes the endpoint unusable for common edit cases.Restructure the control flow:
- // If parentId is provided and not null, verify it exists and belongs to the same organization - if (!data.parentId) return; - const parentId = data.parentId; + // If parentId is provided, verify it exists and belongs to the same organization + if (data.parentId) { + const parentId = data.parentId; - // Check that we're not creating an immediate circular reference - if (parentId === folderId) - return yield* new Folder.RecursiveDefinitionError(); + // Check that we're not creating an immediate circular reference + if (parentId === folderId) + return yield* new Folder.RecursiveDefinitionError(); - const parentFolder = yield* repo - .getById(parentId, { - organizationId: Organisation.OrganisationId.make( - folder.organizationId, - ), - }) - .pipe( - Policy.withPolicy(policy.canEdit(parentId)), - Effect.flatMap( - Effect.catchTag( - "NoSuchElementException", - () => new Folder.ParentNotFoundError(), - ), - ), - ); + const parentFolder = yield* repo + .getById(parentId, { + organizationId: Organisation.OrganisationId.make( + folder.organizationId, + ), + }) + .pipe( + Policy.withPolicy(policy.canEdit(parentId)), + Effect.flatMap( + Effect.catchTag( + "NoSuchElementException", + () => new Folder.ParentNotFoundError(), + ), + ), + ); - // Check for circular references in the folder hierarchy - let currentParentId = parentFolder.parentId; - while (currentParentId) { - if (currentParentId === folderId) - return yield* new Folder.RecursiveDefinitionError(); + // Check for circular references in the folder hierarchy + let currentParentId = parentFolder.parentId; + while (currentParentId) { + if (currentParentId === folderId) + return yield* new Folder.RecursiveDefinitionError(); - const parentId = currentParentId; - const nextParent = yield* repo.getById(parentId, { - organizationId: Organisation.OrganisationId.make( - folder.organizationId, - ), - }); + const parentId = currentParentId; + const nextParent = yield* repo.getById(parentId, { + organizationId: Organisation.OrganisationId.make( + folder.organizationId, + ), + }); - if (Option.isNone(nextParent)) break; - currentParentId = nextParent.value.parentId; + if (Option.isNone(nextParent)) break; + currentParentId = nextParent.value.parentId; + } } yield* db.execute((db) =>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/web-backend/src/Folders/index.ts(3 hunks)packages/web-domain/src/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
packages/web-domain/src/index.tspackages/web-backend/src/Folders/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
packages/web-domain/src/index.tspackages/web-backend/src/Folders/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-domain/src/index.tspackages/web-backend/src/Folders/index.ts
🧬 Code graph analysis (2)
packages/web-domain/src/index.ts (2)
packages/web-domain/src/Organisation.ts (1)
Organisation(8-11)packages/web-api-contract-effect/src/index.ts (1)
User(51-58)
packages/web-backend/src/Folders/index.ts (5)
packages/web-backend/src/Database.ts (1)
Database(9-19)packages/web-backend/src/Folders/FoldersPolicy.ts (1)
FoldersPolicy(8-47)packages/web-backend/src/Folders/FoldersRepo.ts (1)
FoldersRepo(17-88)packages/web-domain/src/Organisation.ts (1)
Organisation(8-11)packages/web-domain/src/Folder.ts (4)
Folder(37-45)FolderId(11-11)FolderId(12-12)FolderUpdate(47-52)
⏰ 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 (4)
packages/web-domain/src/index.ts (2)
5-5: LGTM! Export collision fixed.The Organisation export has been updated to use a namespace export, resolving the critical duplicate-export issue flagged in the previous review. This change is consistent with the other domain module exports (Folder, Policy, Video, etc.).
Note: This is a breaking change for consumers—
Organisationis now a namespace rather than a direct class re-export. However, this aligns with the branded-ID refactoring described in the PR context and is consistent with the overall module structure.
2-2: LGTM! Namespace exports are consistent.The new namespace exports for Comment, Space, and User follow the established pattern used for other domain modules (Folder, Policy, S3Bucket, Video). This provides a clean, uniform API surface for accessing domain types and their branded identifiers.
Also applies to: 9-10
packages/web-backend/src/Folders/index.ts (2)
84-110: LGTM: Clean repository-based refactor.The parent validation and creation logic correctly:
- Scopes parent lookup to the user's organization
- Validates edit permissions on the parent folder
- Maps repository exceptions to domain errors
- Uses typed domain IDs (Organisation.OrganisationId, User.UserId)
199-199: LGTM: Dependency correctly added.The
FoldersRepo.Defaultdependency is properly included, supporting the repository-based refactor.
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (2)
packages/web-backend/src/Folders/index.ts (2)
166-180: Fix Option handling in circular reference detection.This issue was already flagged in a previous review. The loop incorrectly mixes
Optiontypes with direct comparisons:
- Line 166:
currentParentIdisOption<FolderId>but used in a truthiness check- Line 168: Compares
Option<FolderId>withFolder.FolderIddirectly- Line 171: Shadows the outer
parentIdvariable unnecessarilySee the previous review comment for the detailed fix.
183-194: Replace direct DB update with repo.update for consistency.This issue was already flagged in a previous review. The update bypasses
repo.update, duplicating logic and omitting the returned folder.See the previous review comment for the detailed fix.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/web-backend/src/Folders/FoldersPolicy.ts(1 hunks)packages/web-backend/src/Folders/index.ts(3 hunks)packages/web-backend/src/Spaces/SpacesPolicy.ts(1 hunks)packages/web-domain/src/Folder.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
packages/web-backend/src/Spaces/SpacesPolicy.tspackages/web-backend/src/Folders/FoldersPolicy.tspackages/web-backend/src/Folders/index.tspackages/web-domain/src/Folder.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
packages/web-backend/src/Spaces/SpacesPolicy.tspackages/web-backend/src/Folders/FoldersPolicy.tspackages/web-backend/src/Folders/index.tspackages/web-domain/src/Folder.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/Spaces/SpacesPolicy.tspackages/web-backend/src/Folders/FoldersPolicy.tspackages/web-backend/src/Folders/index.tspackages/web-domain/src/Folder.ts
🧬 Code graph analysis (4)
packages/web-backend/src/Spaces/SpacesPolicy.ts (1)
packages/web-domain/src/Policy.ts (1)
Policy(7-11)
packages/web-backend/src/Folders/FoldersPolicy.ts (3)
packages/web-backend/src/Folders/FoldersRepo.ts (1)
FoldersRepo(17-88)packages/web-backend/src/Spaces/SpacesPolicy.ts (1)
SpacesPolicy(8-40)packages/web-domain/src/Folder.ts (3)
Folder(37-45)FolderId(11-11)FolderId(12-12)
packages/web-backend/src/Folders/index.ts (7)
packages/web-backend/src/Database.ts (1)
Database(9-19)packages/web-domain/src/Policy.ts (2)
policy(27-39)Policy(7-11)packages/web-backend/src/Folders/FoldersPolicy.ts (1)
FoldersPolicy(8-37)packages/web-backend/src/Folders/FoldersRepo.ts (1)
FoldersRepo(17-88)packages/web-domain/src/Organisation.ts (1)
Organisation(8-11)packages/web-domain/src/Folder.ts (5)
Folder(37-45)FolderId(11-11)FolderId(12-12)FolderUpdate(47-52)FolderUpdate(53-53)packages/web-api-contract-effect/src/index.ts (1)
User(51-58)
packages/web-domain/src/Folder.ts (6)
packages/web-domain/src/Organisation.ts (2)
OrganisationId(3-5)OrganisationId(6-6)packages/web-domain/src/User.ts (2)
UserId(3-3)UserId(4-4)packages/web-domain/src/Space.ts (2)
SpaceId(3-3)SpaceId(4-4)packages/web-domain/src/Errors.ts (1)
InternalError(3-6)packages/web-domain/src/Policy.ts (1)
PolicyDeniedError(19-22)packages/web-domain/src/Authentication.ts (1)
RpcAuthMiddleware(29-35)
⏰ 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 (1)
packages/web-backend/src/Spaces/SpacesPolicy.ts (1)
14-17: LGTM! Cleaner policy composition.The refactor from a generator-based
Effect.fnwrapper to a direct pipe-based composition is correct and more idiomatic. Both returnEffect<boolean, ...>as expected byPolicy.policy, but the new approach eliminates unnecessary nesting.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (7)
packages/web-backend/src/Folders/FoldersPolicy.ts (2)
22-27: Fix double-yield syntax and Option handling.The double-yield pattern is incorrect:
repo.getById(id)returnsEffect<Option<Folder>, DatabaseError, ...>- The first
yield*unwraps toOption<Folder>- Calling
.pipe(Effect.catchTag(...))on anOptionfails becauseOptiondoesn't supportEffectcombinatorsAdditionally,
repo.getByIdreturns anEffect<Option<...>>, so when the array is empty,Array.get(0)producesOption.none(), not aNoSuchElementException. ThecatchTagwill never trigger.Apply this fix:
-const folder = yield* (yield* repo.getById(id)).pipe( - Effect.catchTag( - "NoSuchElementException", - () => new Policy.PolicyDeniedError(), - ), -); +const folder = yield* repo.getById(id).pipe( + Effect.flatMap((opt) => + Option.match(opt, { + onNone: () => Effect.fail(new Policy.PolicyDeniedError()), + onSome: (f) => Effect.succeed(f), + }), + ), +);
29-38: Fix Option type handling for spaceId.Multiple issues:
- Line 29:
folder.spaceIdisOption<string>per the schema, not nullable. The=== nullcheck will never be true.- Line 31:
spaces.getSpaceOrOrgexpects astring, butfolder.spaceIdis anOption<string>and must be unwrapped.- Line 32: Based on the implementation in
Spaces/index.ts,getSpaceOrOrgcan returnundefinedwhen neither space nor org is found, making the falsy check necessary but potentially masking the underlying issue.Apply this fix:
-if (folder.spaceId === null) return folder.createdById === user.id; +if (Option.isNone(folder.spaceId)) return folder.createdById === user.id; -const spaceOrOrg = yield* spaces.getSpaceOrOrg(folder.spaceId); +const spaceOrOrg = yield* spaces.getSpaceOrOrg(folder.spaceId.value); if (!spaceOrOrg) return false;packages/web-backend/src/Folders/index.ts (5)
84-98: Fix Effect.flatMap syntax and Option handling.Multiple issues:
Effect.flatMap(Effect.catchTag(...))is incorrect—flatMapexpects a function(a) => Effect<...>, not anEffectdirectlyrepo.getByIdreturnsEffect<Option<Folder>>, soArray.get(0)producesOption.none()when not found, not aNoSuchElementExceptionApply this fix:
yield* repo .getById(parentId, { organizationId: Organisation.OrganisationId.make( user.activeOrganizationId, ), }) .pipe( Policy.withPolicy(policy.canEdit(parentId)), - Effect.flatMap( - Effect.catchTag( - "NoSuchElementException", - () => new Folder.NotFoundError(), - ), - ), + Effect.flatMap((opt) => + Option.match(opt, { + onNone: () => Effect.fail(new Folder.NotFoundError()), + onSome: (f) => Effect.succeed(f), + }), + ), );
133-140: Fix double-yield syntax in folder fetch.Same double-yield issue:
repo.getById(folderId).pipe(Policy.withPolicy(...))returns anEffect<Option<Folder>, ...>, so the firstyield*gives anOption<Folder>, not anEffect. Calling.pipe(Effect.catchTag(...))on thatOptionis invalid.Apply this fix:
-const folder = yield* (yield* repo - .getById(folderId) - .pipe(Policy.withPolicy(policy.canEdit(folderId)))).pipe( - Effect.catchTag( - "NoSuchElementException", - () => new Folder.NotFoundError(), - ), -); +const folder = yield* repo + .getById(folderId) + .pipe( + Policy.withPolicy(policy.canEdit(folderId)), + Effect.flatMap((opt) => + Option.match(opt, { + onNone: () => Effect.fail(new Folder.NotFoundError()), + onSome: (f) => Effect.succeed(f), + }), + ), + );
149-163: Fix Effect.flatMap syntax in parent validation.
Effect.flatMap(Effect.catchTag(...))is incorrect—flatMapexpects a function, not anEffect.Apply this fix:
const parentFolder = yield* repo .getById(parentId, { organizationId: Organisation.OrganisationId.make( folder.organizationId, ), }) .pipe( Policy.withPolicy(policy.canEdit(parentId)), - Effect.flatMap( - Effect.catchTag( - "NoSuchElementException", - () => new Folder.ParentNotFoundError(), - ), - ), + Effect.flatMap((opt) => + Option.match(opt, { + onNone: () => Effect.fail(new Folder.ParentNotFoundError()), + onSome: (f) => Effect.succeed(f), + }), + ), );
166-180: Fix Option handling in circular reference detection.The loop incorrectly mixes Option types with direct comparisons:
- Line 166:
parentFolder.parentIdisOption<FolderId>- Line 167:
while (currentParentId)checks truthiness of the Option object itself (always true), not whether it contains a value- Line 168: Compares
Option<FolderId>withFolder.FolderIddirectly- Line 171: Shadows the outer
parentIdvariable unnecessarily- Line 179: Assigns an
Option<FolderId>tocurrentParentId, perpetuating the type mismatchProperly handle Option types:
-let currentParentId = parentFolder.parentId; -while (currentParentId) { - if (currentParentId === folderId) +let currentParentId = Option.getOrNull(parentFolder.parentId); +while (currentParentId !== null) { + if (currentParentId === folderId) { return yield* new Folder.RecursiveDefinitionError(); + } - const parentId = currentParentId; - const nextParent = yield* repo.getById(parentId, { + const nextParent = yield* repo.getById(currentParentId, { organizationId: Organisation.OrganisationId.make( folder.organizationId, ), }); - if (Option.isNone(nextParent)) break; - currentParentId = nextParent.value.parentId; + currentParentId = Option.match(nextParent, { + onNone: () => null, + onSome: (parent) => Option.getOrNull(parent.parentId), + }); }
183-194: Replace direct DB update with repo.update for consistency.The update bypasses
repo.update, duplicating logic and omitting the returned folder. Additionally, the optional field handling has issues:
- Setting
nameandcolortoundefinedwhen not provided may unintentionally clear these fields- Line 189:
data.parentId ? ...will be truthy even forOption.none()because the Option object itself is truthy- The update doesn't set
updatedAt, whichrepo.updatehandles automaticallyReplace the direct database update:
-yield* db.execute((db) => - db - .update(Db.folders) - .set({ - name: data.name, - color: data.color, - parentId: data.parentId - ? Option.getOrNull(data.parentId) - : undefined, - }) - .where(Dz.eq(Db.folders.id, folderId)), -); +yield* repo.update(folderId, { + ...(data.name !== undefined ? { name: data.name } : {}), + ...(data.color !== undefined ? { color: data.color } : {}), + ...(data.parentId !== undefined ? { parentId: data.parentId } : {}), +});Note:
repo.updateexpectsparentIdasOption<Folder.FolderId>perFoldersRepo.ts. This also ensuresupdatedAtis set automatically.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/web/app/(org)/dashboard/spaces/[spaceId]/folder/[folderId]/page.tsx(2 hunks)apps/web/app/(org)/dashboard/spaces/[spaceId]/page.tsx(2 hunks)apps/web/app/(org)/dashboard/spaces/[spaceId]/utils.ts(0 hunks)apps/web/lib/server.ts(2 hunks)packages/web-backend/src/Folders/FoldersPolicy.ts(1 hunks)packages/web-backend/src/Folders/index.ts(3 hunks)packages/web-backend/src/Organisations/OrganisationsPolicy.ts(1 hunks)packages/web-backend/src/Organisations/OrganisationsRepo.ts(0 hunks)packages/web-backend/src/Spaces/index.ts(1 hunks)packages/web-backend/src/index.ts(1 hunks)
💤 Files with no reviewable changes (2)
- apps/web/app/(org)/dashboard/spaces/[spaceId]/utils.ts
- packages/web-backend/src/Organisations/OrganisationsRepo.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
packages/web-backend/src/index.tspackages/web-backend/src/Organisations/OrganisationsPolicy.tspackages/web-backend/src/Spaces/index.tspackages/web-backend/src/Folders/FoldersPolicy.tsapps/web/app/(org)/dashboard/spaces/[spaceId]/folder/[folderId]/page.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/page.tsxpackages/web-backend/src/Folders/index.tsapps/web/lib/server.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
packages/web-backend/src/index.tspackages/web-backend/src/Organisations/OrganisationsPolicy.tspackages/web-backend/src/Spaces/index.tspackages/web-backend/src/Folders/FoldersPolicy.tsapps/web/app/(org)/dashboard/spaces/[spaceId]/folder/[folderId]/page.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/page.tsxpackages/web-backend/src/Folders/index.tsapps/web/lib/server.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/index.tspackages/web-backend/src/Organisations/OrganisationsPolicy.tspackages/web-backend/src/Spaces/index.tspackages/web-backend/src/Folders/FoldersPolicy.tsapps/web/app/(org)/dashboard/spaces/[spaceId]/folder/[folderId]/page.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/page.tsxpackages/web-backend/src/Folders/index.tsapps/web/lib/server.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 data fetching in the web app
Web mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData rather than broad invalidations
Client code should use useEffectQuery/useEffectMutation and useRpcClient from apps/web/lib/EffectRuntime.ts; do not create ManagedRuntime inside components
Files:
apps/web/app/(org)/dashboard/spaces/[spaceId]/folder/[folderId]/page.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/page.tsxapps/web/lib/server.ts
apps/web/app/**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer Server Components for initial data in the Next.js App Router and pass initialData to client components
Files:
apps/web/app/(org)/dashboard/spaces/[spaceId]/folder/[folderId]/page.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/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/app/(org)/dashboard/spaces/[spaceId]/folder/[folderId]/page.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/page.tsxapps/web/lib/server.ts
🧬 Code graph analysis (7)
packages/web-backend/src/Organisations/OrganisationsPolicy.ts (1)
packages/web-domain/src/Policy.ts (1)
Policy(7-11)
packages/web-backend/src/Spaces/index.ts (3)
packages/web-backend/src/Spaces/SpacesPolicy.ts (1)
SpacesPolicy(8-40)packages/web-backend/src/Organisations/OrganisationsPolicy.ts (1)
OrganisationsPolicy(8-39)packages/web-domain/src/Policy.ts (1)
Policy(7-11)
packages/web-backend/src/Folders/FoldersPolicy.ts (6)
packages/web-backend/src/Folders/FoldersRepo.ts (1)
FoldersRepo(17-88)packages/web-backend/src/Spaces/SpacesPolicy.ts (1)
SpacesPolicy(8-40)packages/web-backend/src/Organisations/OrganisationsPolicy.ts (1)
OrganisationsPolicy(8-39)packages/web-backend/src/Spaces/index.ts (1)
Spaces(10-62)packages/web-domain/src/Folder.ts (3)
Folder(37-45)FolderId(11-11)FolderId(12-12)packages/web-backend/src/Database.ts (1)
Database(9-19)
apps/web/app/(org)/dashboard/spaces/[spaceId]/folder/[folderId]/page.tsx (2)
packages/web-backend/src/Spaces/index.ts (1)
Spaces(10-62)packages/web-backend/src/index.ts (1)
Spaces(8-8)
apps/web/app/(org)/dashboard/spaces/[spaceId]/page.tsx (2)
packages/web-backend/src/Spaces/index.ts (1)
Spaces(10-62)packages/web-backend/src/index.ts (1)
Spaces(8-8)
packages/web-backend/src/Folders/index.ts (6)
packages/web-backend/src/Database.ts (1)
Database(9-19)packages/web-backend/src/Folders/FoldersPolicy.ts (1)
FoldersPolicy(10-52)packages/web-backend/src/Folders/FoldersRepo.ts (1)
FoldersRepo(17-88)packages/web-domain/src/Organisation.ts (1)
Organisation(8-11)packages/web-domain/src/Folder.ts (5)
Folder(37-45)FolderId(11-11)FolderId(12-12)FolderUpdate(47-52)FolderUpdate(53-53)packages/web-api-contract-effect/src/index.ts (1)
User(51-58)
apps/web/lib/server.ts (2)
packages/web-backend/src/Spaces/index.ts (1)
Spaces(10-62)packages/web-backend/src/index.ts (1)
Spaces(8-8)
⏰ 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)
Was waiting for Claude to do some stuff so thought I would start.
Summary by CodeRabbit
New Features
Bug Fixes