remove non-effect s3 abstraction #1060
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughMigrates S3 interactions from a local S3 util/provider to an Effect-based S3Buckets/S3BucketAccess model (getBucketAccess / getBucketAccessForUser) executed via runPromise across actions, API routes, pages and libs; removes legacy Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant App as Action / Route
participant S3Buckets
participant Bucket as S3BucketAccess
participant S3 as AWS S3
Client->>App: request (upload / delete / presign / get)
App->>S3Buckets: S3Buckets.getBucketAccess(bucketId | userId)
S3Buckets-->>App: Effect builder
rect rgba(200,240,220,0.12)
note right of App: run Effect.gen { use S3Buckets } → runPromise
App->>Bucket: putObject / getObject / listObjects / deleteObjects / getPresigned...
Bucket->>S3: execute AWS command (uses bucket.bucketName)
S3-->>Bucket: bytes / URL / list / ack
Bucket-->>App: Option/Result
end
App-->>Client: response (url / status / data)
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
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (6)apps/web/actions/**/*.ts📄 CodeRabbit inference engine (CLAUDE.md)
Files:
apps/web/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx,js,jsx,rs}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/web/**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧬 Code graph analysis (1)apps/web/actions/videos/download.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 (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 |
![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: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/actions/organization/delete-space.ts (2)
36-41: Ensure space belongs to the user’s active organizationThe query only filters by spaceId; it should also scope to user.activeOrganizationId to prevent cross‑org deletion.
Apply this diff:
- const space = await db() - .select() - .from(spaces) - .where(eq(spaces.id, spaceId)) - .limit(1); + const space = await db() + .select() + .from(spaces) + .where( + and(eq(spaces.id, spaceId), eq(spaces.organizationId, user.activeOrganizationId)), + ) + .limit(1);Add import:
-import { eq } from "drizzle-orm"; +import { and, eq } from "drizzle-orm";
49-57: Authorization check is too restrictiveComment says creators or org owners can delete, but code only checks creator. Query org ownership or membership (role "owner") and allow either.
I can propose a concise query against organizationMembers/organizations to allow owners. Want me to draft it?
🧹 Nitpick comments (16)
packages/database/schema.ts (2)
374-383: Same S3BucketId reference concern on primary keyMirror the earlier note for the PK typing; prefer the exported S3BucketId alias if the nested type is not present.
Apply similarly:
- id: nanoId("id").notNull().primaryKey().unique().$type<S3Bucket.S3BucketId>(), + id: nanoId("id").notNull().primaryKey().unique().$type<S3BucketId>(),If needed, update imports:
-import type { Folder, S3Bucket, Video } from "@cap/web-domain"; +import type { Folder, Video } from "@cap/web-domain"; +import type { S3BucketId } from "@cap/web-domain";
90-92: Align users.customBucket to the branded S3BucketIdFor consistency and stronger type-safety, brand users.customBucket as S3BucketId | null.
- customBucket: nanoIdNullable("customBucket"), + customBucket: nanoIdNullable("customBucket").$type<S3BucketId | null>(),apps/web/app/api/desktop/[...route]/video.ts (1)
226-247: Delete flow: paginate and consider operation ordering
- S3 listObjectsV2 may paginate; handle ContinuationToken to delete all objects.
- Consider deleting storage first (best-effort) before removing the DB row, or wrap with compensating logic to avoid orphaned blobs on failure.
- const listedObjects = yield* bucket.listObjects({ - prefix: `${user.id}/${videoId}/`, - }); - - if (listedObjects.Contents?.length) - yield* bucket.deleteObjects( - listedObjects.Contents.map((content: any) => ({ - Key: content.Key, - })), - ); + let token: string | undefined = undefined; + do { + const page = yield* bucket.listObjects({ + prefix: `${user.id}/${videoId}/`, + maxKeys: 1000, + continuationToken: token, + } as any); + const toDelete = + page.Contents?.map((c: any) => ({ Key: c.Key })) ?? []; + if (toDelete.length) { + yield* bucket.deleteObjects(toDelete); + } + token = (page as any).NextContinuationToken; + } while ((page as any).IsTruncated);If you prefer DB-first delete, consider enqueueing an async cleanup task or logging the prefix to reconcile later on failure.
apps/web/actions/organization/delete-space.ts (2)
61-69: Wrap DB deletes in a transactionMultiple dependent deletes should be atomic to avoid partial removal on failure.
I can provide a drizzle transaction wrapper for these statements. Proceed?
99-100: Remove inline comments per repo guidelinesThis file adds/changes inline comments; policy says no inline/block/doc comments in code.
apps/web/actions/organization/upload-organization-icon.ts (1)
76-88: Remove inline comments per repo guidelinesNewly added comment lines violate the “no comments” rule.
apps/web/actions/videos/get-transcript.ts (1)
53-56: Avoid shadowing imported s3Buckets symbolLocal const s3Buckets shadows the imported table alias, hurting readability.
Apply this diff:
- const s3Buckets = yield* S3Buckets; - const [S3ProviderLayer] = yield* s3Buckets.getProviderForBucket( + const s3 = yield* S3Buckets; + const [S3ProviderLayer] = yield* s3.getProviderForBucket( Option.fromNullable(result.bucket?.id), );apps/web/lib/transcribe.ts (1)
76-82: Avoid shadowing imported s3Buckets table nameRename local service var for clarity.
Apply this diff:
-const [S3ProviderLayer] = await Effect.gen(function* () { - const s3Buckets = yield* S3Buckets; - return yield* s3Buckets.getProviderForBucket( +const [S3ProviderLayer] = await Effect.gen(function* () { + const s3 = yield* S3Buckets; + return yield* s3.getProviderForBucket( Option.fromNullable(result.bucket?.id), ); }).pipe(runPromise);apps/web/components/forms/server.ts (2)
71-99: Remove inline comments per repo guidelinesThis block adds inline comments; policy forbids them.
52-63: Optional: Sanitize icon file like other upload pathupload-organization-icon sanitizes the image before upload; mirror that for consistency.
I can draft a small helper to reuse sanitizeFile here. Interested?
apps/web/actions/organization/update-space.ts (1)
53-71: Consider resolving provider by user to honor custom bucketsIf icons may reside in per-user buckets, prefer
getProviderForUser(user.id)over the default provider.Apply this diff:
- const [S3ProviderLayer] = yield* s3Buckets.getProviderForBucket( - Option.none(), - ); + const [S3ProviderLayer] = yield* s3Buckets.getProviderForUser( + user.id, + );apps/web/actions/organization/upload-space-icon.ts (1)
57-61: Prefer provider resolution by user instead of defaultThis ensures deletes/uploads target the correct (possibly custom) bucket.
Apply this diff:
- const [S3ProviderLayer] = await Effect.gen(function* () { - const s3Buckets = yield* S3Buckets; - return yield* s3Buckets.getProviderForBucket(Option.none()); - }).pipe(runPromise); + const [S3ProviderLayer] = await Effect.gen(function* () { + const s3Buckets = yield* S3Buckets; + return yield* s3Buckets.getProviderForUser(user.id); + }).pipe(runPromise);apps/web/actions/organization/create-space.ts (1)
94-99: Resolve provider by user to support custom bucketsDefault provider may delete/upload to the wrong bucket if the org/user has a custom bucket configured.
Apply this diff:
- const [S3ProviderLayer] = yield* s3Buckets.getProviderForBucket( - Option.none(), - ); + const [S3ProviderLayer] = yield* s3Buckets.getProviderForUser( + user.id, + );apps/web/actions/videos/generate-ai-metadata.ts (1)
124-136: Avoid name shadowing: rename local s3Buckets bindingThe local
const s3Buckets = yield* S3Buckets;shadows the importeds3Bucketstable and hurts readability.- const s3Buckets = yield* S3Buckets; - const [S3ProviderLayer] = yield* s3Buckets.getProviderForBucket( + const buckets = yield* S3Buckets; + const [S3ProviderLayer] = yield* buckets.getProviderForBucket( Option.fromNullable(row.bucket?.id), );apps/web/actions/videos/get-og-image.tsx (2)
68-69: Type screenshotUrl to avoid implicit anyInitialize with an explicit union to satisfy strict TS and avoid implicit any.
- let screenshotUrl = null; + let screenshotUrl: string | null = null;
71-82: Avoid name shadowing: rename local s3Buckets bindingThe local
s3Bucketsvalue from the service shadows the imported table name in this module.- const s3Buckets = yield* S3Buckets; - const [S3ProviderLayer] = yield* s3Buckets.getProviderForBucket( + const buckets = yield* S3Buckets; + const [S3ProviderLayer] = yield* buckets.getProviderForBucket( Option.fromNullable(videoData.bucket?.id), );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
apps/web/actions/organization/create-space.ts(2 hunks)apps/web/actions/organization/delete-space.ts(2 hunks)apps/web/actions/organization/update-space.ts(2 hunks)apps/web/actions/organization/upload-organization-icon.ts(2 hunks)apps/web/actions/organization/upload-space-icon.ts(3 hunks)apps/web/actions/video/upload.ts(2 hunks)apps/web/actions/videos/download.ts(2 hunks)apps/web/actions/videos/edit-transcript.ts(2 hunks)apps/web/actions/videos/generate-ai-metadata.ts(2 hunks)apps/web/actions/videos/get-og-image.tsx(2 hunks)apps/web/actions/videos/get-transcript.ts(2 hunks)apps/web/app/api/desktop/[...route]/video.ts(2 hunks)apps/web/app/api/thumbnail/route.ts(3 hunks)apps/web/app/api/upload/[...route]/multipart.ts(5 hunks)apps/web/app/api/upload/[...route]/signed.ts(3 hunks)apps/web/app/embed/[videoId]/page.tsx(0 hunks)apps/web/app/layout.tsx(0 hunks)apps/web/app/s/[videoId]/page.tsx(0 hunks)apps/web/components/forms/server.ts(2 hunks)apps/web/lib/server.ts(1 hunks)apps/web/lib/transcribe.ts(3 hunks)apps/web/utils/public-env.tsx(0 hunks)apps/web/utils/s3.ts(0 hunks)packages/database/schema.ts(3 hunks)
💤 Files with no reviewable changes (5)
- apps/web/app/s/[videoId]/page.tsx
- apps/web/app/layout.tsx
- apps/web/utils/s3.ts
- apps/web/utils/public-env.tsx
- apps/web/app/embed/[videoId]/page.tsx
🧰 Additional context used
📓 Path-based instructions (8)
apps/web/actions/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
All Groq/OpenAI calls must be implemented in Next.js Server Actions under apps/web/actions; do not place AI calls elsewhere
Files:
apps/web/actions/organization/delete-space.tsapps/web/actions/videos/get-transcript.tsapps/web/actions/organization/create-space.tsapps/web/actions/video/upload.tsapps/web/actions/videos/download.tsapps/web/actions/organization/upload-organization-icon.tsapps/web/actions/organization/update-space.tsapps/web/actions/videos/generate-ai-metadata.tsapps/web/actions/videos/edit-transcript.tsapps/web/actions/organization/upload-space-icon.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/actions/organization/delete-space.tsapps/web/actions/videos/get-transcript.tsapps/web/lib/transcribe.tsapps/web/actions/organization/create-space.tsapps/web/app/api/thumbnail/route.tsapps/web/actions/video/upload.tsapps/web/actions/videos/get-og-image.tsxapps/web/actions/videos/download.tsapps/web/components/forms/server.tsapps/web/actions/organization/upload-organization-icon.tsapps/web/app/api/upload/[...route]/multipart.tsapps/web/actions/organization/update-space.tsapps/web/actions/videos/generate-ai-metadata.tsapps/web/app/api/upload/[...route]/signed.tsapps/web/actions/videos/edit-transcript.tsapps/web/lib/server.tsapps/web/actions/organization/upload-space-icon.tsapps/web/app/api/desktop/[...route]/video.ts
**/*.{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:
apps/web/actions/organization/delete-space.tspackages/database/schema.tsapps/web/actions/videos/get-transcript.tsapps/web/lib/transcribe.tsapps/web/actions/organization/create-space.tsapps/web/app/api/thumbnail/route.tsapps/web/actions/video/upload.tsapps/web/actions/videos/get-og-image.tsxapps/web/actions/videos/download.tsapps/web/components/forms/server.tsapps/web/actions/organization/upload-organization-icon.tsapps/web/app/api/upload/[...route]/multipart.tsapps/web/actions/organization/update-space.tsapps/web/actions/videos/generate-ai-metadata.tsapps/web/app/api/upload/[...route]/signed.tsapps/web/actions/videos/edit-transcript.tsapps/web/lib/server.tsapps/web/actions/organization/upload-space-icon.tsapps/web/app/api/desktop/[...route]/video.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:
apps/web/actions/organization/delete-space.tspackages/database/schema.tsapps/web/actions/videos/get-transcript.tsapps/web/lib/transcribe.tsapps/web/actions/organization/create-space.tsapps/web/app/api/thumbnail/route.tsapps/web/actions/video/upload.tsapps/web/actions/videos/get-og-image.tsxapps/web/actions/videos/download.tsapps/web/components/forms/server.tsapps/web/actions/organization/upload-organization-icon.tsapps/web/app/api/upload/[...route]/multipart.tsapps/web/actions/organization/update-space.tsapps/web/actions/videos/generate-ai-metadata.tsapps/web/app/api/upload/[...route]/signed.tsapps/web/actions/videos/edit-transcript.tsapps/web/lib/server.tsapps/web/actions/organization/upload-space-icon.tsapps/web/app/api/desktop/[...route]/video.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/actions/organization/delete-space.tspackages/database/schema.tsapps/web/actions/videos/get-transcript.tsapps/web/lib/transcribe.tsapps/web/actions/organization/create-space.tsapps/web/app/api/thumbnail/route.tsapps/web/actions/video/upload.tsapps/web/actions/videos/get-og-image.tsxapps/web/actions/videos/download.tsapps/web/components/forms/server.tsapps/web/actions/organization/upload-organization-icon.tsapps/web/app/api/upload/[...route]/multipart.tsapps/web/actions/organization/update-space.tsapps/web/actions/videos/generate-ai-metadata.tsapps/web/app/api/upload/[...route]/signed.tsapps/web/actions/videos/edit-transcript.tsapps/web/lib/server.tsapps/web/actions/organization/upload-space-icon.tsapps/web/app/api/desktop/[...route]/video.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/actions/organization/delete-space.tsapps/web/actions/videos/get-transcript.tsapps/web/lib/transcribe.tsapps/web/actions/organization/create-space.tsapps/web/app/api/thumbnail/route.tsapps/web/actions/video/upload.tsapps/web/actions/videos/get-og-image.tsxapps/web/actions/videos/download.tsapps/web/components/forms/server.tsapps/web/actions/organization/upload-organization-icon.tsapps/web/app/api/upload/[...route]/multipart.tsapps/web/actions/organization/update-space.tsapps/web/actions/videos/generate-ai-metadata.tsapps/web/app/api/upload/[...route]/signed.tsapps/web/actions/videos/edit-transcript.tsapps/web/lib/server.tsapps/web/actions/organization/upload-space-icon.tsapps/web/app/api/desktop/[...route]/video.ts
apps/web/app/api/**/route.ts
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/app/api/**/route.ts: Place API routes only under apps/web/app/api and implement each route in a route.ts file
Construct API routes with @effect/platform HttpApi/HttpApiBuilder and export only the handler from apiToHandler(ApiLive)
Map domain errors to transport errors with HttpApiError.* and keep error translation exhaustive
Use HttpAuthMiddleware for required auth and provideOptionalAuth for guest routes; avoid duplicate session lookups
Provide dependencies via Layer.provide in API routes instead of manual provideService calls
Files:
apps/web/app/api/thumbnail/route.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/api/thumbnail/route.tsapps/web/app/api/upload/[...route]/multipart.tsapps/web/app/api/upload/[...route]/signed.tsapps/web/app/api/desktop/[...route]/video.ts
🧠 Learnings (1)
📚 Learning: 2025-09-22T14:17:47.380Z
Learnt from: CR
PR: CapSoftware/Cap#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-22T14:17:47.380Z
Learning: Applies to apps/web/app/api/**/route.ts : Provide dependencies via Layer.provide in API routes instead of manual provideService calls
Applied to files:
apps/web/lib/server.ts
🧬 Code graph analysis (19)
apps/web/actions/organization/delete-space.ts (2)
packages/database/schema.ts (1)
s3Buckets(373-383)apps/web/lib/server.ts (1)
runPromise(56-71)
packages/database/schema.ts (1)
packages/web-domain/src/S3Bucket.ts (3)
S3Bucket(6-14)S3BucketId(3-3)S3BucketId(4-4)
apps/web/actions/videos/get-transcript.ts (2)
packages/database/schema.ts (1)
s3Buckets(373-383)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/lib/transcribe.ts (3)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-195)apps/web/lib/server.ts (1)
runPromise(56-71)packages/web-backend/src/S3Buckets/S3BucketAccess.ts (1)
S3BucketAccess(47-299)
apps/web/actions/organization/create-space.ts (3)
packages/database/schema.ts (1)
s3Buckets(373-383)packages/env/server.ts (1)
serverEnv(83-87)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/app/api/thumbnail/route.ts (2)
packages/database/schema.ts (1)
s3Buckets(373-383)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/actions/video/upload.ts (2)
packages/database/schema.ts (1)
s3Buckets(373-383)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/actions/videos/get-og-image.tsx (2)
packages/database/schema.ts (1)
s3Buckets(373-383)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/actions/videos/download.ts (2)
packages/database/schema.ts (1)
s3Buckets(373-383)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/components/forms/server.ts (2)
packages/env/server.ts (1)
serverEnv(83-87)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/actions/organization/upload-organization-icon.ts (2)
packages/database/schema.ts (1)
s3Buckets(373-383)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/app/api/upload/[...route]/multipart.ts (4)
packages/database/schema.ts (1)
s3Buckets(373-383)packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-195)packages/web-backend/src/S3Buckets/S3BucketAccess.ts (1)
S3BucketAccess(47-299)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/actions/organization/update-space.ts (3)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-195)packages/web-backend/src/S3Buckets/S3BucketAccess.ts (1)
S3BucketAccess(47-299)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/actions/videos/generate-ai-metadata.ts (1)
apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/app/api/upload/[...route]/signed.ts (2)
packages/database/schema.ts (1)
s3Buckets(373-383)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/actions/videos/edit-transcript.ts (1)
apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/lib/server.ts (1)
packages/web-backend/src/Database.ts (1)
Database(9-19)
apps/web/actions/organization/upload-space-icon.ts (3)
packages/database/schema.ts (1)
s3Buckets(373-383)apps/web/lib/server.ts (1)
runPromise(56-71)packages/env/server.ts (1)
serverEnv(83-87)
apps/web/app/api/desktop/[...route]/video.ts (3)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-195)packages/web-backend/src/S3Buckets/S3BucketAccess.ts (1)
S3BucketAccess(47-299)apps/web/lib/server.ts (1)
runPromise(56-71)
🪛 GitHub Check: CodeQL
apps/web/actions/video/upload.ts
[failure] 127-127: Incomplete URL substring sanitization
'amazonaws.com' can be anywhere in the URL, and arbitrary hosts may come before or after it.
🪛 GitHub Check: Typecheck
apps/web/app/api/upload/[...route]/multipart.ts
[failure] 254-254:
Property 'name' does not exist on type 'S3BucketAccess'.
⏰ 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 (rust)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (15)
apps/web/lib/server.ts (1)
47-51: Dependency layering LGTMMerging Database.Default with tracing and fetch inside provideMerge is correct and will satisfy S3Buckets/Videos deps during composition.
apps/web/app/api/thumbnail/route.ts (2)
65-71: Provider resolution pattern looks goodAcquiring the S3 provider via S3Buckets and wiring the Layer for subsequent S3BucketAccess is correct.
72-76: Effect usage for list/signing is correct and scopedUsing Effect.provide(S3ProviderLayer) for both list and signed URL ops is sound.
Ensure apps/web/lib/server.ts Dependencies includes S3Buckets.Default (it does) so S3Buckets resolves in this route context.
Also applies to: 95-99
packages/database/schema.ts (1)
248-249: S3Bucket.S3BucketId is correct – no change needed
S3Bucket.S3BucketIdrefers to the branded type exported under theS3Bucketnamespace; there is no top-levelS3BucketIdexport to use instead.Likely an incorrect or invalid review comment.
apps/web/actions/organization/delete-space.ts (1)
74-76: Confirm correct bucket selectionUsing Option.none() forces default bucket. If spaces can be stored in custom buckets, pass the space’s bucket id.
Would you like a follow‑up patch to accept an optional bucketId here?
apps/web/actions/videos/get-transcript.ts (1)
65-72: LGTM: Option-aware read and result handlingCorrectly returns 404-ish message when object Option is None and unwraps when present.
apps/web/lib/transcribe.ts (2)
86-90: LGTM: Presigned URL via provider layerProvider provisioning and usage are correct here.
131-139: LGTM: Upload transcript via effectful S3 accessProperly writes VTT with contentType and provided layer.
apps/web/components/forms/server.ts (1)
73-76: Confirm correct bucket selectionUsing Option.none() forces default bucket. If orgs may have custom buckets, pass org’s bucket id.
apps/web/actions/organization/update-space.ts (1)
7-13: Imports and runtime wiring look goodEffect-based S3 provider usage and
runPromiseintegration are correct.apps/web/actions/organization/create-space.ts (1)
8-14: Imports and runtime wiring look goodEffect-based S3 provider,
Effect,Option, andrunPromiseintegration are correct.apps/web/app/api/upload/[...route]/signed.ts (2)
118-157: LGTM: Effect-based provider flow and presign logicProvider acquisition, scoping via
Effect.provide, and presign calls look correct.
194-195: Response branching looks goodReturning distinct keys for post/put keeps the API clear.
apps/web/actions/video/upload.ts (1)
112-118: LGTM: Provider resolution and runtime usageAcquiring the provider via
S3Bucketsand running throughrunPromiseis correct.apps/web/app/api/upload/[...route]/multipart.ts (1)
52-77: Good: provider resolution and multipart initiation via Effect layerFetching the provider via S3Buckets and performing create inside the provided layer looks correct and resilient.
Repo-wide scan for direct AWS SDK S3 usages returned "No files were searched" in the sandbox. Run locally from the repo root and paste the output:
rg -n --hidden -S -C2 '(?:@aws-sdk/client-s3|S3Client|CreateMultipartUploadCommand|UploadPartCommand|CompleteMultipartUploadCommand|CopyObjectCommand|HeadObjectCommand|createMultipartUpload|new\s+AWS.S3|\baws-sdk\b|S3Buckets|bucket.multipart|S3Provider)' --glob '!/node_modules/'
apps/web/actions/video/upload.ts
Outdated
| const presignedPostData = await Effect.gen(function* () { | ||
| const bucket = yield* S3BucketAccess; | ||
| const presignedPostData = yield* bucket.getPresignedPostUrl(fileKey, { | ||
| Fields, | ||
| Expires: 1800, | ||
| }); | ||
|
|
||
| const customEndpoint = serverEnv().CAP_AWS_ENDPOINT; | ||
| if (customEndpoint && !customEndpoint.includes("amazonaws.com")) { | ||
| if (serverEnv().S3_PATH_STYLE) { | ||
| presignedPostData.url = `${customEndpoint}/${bucket.bucketName}`; | ||
| } else { | ||
| presignedPostData.url = customEndpoint; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return presignedPostData; | ||
| }).pipe(Effect.provide(S3ProviderLayer), runPromise); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Fix: using Effect value (bucketName) as string; also re-check endpoint logic
bucket.bucketNameis an Effect; you mustyield*it before using.- Optional: parsing/validating
CAP_AWS_ENDPOINTvia URL API is safer than substring checks.
Apply this diff:
const presignedPostData = await Effect.gen(function* () {
const bucket = yield* S3BucketAccess;
+ const bucketName = yield* bucket.bucketName;
const presignedPostData = yield* bucket.getPresignedPostUrl(fileKey, {
Fields,
Expires: 1800,
});
const customEndpoint = serverEnv().CAP_AWS_ENDPOINT;
if (customEndpoint && !customEndpoint.includes("amazonaws.com")) {
if (serverEnv().S3_PATH_STYLE) {
- presignedPostData.url = `${customEndpoint}/${bucket.bucketName}`;
+ presignedPostData.url = `${customEndpoint}/${bucketName}`;
} else {
presignedPostData.url = customEndpoint;
}
}
return presignedPostData;
}).pipe(Effect.provide(S3ProviderLayer), runPromise);Optionally verify other occurrences of direct bucket.bucketName usage:
🏁 Script executed:
#!/bin/bash
rg -n -C2 --type=ts '\bbucket\.bucketName\b'Length of output: 6467
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- upload.ts (around the relevant region) ----"
nl -ba apps/web/actions/video/upload.ts | sed -n '1,220p' || true
echo
echo "---- Search for S3BucketAccess definition/usage ----"
rg -n --hidden --no-ignore -S 'S3BucketAccess' || true
echo
echo "---- Show files that define `bucketName` fields/types ----"
rg -n --hidden --no-ignore -S 'bucketName' -g '!node_modules' -g '!**/dist/**' -C2 | sed -n '1,200p' || true
echo
echo "---- Occurrences of `bucket.bucketName` with context ----"
rg -n --hidden --no-ignore -S '\bbucket\.bucketName\b' -C3 -g '!node_modules' -g '!**/dist/**' || trueLength of output: 6035
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- apps/web/actions/video/upload.ts (1-240) ----"
sed -n '1,240p' apps/web/actions/video/upload.ts || true
echo
echo "---- packages/web-backend/src/S3Buckets/S3BucketAccess.ts (1-240) ----"
sed -n '1,240p' packages/web-backend/src/S3Buckets/S3BucketAccess.ts || true
echo
echo "---- packages/web-backend/src/S3Buckets/index.ts (1-240) ----"
sed -n '1,240p' packages/web-backend/src/S3Buckets/index.ts || true
echo
echo "---- packages/web-backend/src/S3Buckets/S3BucketsRepo.ts (1-240) ----"
sed -n '1,240p' packages/web-backend/src/S3Buckets/S3BucketsRepo.ts || true
echo
echo "---- Locate S3Bucket type/codec ----"
rg -n --hidden --no-ignore -S 'export (type|const|interface).*S3Bucket|S3Bucket.decodeSync' || trueLength of output: 21416
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files that both yield S3BucketAccess and reference bucket.bucketName:"
files=$(rg -l -S 'const bucket = yield\* S3BucketAccess' || true)
for f in $files; do
if rg -q -S 'bucket\.bucketName' "$f"; then
echo "---- $f ----"
rg -n -C2 'const bucket = yield\* S3BucketAccess|bucket\.bucketName' "$f" || true
echo
fi
doneLength of output: 4324
Fix: yield the Effect (bucketName) before using; validate custom endpoint*
- S3BucketAccess.bucketName is an Effect — inside Effect.gen do
const bucketName = yield* bucket.bucketNameand usebucketNamewhen building URLs. Prefer parsing/validating CAP_AWS_ENDPOINT with the URL API instead of substring checks.
Apply this diff for apps/web/actions/video/upload.ts:
const presignedPostData = await Effect.gen(function* () {
const bucket = yield* S3BucketAccess;
+ const bucketName = yield* bucket.bucketName;
const presignedPostData = yield* bucket.getPresignedPostUrl(fileKey, {
Fields,
Expires: 1800,
});
const customEndpoint = serverEnv().CAP_AWS_ENDPOINT;
if (customEndpoint && !customEndpoint.includes("amazonaws.com")) {
if (serverEnv().S3_PATH_STYLE) {
- presignedPostData.url = `${customEndpoint}/${bucket.bucketName}`;
+ presignedPostData.url = `${customEndpoint}/${bucketName}`;
} else {
presignedPostData.url = customEndpoint;
}
}
return presignedPostData;
}).pipe(Effect.provide(S3ProviderLayer), runPromise);Also fix other direct bucket.bucketName usages found in the repo (yield the Effect before use):
- apps/web/components/forms/server.ts
- apps/web/actions/video/upload.ts (above)
- apps/web/actions/organization/create-space.ts
- apps/web/actions/organization/upload-organization-icon.ts
- apps/web/actions/organization/upload-space-icon.ts
- apps/web/app/api/upload/[...route]/multipart.ts
📝 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 presignedPostData = await Effect.gen(function* () { | |
| const bucket = yield* S3BucketAccess; | |
| const presignedPostData = yield* bucket.getPresignedPostUrl(fileKey, { | |
| Fields, | |
| Expires: 1800, | |
| }); | |
| const customEndpoint = serverEnv().CAP_AWS_ENDPOINT; | |
| if (customEndpoint && !customEndpoint.includes("amazonaws.com")) { | |
| if (serverEnv().S3_PATH_STYLE) { | |
| presignedPostData.url = `${customEndpoint}/${bucket.bucketName}`; | |
| } else { | |
| presignedPostData.url = customEndpoint; | |
| } | |
| } | |
| } | |
| return presignedPostData; | |
| }).pipe(Effect.provide(S3ProviderLayer), runPromise); | |
| const presignedPostData = await Effect.gen(function* () { | |
| const bucket = yield* S3BucketAccess; | |
| const bucketName = yield* bucket.bucketName; | |
| const presignedPostData = yield* bucket.getPresignedPostUrl(fileKey, { | |
| Fields, | |
| Expires: 1800, | |
| }); | |
| const customEndpoint = serverEnv().CAP_AWS_ENDPOINT; | |
| if (customEndpoint && !customEndpoint.includes("amazonaws.com")) { | |
| if (serverEnv().S3_PATH_STYLE) { | |
| presignedPostData.url = `${customEndpoint}/${bucketName}`; | |
| } else { | |
| presignedPostData.url = customEndpoint; | |
| } | |
| } | |
| return presignedPostData; | |
| }).pipe(Effect.provide(S3ProviderLayer), runPromise); |
🧰 Tools
🪛 GitHub Check: CodeQL
[failure] 127-127: Incomplete URL substring sanitization
'amazonaws.com' can be anywhere in the URL, and arbitrary hosts may come before or after it.
🤖 Prompt for AI Agents
In apps/web/actions/video/upload.ts around lines 119 to 137, inside the
Effect.gen you are using bucket.bucketName (an Effect) directly when building
custom presigned URLs and using a substring check on CAP_AWS_ENDPOINT; instead
yield* the bucket.bucketName (e.g., const bucketName = yield* bucket.bucketName)
and use that variable when composing URLs, and replace the naive
includes("amazonaws.com") check by parsing serverEnv().CAP_AWS_ENDPOINT with the
URL API (new URL(...)) to validate host and scheme and to safely build either
`${endpoint}/${bucketName}` for path-style or just the endpoint for
virtual-hosted style; apply the same yield* bucket.bucketName fix to the other
listed files (apps/web/components/forms/server.ts,
apps/web/actions/organization/create-space.ts,
apps/web/actions/organization/upload-organization-icon.ts,
apps/web/actions/organization/upload-space-icon.ts,
apps/web/app/api/upload/[...route]/multipart.ts).
apps/web/components/forms/server.ts
Outdated
| // Construct the icon URL | ||
| if (serverEnv().CAP_AWS_BUCKET_URL) { | ||
| // If a custom bucket URL is defined, use it | ||
| iconUrl = `${serverEnv().CAP_AWS_BUCKET_URL}/${fileKey}`; | ||
| } else if (serverEnv().CAP_AWS_ENDPOINT) { | ||
| // For custom endpoints like MinIO | ||
| iconUrl = `${serverEnv().CAP_AWS_ENDPOINT}/${bucket.bucketName}/${fileKey}`; | ||
| } else { | ||
| // Default AWS S3 URL format | ||
| iconUrl = `https://${bucket.bucketName}.s3.${ | ||
| serverEnv().CAP_AWS_REGION || "us-east-1" | ||
| }.amazonaws.com/${fileKey}`; | ||
| } |
There was a problem hiding this comment.
Bug: bucket.bucketName must be yielded before string interpolation
bucketName is an Effect; using it directly will break.
Apply this diff:
- // Construct the icon URL
- if (serverEnv().CAP_AWS_BUCKET_URL) {
- // If a custom bucket URL is defined, use it
- iconUrl = `${serverEnv().CAP_AWS_BUCKET_URL}/${fileKey}`;
- } else if (serverEnv().CAP_AWS_ENDPOINT) {
- // For custom endpoints like MinIO
- iconUrl = `${serverEnv().CAP_AWS_ENDPOINT}/${bucket.bucketName}/${fileKey}`;
- } else {
- // Default AWS S3 URL format
- iconUrl = `https://${bucket.bucketName}.s3.${
- serverEnv().CAP_AWS_REGION || "us-east-1"
- }.amazonaws.com/${fileKey}`;
- }
+ const bucketName = yield* bucket.bucketName;
+ if (serverEnv().CAP_AWS_BUCKET_URL) {
+ iconUrl = `${serverEnv().CAP_AWS_BUCKET_URL}/${fileKey}`;
+ } else if (serverEnv().CAP_AWS_ENDPOINT) {
+ iconUrl = `${serverEnv().CAP_AWS_ENDPOINT}/${bucketName}/${fileKey}`;
+ } else {
+ iconUrl = `https://${bucketName}.s3.${
+ serverEnv().CAP_AWS_REGION || "us-east-1"
+ }.amazonaws.com/${fileKey}`;
+ }📝 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.
| // Construct the icon URL | |
| if (serverEnv().CAP_AWS_BUCKET_URL) { | |
| // If a custom bucket URL is defined, use it | |
| iconUrl = `${serverEnv().CAP_AWS_BUCKET_URL}/${fileKey}`; | |
| } else if (serverEnv().CAP_AWS_ENDPOINT) { | |
| // For custom endpoints like MinIO | |
| iconUrl = `${serverEnv().CAP_AWS_ENDPOINT}/${bucket.bucketName}/${fileKey}`; | |
| } else { | |
| // Default AWS S3 URL format | |
| iconUrl = `https://${bucket.bucketName}.s3.${ | |
| serverEnv().CAP_AWS_REGION || "us-east-1" | |
| }.amazonaws.com/${fileKey}`; | |
| } | |
| // Construct the icon URL | |
| const bucketName = yield* bucket.bucketName; | |
| if (serverEnv().CAP_AWS_BUCKET_URL) { | |
| // If a custom bucket URL is defined, use it | |
| iconUrl = `${serverEnv().CAP_AWS_BUCKET_URL}/${fileKey}`; | |
| } else if (serverEnv().CAP_AWS_ENDPOINT) { | |
| // For custom endpoints like MinIO | |
| iconUrl = `${serverEnv().CAP_AWS_ENDPOINT}/${bucketName}/${fileKey}`; | |
| } else { | |
| // Default AWS S3 URL format | |
| iconUrl = `https://${bucketName}.s3.${ | |
| serverEnv().CAP_AWS_REGION || "us-east-1" | |
| }.amazonaws.com/${fileKey}`; | |
| } |
🤖 Prompt for AI Agents
In apps/web/components/forms/server.ts around lines 85 to 97, the code
interpolates bucket.bucketName directly even though bucketName is an Effect;
resolve/yield the Effect to a string first (e.g., const bucketName = await
bucket.bucketName or use the appropriate Effect.run/await mechanism used in this
codebase) and then use bucketName in the iconUrl template
(`${serverEnv().CAP_AWS_ENDPOINT}/${bucketName}/${fileKey}`); ensure any other
uses within this block similarly await/resolve Effects before string
interpolation.
There was a problem hiding this comment.
@coderabbitai bucketName is not an effect
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for S3BucketAccess interface/type definition to understand bucketName property
ast-grep --pattern 'interface S3BucketAccess {
$$$
}'
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Also search with rg for S3BucketAccess definitions
rg -nP --type=ts -C5 'interface.*S3BucketAccess|type.*S3BucketAccess|class.*S3BucketAccess'
Length of output: 978
🏁 Script executed:
#!/bin/bash
# Search for bucketName property usage in the codebase to see how it's accessed
rg -nP --type=ts -C3 '\.bucketName'
Length of output: 12277
@Brendonovich, I respectfully disagree. Based on the codebase evidence, bucketName is an Effect that needs to be yielded.
Looking at the S3BucketAccess class definition in packages/web-backend/src/S3Buckets/S3BucketAccess.ts:
bucketName: Effect.map(S3BucketClientProvider, (p) => p.bucket),And other parts of the codebase correctly handle it as an Effect, like in packages/web-backend/src/Videos/index.ts:
const s3 = yield* S3BucketAccess;
const bucketName = yield* s3.bucketName;The current code in this file is indeed incorrectly using bucket.bucketName directly in string interpolation without yielding it first. My original suggestion to add const bucketName = yield* bucket.bucketName; before the URL construction is correct.
✏ Learnings added
Learnt from: Brendonovich
PR: CapSoftware/Cap#1060
File: apps/web/components/forms/server.ts:85-97
Timestamp: 2025-09-24T07:24:21.429Z
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.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/web/actions/videos/get-og-image.tsx (2)
64-64: TypescreenshotUrlto avoid implicitany.Prevents type widening and aligns with strict TS guidelines.
- let screenshotUrl = null; + let screenshotUrl: string | null = null;
67-77: Avoid shadowing the importeds3Bucketstable; rename the local service variable.Local
const s3Buckets = yield* S3Buckets;shadows thes3BucketsDB import — rename for clarity. Confirmed:getProviderForBucketreturns the provider layer as the first tuple element (index 0), so destructuring [S3ProviderLayer] is correct.- const s3Buckets = yield* S3Buckets; - const [S3ProviderLayer] = yield* s3Buckets.getProviderForBucket( + const s3Registry = yield* S3Buckets; + const [S3ProviderLayer] = yield* s3Registry.getProviderForBucket( Option.fromNullable(videoData.bucket?.id), );apps/web/app/api/upload/[...route]/multipart.ts (2)
5-5: Remove unused import: DatabaseDatabase is not used in this module.
Apply this diff:
-import { Database, S3BucketAccess, S3Buckets } from "@cap/web-backend"; +import { S3BucketAccess, S3Buckets } from "@cap/web-backend";
43-47: Avoid shadowing s3Buckets (DB table) with local variable nameRename the service binding to reduce confusion with the imported s3Buckets table name.
Apply these diffs in the respective sections:
- const s3Buckets = yield* S3Buckets; - const [S3ProviderLayer] = yield* s3Buckets.getProviderForUser( + const s3BucketsService = yield* S3Buckets; + const [S3ProviderLayer] = yield* s3BucketsService.getProviderForUser( user.id, );- const s3Buckets = yield* S3Buckets; - const [S3ProviderLayer] = yield* s3Buckets.getProviderForUser( + const s3BucketsService = yield* S3Buckets; + const [S3ProviderLayer] = yield* s3BucketsService.getProviderForUser( user.id, );- const s3Buckets = yield* S3Buckets; - return yield* s3Buckets.getProviderForUser(user.id); + const s3BucketsService = yield* S3Buckets; + return yield* s3BucketsService.getProviderForUser(user.id);Also applies to: 132-136, 216-220
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/actions/videos/get-og-image.tsx(2 hunks)apps/web/app/api/desktop/[...route]/s3Config.ts(2 hunks)apps/web/app/api/upload/[...route]/multipart.ts(5 hunks)apps/web/app/embed/[videoId]/page.tsx(1 hunks)apps/web/app/s/[videoId]/page.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/app/embed/[videoId]/page.tsx
- apps/web/app/s/[videoId]/page.tsx
🧰 Additional context used
📓 Path-based instructions (6)
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/api/desktop/[...route]/s3Config.tsapps/web/actions/videos/get-og-image.tsxapps/web/app/api/upload/[...route]/multipart.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/api/desktop/[...route]/s3Config.tsapps/web/app/api/upload/[...route]/multipart.ts
**/*.{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:
apps/web/app/api/desktop/[...route]/s3Config.tsapps/web/actions/videos/get-og-image.tsxapps/web/app/api/upload/[...route]/multipart.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:
apps/web/app/api/desktop/[...route]/s3Config.tsapps/web/actions/videos/get-og-image.tsxapps/web/app/api/upload/[...route]/multipart.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/api/desktop/[...route]/s3Config.tsapps/web/actions/videos/get-og-image.tsxapps/web/app/api/upload/[...route]/multipart.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/api/desktop/[...route]/s3Config.tsapps/web/actions/videos/get-og-image.tsxapps/web/app/api/upload/[...route]/multipart.ts
🧬 Code graph analysis (3)
apps/web/app/api/desktop/[...route]/s3Config.ts (2)
packages/web-domain/src/S3Bucket.ts (1)
S3Bucket(6-14)packages/database/helpers.ts (1)
nanoId(6-9)
apps/web/actions/videos/get-og-image.tsx (2)
packages/database/schema.ts (1)
s3Buckets(373-383)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/app/api/upload/[...route]/multipart.ts (3)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-195)packages/web-backend/src/S3Buckets/S3BucketAccess.ts (1)
S3BucketAccess(47-299)apps/web/lib/server.ts (1)
runPromise(56-71)
🔇 Additional comments (5)
apps/web/actions/videos/get-og-image.tsx (1)
3-3: Effect-based S3 provider wiring/imports look good.Imports align with the new provider model and server-side runPromise usage.
Also applies to: 6-6, 8-8
apps/web/app/api/desktop/[...route]/s3Config.ts (1)
35-35: Typed S3BucketId for DB id looks goodUsing S3Bucket.S3BucketId.make(nanoId()) improves type-safety for the bucket id.
Confirm s3Buckets.id in the Drizzle schema is branded as S3BucketId (or is compatible) to avoid cast friction in future queries/joins.
apps/web/app/api/upload/[...route]/multipart.ts (3)
56-64: LGTM on Effect-based S3 flow and validations
- Correctly defers to S3ProviderLayer and S3BucketAccess.
- Good UploadId presence check and structured CompleteMultipart call.
Also applies to: 65-77, 140-152, 231-237, 245-249, 264-271
250-261: Fix confirmed: use bucket.bucketName (not bucket.name)The logged payload now uses bucket.bucketName, aligning with S3BucketAccess types.
281-314: Remove inline disable comments; handle errors with Effect combinatorsInline comments are disallowed by repo guidelines, and try/catch inside Effect.gen can be replaced with Effect.matchEffect.
Apply this diff:
await Effect.gen(function* () { const bucket = yield* S3BucketAccess; - - // @effect-diagnostics-next-line tryCatchInEffectGen:off - try { - console.log( - "Performing metadata fix by copying the object to itself...", - ); - - const copyResult = yield* bucket.copyObject( - `${bucket.bucketName}/${fileKey}`, - fileKey, - { ContentType: "video/mp4", MetadataDirective: "REPLACE" }, - ); - - console.log("Copy for metadata fix successful:", copyResult); - } catch (copyError) { - console.error( - "Warning: Failed to copy object to fix metadata:", - copyError, - ); - } - - // @effect-diagnostics-next-line tryCatchInEffectGen:off - try { - const headResult = yield* bucket.headObject(fileKey); - console.log( - `Object verification successful: ContentType=${headResult.ContentType}, ContentLength=${headResult.ContentLength}`, - ); - } catch (headError) { - console.error(`Warning: Unable to verify object: ${headError}`); - } + yield* Effect.matchEffect( + bucket.copyObject( + `${bucket.bucketName}/${fileKey}`, + fileKey, + { ContentType: "video/mp4", MetadataDirective: "REPLACE" }, + ), + (copyError) => + Effect.sync(() => + console.error("Warning: Failed to copy object to fix metadata:", copyError), + ), + (copyResult) => + Effect.sync(() => + console.log("Copy for metadata fix successful:", copyResult), + ), + ); + + yield* Effect.matchEffect( + bucket.headObject(fileKey), + (headError) => + Effect.sync(() => + console.error(`Warning: Unable to verify object: ${headError}`), + ), + (headResult) => + Effect.sync(() => + console.log( + `Object verification successful: ContentType=${headResult.ContentType}, ContentLength=${headResult.ContentLength}`, + ), + ), + ); }).pipe(Effect.provide(S3ProviderLayer), runPromise);
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/actions/videos/edit-transcript.ts (1)
68-69: Consider extracting Option check to avoid nested return.The Option check is correct, but returning from within the try block can make error handling harder to follow.
Consider moving the Option check outside the try block for clearer control flow:
- try { - const transcriptKey = `${video.ownerId}/${videoId}/transcription.vtt`; - - const vttContent = await Effect.gen(function* () { - const bucket = yield* S3BucketAccess; - return yield* bucket.getObject(transcriptKey); - }).pipe(Effect.provide(S3ProviderLayer), runPromise); - if (Option.isNone(vttContent)) - return { success: false, message: "Transcript file not found" }; + const transcriptKey = `${video.ownerId}/${videoId}/transcription.vtt`; + + const vttContent = await Effect.gen(function* () { + const bucket = yield* S3BucketAccess; + return yield* bucket.getObject(transcriptKey); + }).pipe(Effect.provide(S3ProviderLayer), runPromise); + + if (Option.isNone(vttContent)) + return { success: false, message: "Transcript file not found" }; + + try {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/actions/videos/edit-transcript.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/actions/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
All Groq/OpenAI calls must be implemented in Next.js Server Actions under apps/web/actions; do not place AI calls elsewhere
Files:
apps/web/actions/videos/edit-transcript.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/actions/videos/edit-transcript.ts
**/*.{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:
apps/web/actions/videos/edit-transcript.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:
apps/web/actions/videos/edit-transcript.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/actions/videos/edit-transcript.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/actions/videos/edit-transcript.ts
🧬 Code graph analysis (1)
apps/web/actions/videos/edit-transcript.ts (1)
apps/web/lib/server.ts (1)
runPromise(56-71)
⏰ 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 (3)
apps/web/actions/videos/edit-transcript.ts (3)
55-60: LGTM! Bucket selection correctly implemented.The code properly uses
Option.fromNullable(result.bucket?.id)to get the provider for the video's specific bucket, addressing the previous concern about using the wrong bucket for custom bucket users.
64-67: Effect-based S3 access pattern looks good.The migration to Effect-based S3 access with proper provider layer injection is clean and follows the established pattern across the codebase.
77-82: Clean Effect-based write operation.The putObject implementation correctly uses the Effect pattern with proper provider layer and maintains the correct content type for VTT files.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 3
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/api/thumbnail/route.ts (1)
64-94: Block public access to private thumbnailsThe route returns a signed URL without verifying access. Anyone with
videoId(and a guesseduserId) can retrieve private thumbnails. Enforce that either the video is public or the requester is the owner (or authenticated per policy). Minimal patch below; ideal is to move to HttpApi with auth middleware and policy checks.Apply this diff inside GET after loading
resultand before bucket ops:const result = query[0]; if (!result?.video) { return new Response( JSON.stringify({ error: true, message: "Video not found" }), { - status: 401, + status: 404, headers: getHeaders(origin), }, ); } + // Restrict access to private videos + if (result.video.public === false) { + // If you want owner access, use session instead of trusting query userId + const currentUser = await import("@cap/database/auth/session").then((m) => + m.getCurrentUser(), + ); + if (!currentUser || currentUser.id !== result.video.ownerId) { + return new Response( + JSON.stringify({ error: true, message: "Forbidden" }), + { status: 403, headers: getHeaders(origin) }, + ); + } + }Additionally, prefer HttpApi + HttpAuthMiddleware for consistent auth and error mapping in
apps/web/app/api/**/route.ts.
🧹 Nitpick comments (22)
apps/web/actions/videos/get-og-image.tsx (1)
138-150: Optional: add a brief alt for thetag
OG rendering doesn’t need it functionally, but providing a short alt string can help tooling and future reuse.
- <img + <img + alt="Video screenshot" style={{ width: "100%",apps/web/app/api/thumbnail/route.ts (2)
41-49: Use 404 for “not found” cases, not 401These branches represent missing resources, not authentication failures.
- { - status: 401, + { + status: 404, headers: getHeaders(origin), },Also applies to: 53-60
74-76: Avoid any; type the ListObjects contentsKeep TS strict by typing the item shape.
- const thumbnailKey = contents.find((item: any) => + const thumbnailKey = contents.find((item: { Key?: string }) => item.Key?.endsWith("screen-capture.jpg"), )?.Key;packages/web-backend/src/Videos/index.ts (2)
42-60: Deletion ordering can leave orphaned S3 objects on failureDB delete happens before S3 cleanup. If S3 calls fail, you lose reference and leak storage. Consider deleting S3 first (best-effort) and then the DB row, or adopt a background cleanup job/lifecycle rule.
52-60: ListObjects may be truncatedSingle call may not list >1K keys. If large uploads exist, add pagination or a helper that lists all keys under prefix before deletion.
apps/web/actions/organization/update-space.ts (1)
53-61: Parse the icon URL and extract the S3 key from the pathname; apply to both delete sites
- Replace the brittle full-URL regex with URL parsing + pathname match, e.g.:
try { const u = new URL(space.iconUrl); const key = u.pathname.match(/organizations/.+/)?.[0] ?? null; } catch { key = null; }- Apply the change in:
- apps/web/actions/organization/update-space.ts (existing delete)
- apps/web/actions/organization/upload-space-icon.ts (previous-icon delete)
- Verified uploads use the same bucket access (S3Buckets.getBucketAccess(Option.none()) in upload-space-icon.ts, create-space.ts, upload-organization-icon.ts and apps/web/components/forms/server.ts) — deleting with Option.none() is consistent.
apps/web/app/api/desktop/[...route]/video.ts (1)
226-242: Harden delete: filter out undefined Keys before deleteObjectsS3 ListObjectsV2 may include entries with undefined Key. Filter first to avoid errors.
Apply this diff:
- const listedObjects = yield* bucket.listObjects({ + const listedObjects = yield* bucket.listObjects({ prefix: `${user.id}/${videoId}/`, }); - if (listedObjects.Contents?.length) - yield* bucket.deleteObjects( - listedObjects.Contents.map((content: any) => ({ - Key: content.Key, - })), - ); + const objects = (listedObjects.Contents ?? []) + .filter((c) => !!c.Key) + .map((c) => ({ Key: c.Key! })); + if (objects.length > 0) { + yield* bucket.deleteObjects(objects); + }apps/web/app/api/upload/[...route]/multipart.ts (3)
217-226: Fail early on non-sequential part numbers (400 Bad Request).S3 requires parts to be numbered consecutively starting at 1 for CompleteMultipartUpload. Logging a warning but proceeding risks a hard-to-debug S3 failure. Return a 400 with a clear message when validation fails.
- if (!sequentialCheck) { - console.warn( - "WARNING: Part numbers are not sequential! This may cause issues with the assembled file.", - ); - } + if (!sequentialCheck) { + return Effect.sync(() => { + throw Object.assign(new Error("Part numbers must be sequential starting at 1"), { + status: 400, + }); + }); + }And in the outer handler’s catch, map a thrown
status: 400toc.json(..., 400)if you want a client error response rather than 500.
246-253: Surface S3 completion errors with more context.Wrap
bucket.multipart.completewith contextual logging on failure; right now the outer catch re-wraps generically as “S3 operation failed”.- const result = yield* bucket.multipart.complete(fileKey, uploadId, { + const result = yield* bucket.multipart.complete(fileKey, uploadId, { MultipartUpload: { Parts: formattedParts, }, - }); + }).pipe( + Effect.tapError((e) => + Effect.logError("S3 CompleteMultipartUpload failed", { + uploadId, + fileKey, + partsCount: formattedParts.length, + error: String(e), + }), + ), + );
49-56: Carry through client-provided content type when fixing metadata.You default to “video/mp4” at initiation, but at completion you hardcode “video/mp4” in the self-copy metadata fix. If the upload isn’t MP4, this mismatches. Consider deriving content type (e.g., from request body or object metadata) and using that consistently.
apps/web/actions/videos/edit-transcript.ts (1)
62-77: Avoid writing when nothing changed; inform the caller instead.If the target cue isn’t found,
updateVttEntryreturns the original content and you still write and return success. Compare before/after and short‑circuit.- const updatedVttContent = updateVttEntry( + const updatedVttContent = updateVttEntry( vttContent.value, entryId, newText, ); - await bucket + if (updatedVttContent === vttContent.value) { + return { success: false, message: "Transcript entry not found or unchanged" }; + } + + await bucket .putObject(transcriptKey, updatedVttContent, { contentType: "text/vtt", }) .pipe(runPromise);apps/web/app/api/playlist/route.ts (1)
170-198: Guard against missing objects when generating master playlist.If
videoSegment.Contents?.[0]?.Keyis undefined,headObject("")may throw. Consider short‑circuiting with NotFound when no segments are listed.apps/web/app/api/upload/[...route]/signed.ts (3)
116-151: Eliminate non-null assertion by returning a discriminated result from the Effect.
let data: PresignedPost;plusdata!relies on control-flow that TS can’t track. Return a tagged result from the Effect and branch afterwards.- let data: PresignedPost; - - await Effect.gen(function* () { + const result = await Effect.gen(function* () { const [bucket] = yield* S3Buckets.getBucketAccess( Option.fromNullable(customBucket?.id), ); if (method === "post") { const Fields = { "Content-Type": contentType, "x-amz-meta-userid": user.id, "x-amz-meta-duration": durationInSecs ? durationInSecs.toString() : "", }; - data = yield* bucket.getPresignedPostUrl(fileKey, { + const data = yield* bucket.getPresignedPostUrl(fileKey, { Fields, Expires: 1800, }); - } else if (method === "put") { + return { kind: "post" as const, data }; + } else { const presignedUrl = yield* bucket.getPresignedPutUrl( fileKey, { ContentType: contentType, Metadata: { userid: user.id, duration: durationInSecs ? durationInSecs.toString() : "", }, }, { expiresIn: 1800 }, ); - data = { url: presignedUrl, fields: {} }; + return { kind: "put" as const, data: { url: presignedUrl, fields: {} } }; } - }).pipe(runPromise); + }).pipe(runPromise);
189-191: Use the discriminated result to shape the response without!.- if (method === "post") return c.json({ presignedPostData: data! }); - else return c.json({ presignedPutData: data! }); + if (result.kind === "post") return c.json({ presignedPostData: result.data }); + return c.json({ presignedPutData: result.data });
70-101: CloudFront invalidation likely triggers for custom buckets erroneously.The condition invalidates when a custom bucket name differs from the default (
!==). You typically want to invalidate only for the default bucket, i.e., when there’s no custom bucket or it equals the default.- if ( - !customBucket || - !s3Config || - customBucket.bucketName !== serverEnv().CAP_AWS_BUCKET - ) { + if (!customBucket || customBucket.bucketName === serverEnv().CAP_AWS_BUCKET) {packages/web-backend/src/Loom/ImportVideo.ts (1)
119-131: Guard against unknown content length.If
contentLengthis undefined, some S3 backends require it. Either compute it (HEAD first) or omit when unsupported; at least log and proceed with a sane default behavior.packages/web-backend/src/S3Buckets/index.ts (4)
57-78: Parallelize decryption to reduce latency and fix forcePathStyle logic for AWS endpoints.
- Decrypt fields concurrently to avoid 3 sequential awaits.
- Path-style should generally be disabled for AWS (
s3.amazonaws.com), enabled for custom endpoints.Apply:
-const createBucketClient = async (bucket: S3Bucket.S3Bucket) => { - const endpoint = await (() => { - const v = bucket.endpoint.pipe(Option.getOrUndefined); - if (!v) return; - return decrypt(v); - })(); - - return new S3.S3Client({ - endpoint, - region: await decrypt(bucket.region), - credentials: { - accessKeyId: await decrypt(bucket.accessKeyId), - secretAccessKey: await decrypt(bucket.secretAccessKey), - }, - forcePathStyle: - Option.fromNullable(endpoint).pipe( - Option.map((e) => e.endsWith("s3.amazonaws.com")), - Option.getOrNull, - ) ?? true, - useArnRegion: false, - }); -}; +const createBucketClient = async (bucket: S3Bucket.S3Bucket) => { + const endpointRaw = Option.getOrUndefined(bucket.endpoint); + const [endpoint, region, accessKeyId, secretAccessKey] = await Promise.all([ + endpointRaw ? decrypt(endpointRaw) : Promise.resolve<string | undefined>(undefined), + decrypt(bucket.region), + decrypt(bucket.accessKeyId), + decrypt(bucket.secretAccessKey), + ]); + + return new S3.S3Client({ + endpoint, + region, + credentials: { accessKeyId, secretAccessKey }, + forcePathStyle: + Option.fromNullable(endpoint).pipe( + Option.map((e) => !e.endsWith("s3.amazonaws.com")), + Option.getOrNull, + ) ?? true, + useArnRegion: false, + }); +};
128-131: Rename effect label for clarity.The label still references “ProviderLayer”. Align to the new API.
-const getBucketAccess = Effect.fn("S3Buckets.getProviderLayer")(function* ( +const getBucketAccess = Effect.fn("S3Buckets.getBucketAccessInternal")(function* (
177-185: Align effect label with method name.Keep tracing consistent.
- getBucketAccessForUser: Effect.fn("S3Buckets.getProviderForUser")( + getBucketAccessForUser: Effect.fn("S3Buckets.getBucketAccessForUser")(
9-9: Remove unused Database dependency.
Databaseisn’t used directly; it’s already a dependency ofS3BucketsRepo. Avoid over-declaring.Apply:
-import { Database } from "../Database.ts";- dependencies: [S3BucketsRepo.Default, Database.Default], + dependencies: [S3BucketsRepo.Default],Also applies to: 188-189
packages/web-backend/src/S3Buckets/S3BucketAccess.ts (2)
68-75: Handle NoSuchKey robustly (SDK v3 error shapes).
instanceof S3.NoSuchKeycan miss 404s. Check name and httpStatusCode.- .catch((e) => { - if (e instanceof S3.NoSuchKey) { - return null; - } else { - throw e; - } - }); + .catch((e) => { + const err = e as { name?: string; $metadata?: { httpStatusCode?: number } }; + if (err?.name === "NoSuchKey" || err?.$metadata?.httpStatusCode === 404) { + return null; + } + throw e; + });
151-155: Build CopySource for same‑bucket copies.Footgun: callers currently must prepend the bucket. Compose it here.
- new S3.CopyObjectCommand({ - Bucket: provider.bucket, - CopySource: source, - Key: key, - ...args, - }), + new S3.CopyObjectCommand({ + Bucket: provider.bucket, + CopySource: `/${provider.bucket}/${source}`, + Key: key, + ...args, + }),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
apps/web/actions/organization/create-space.ts(2 hunks)apps/web/actions/organization/delete-space.ts(2 hunks)apps/web/actions/organization/update-space.ts(2 hunks)apps/web/actions/organization/upload-organization-icon.ts(2 hunks)apps/web/actions/organization/upload-space-icon.ts(3 hunks)apps/web/actions/video/upload.ts(2 hunks)apps/web/actions/videos/download.ts(2 hunks)apps/web/actions/videos/edit-transcript.ts(2 hunks)apps/web/actions/videos/generate-ai-metadata.ts(2 hunks)apps/web/actions/videos/get-og-image.tsx(2 hunks)apps/web/actions/videos/get-transcript.ts(2 hunks)apps/web/app/api/desktop/[...route]/video.ts(2 hunks)apps/web/app/api/playlist/route.ts(4 hunks)apps/web/app/api/thumbnail/route.ts(3 hunks)apps/web/app/api/upload/[...route]/multipart.ts(5 hunks)apps/web/app/api/upload/[...route]/signed.ts(3 hunks)apps/web/components/forms/server.ts(2 hunks)apps/web/lib/transcribe.ts(3 hunks)packages/web-backend/src/Loom/ImportVideo.ts(3 hunks)packages/web-backend/src/S3Buckets/S3BucketAccess.ts(2 hunks)packages/web-backend/src/S3Buckets/index.ts(8 hunks)packages/web-backend/src/Videos/index.ts(2 hunks)packages/web-backend/src/index.ts(0 hunks)
💤 Files with no reviewable changes (1)
- packages/web-backend/src/index.ts
🧰 Additional context used
📓 Path-based instructions (8)
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/api/upload/[...route]/multipart.tsapps/web/actions/organization/upload-organization-icon.tsapps/web/lib/transcribe.tsapps/web/actions/videos/download.tsapps/web/app/api/upload/[...route]/signed.tsapps/web/app/api/desktop/[...route]/video.tsapps/web/components/forms/server.tsapps/web/actions/videos/generate-ai-metadata.tsapps/web/actions/video/upload.tsapps/web/app/api/playlist/route.tsapps/web/app/api/thumbnail/route.tsapps/web/actions/organization/update-space.tsapps/web/actions/organization/upload-space-icon.tsapps/web/actions/videos/get-transcript.tsapps/web/actions/organization/create-space.tsapps/web/actions/videos/get-og-image.tsxapps/web/actions/videos/edit-transcript.tsapps/web/actions/organization/delete-space.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/api/upload/[...route]/multipart.tsapps/web/app/api/upload/[...route]/signed.tsapps/web/app/api/desktop/[...route]/video.tsapps/web/app/api/playlist/route.tsapps/web/app/api/thumbnail/route.ts
**/*.{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:
apps/web/app/api/upload/[...route]/multipart.tsapps/web/actions/organization/upload-organization-icon.tsapps/web/lib/transcribe.tsapps/web/actions/videos/download.tsapps/web/app/api/upload/[...route]/signed.tsapps/web/app/api/desktop/[...route]/video.tspackages/web-backend/src/S3Buckets/index.tsapps/web/components/forms/server.tsapps/web/actions/videos/generate-ai-metadata.tspackages/web-backend/src/S3Buckets/S3BucketAccess.tspackages/web-backend/src/Videos/index.tsapps/web/actions/video/upload.tsapps/web/app/api/playlist/route.tsapps/web/app/api/thumbnail/route.tsapps/web/actions/organization/update-space.tsapps/web/actions/organization/upload-space-icon.tsapps/web/actions/videos/get-transcript.tsapps/web/actions/organization/create-space.tspackages/web-backend/src/Loom/ImportVideo.tsapps/web/actions/videos/get-og-image.tsxapps/web/actions/videos/edit-transcript.tsapps/web/actions/organization/delete-space.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:
apps/web/app/api/upload/[...route]/multipart.tsapps/web/actions/organization/upload-organization-icon.tsapps/web/lib/transcribe.tsapps/web/actions/videos/download.tsapps/web/app/api/upload/[...route]/signed.tsapps/web/app/api/desktop/[...route]/video.tspackages/web-backend/src/S3Buckets/index.tsapps/web/components/forms/server.tsapps/web/actions/videos/generate-ai-metadata.tspackages/web-backend/src/S3Buckets/S3BucketAccess.tspackages/web-backend/src/Videos/index.tsapps/web/actions/video/upload.tsapps/web/app/api/playlist/route.tsapps/web/app/api/thumbnail/route.tsapps/web/actions/organization/update-space.tsapps/web/actions/organization/upload-space-icon.tsapps/web/actions/videos/get-transcript.tsapps/web/actions/organization/create-space.tspackages/web-backend/src/Loom/ImportVideo.tsapps/web/actions/videos/get-og-image.tsxapps/web/actions/videos/edit-transcript.tsapps/web/actions/organization/delete-space.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/api/upload/[...route]/multipart.tsapps/web/actions/organization/upload-organization-icon.tsapps/web/lib/transcribe.tsapps/web/actions/videos/download.tsapps/web/app/api/upload/[...route]/signed.tsapps/web/app/api/desktop/[...route]/video.tspackages/web-backend/src/S3Buckets/index.tsapps/web/components/forms/server.tsapps/web/actions/videos/generate-ai-metadata.tspackages/web-backend/src/S3Buckets/S3BucketAccess.tspackages/web-backend/src/Videos/index.tsapps/web/actions/video/upload.tsapps/web/app/api/playlist/route.tsapps/web/app/api/thumbnail/route.tsapps/web/actions/organization/update-space.tsapps/web/actions/organization/upload-space-icon.tsapps/web/actions/videos/get-transcript.tsapps/web/actions/organization/create-space.tspackages/web-backend/src/Loom/ImportVideo.tsapps/web/actions/videos/get-og-image.tsxapps/web/actions/videos/edit-transcript.tsapps/web/actions/organization/delete-space.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/api/upload/[...route]/multipart.tsapps/web/actions/organization/upload-organization-icon.tsapps/web/lib/transcribe.tsapps/web/actions/videos/download.tsapps/web/app/api/upload/[...route]/signed.tsapps/web/app/api/desktop/[...route]/video.tsapps/web/components/forms/server.tsapps/web/actions/videos/generate-ai-metadata.tsapps/web/actions/video/upload.tsapps/web/app/api/playlist/route.tsapps/web/app/api/thumbnail/route.tsapps/web/actions/organization/update-space.tsapps/web/actions/organization/upload-space-icon.tsapps/web/actions/videos/get-transcript.tsapps/web/actions/organization/create-space.tsapps/web/actions/videos/get-og-image.tsxapps/web/actions/videos/edit-transcript.tsapps/web/actions/organization/delete-space.ts
apps/web/actions/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
All Groq/OpenAI calls must be implemented in Next.js Server Actions under apps/web/actions; do not place AI calls elsewhere
Files:
apps/web/actions/organization/upload-organization-icon.tsapps/web/actions/videos/download.tsapps/web/actions/videos/generate-ai-metadata.tsapps/web/actions/video/upload.tsapps/web/actions/organization/update-space.tsapps/web/actions/organization/upload-space-icon.tsapps/web/actions/videos/get-transcript.tsapps/web/actions/organization/create-space.tsapps/web/actions/videos/edit-transcript.tsapps/web/actions/organization/delete-space.ts
apps/web/app/api/**/route.ts
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/app/api/**/route.ts: Place API routes only under apps/web/app/api and implement each route in a route.ts file
Construct API routes with @effect/platform HttpApi/HttpApiBuilder and export only the handler from apiToHandler(ApiLive)
Map domain errors to transport errors with HttpApiError.* and keep error translation exhaustive
Use HttpAuthMiddleware for required auth and provideOptionalAuth for guest routes; avoid duplicate session lookups
Provide dependencies via Layer.provide in API routes instead of manual provideService calls
Files:
apps/web/app/api/playlist/route.tsapps/web/app/api/thumbnail/route.ts
🧠 Learnings (1)
📚 Learning: 2025-09-24T07:24:21.429Z
Learnt from: Brendonovich
PR: CapSoftware/Cap#1060
File: apps/web/components/forms/server.ts:85-97
Timestamp: 2025-09-24T07:24:21.429Z
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:
apps/web/actions/organization/upload-organization-icon.tspackages/web-backend/src/S3Buckets/index.tsapps/web/components/forms/server.tspackages/web-backend/src/S3Buckets/S3BucketAccess.tsapps/web/actions/video/upload.tsapps/web/actions/organization/upload-space-icon.tsapps/web/actions/organization/create-space.ts
🧬 Code graph analysis (22)
apps/web/app/api/upload/[...route]/multipart.ts (2)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/actions/organization/upload-organization-icon.ts (3)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)packages/env/server.ts (1)
serverEnv(83-87)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/lib/transcribe.ts (2)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/actions/videos/download.ts (2)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/app/api/upload/[...route]/signed.ts (3)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)packages/web-backend/src/index.ts (1)
S3Buckets(6-6)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/app/api/desktop/[...route]/video.ts (2)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)apps/web/lib/server.ts (1)
runPromise(56-71)
packages/web-backend/src/S3Buckets/index.ts (4)
packages/web-backend/src/S3Buckets/S3BucketAccess.ts (1)
createS3BucketAccess(40-290)packages/web-domain/src/S3Bucket.ts (3)
S3Bucket(6-14)S3BucketId(3-3)S3BucketId(4-4)packages/web-backend/src/S3Buckets/S3BucketsRepo.ts (1)
S3BucketsRepo(8-73)packages/web-backend/src/Database.ts (1)
Database(9-19)
apps/web/components/forms/server.ts (2)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/actions/videos/generate-ai-metadata.ts (2)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)apps/web/lib/server.ts (1)
runPromise(56-71)
packages/web-backend/src/S3Buckets/S3BucketAccess.ts (1)
packages/web-backend/src/S3Buckets/S3BucketClientProvider.ts (1)
S3BucketClientProvider(4-13)
packages/web-backend/src/Videos/index.ts (3)
packages/database/schema.ts (1)
s3Buckets(373-383)packages/web-domain/src/Policy.ts (2)
Policy(7-11)policy(27-39)packages/web-domain/src/Authentication.ts (1)
CurrentUser(7-10)
apps/web/actions/video/upload.ts (2)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/app/api/playlist/route.ts (1)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)
apps/web/app/api/thumbnail/route.ts (3)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)packages/web-backend/src/index.ts (1)
S3Buckets(6-6)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/actions/organization/update-space.ts (2)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/actions/organization/upload-space-icon.ts (2)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/actions/videos/get-transcript.ts (2)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/actions/organization/create-space.ts (2)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)apps/web/lib/server.ts (1)
runPromise(56-71)
packages/web-backend/src/Loom/ImportVideo.ts (1)
packages/database/schema.ts (1)
s3Buckets(373-383)
apps/web/actions/videos/get-og-image.tsx (2)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/actions/videos/edit-transcript.ts (2)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/actions/organization/delete-space.ts (2)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)apps/web/lib/server.ts (1)
runPromise(56-71)
⏰ 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/actions/videos/get-og-image.tsx (1)
71-77: S3 access migration looks correctEffect-based bucket resolution and URL signing via
runPromiseis idiomatic and matchesS3Buckets.getBucketAccess’s tuple shape. No changes needed.packages/web-backend/src/Videos/index.ts (1)
77-99: Copy loop is correct; concurrency=1 prevents throttlingUsing
Option.fromNullable(obj.Key)and embeddingbucket.bucketNamein copy source aligns with S3 expectations. Looks good.If large videos have many objects, you might bump concurrency modestly (e.g., 4–8) and add exponential backoff on
SlowDownerrors. Confirm with production metrics before changing.apps/web/actions/videos/generate-ai-metadata.ts (2)
124-131: Bucket access + object retrieval flow looks goodEffect-based access and
runPromiseusage are consistent with the new S3 abstraction.
132-143: getObject already returns a string — no TextDecoder neededS3BucketAccess.getObject awaits resp.Body?.transformToString() (returns Promise) and wraps it with Option.fromNullable, so vtt.value is a string.
apps/web/actions/organization/delete-space.ts (1)
72-91: Harden delete: filter out undefined Keys; log actual countAvoid passing undefined Keys to deleteObjects and report the real count.
Apply this diff:
- if (listedObjects.Contents?.length) { - yield* bucket.deleteObjects( - listedObjects.Contents.map((content) => ({ - Key: content.Key, - })), - ); - - console.log( - `Deleted ${listedObjects.Contents.length} objects for space ${spaceId}`, - ); - } + const objects = (listedObjects.Contents ?? []) + .filter((c) => !!c.Key) + .map((c) => ({ Key: c.Key! })); + if (objects.length > 0) { + yield* bucket.deleteObjects(objects); + console.log(`Deleted ${objects.length} objects for space ${spaceId}`); + }apps/web/actions/organization/upload-space-icon.ts (2)
78-85: Fix: don’t pass an Effect as the body to putObjectputObject expects bytes, not an Effect. Await the bytes and pass the value.
Apply this diff:
- await bucket - .putObject( - fileKey, - Effect.promise(() => sanitizedFile.bytes()), - { contentType: file.type }, - ) - .pipe(runPromise); + await bucket + .putObject( + fileKey, + await sanitizedFile.bytes(), + { contentType: file.type }, + ) + .pipe(runPromise);
86-97: Fix: bucket.bucketName is effectful; resolve it before interpolationResolve bucketName via runPromise, then use it in URLs.
Apply this diff:
- let iconUrl: string | undefined; + let iconUrl: string | undefined; + const bucketName = await bucket.bucketName.pipe(runPromise); // Construct the icon URL if (serverEnv().CAP_AWS_BUCKET_URL) { iconUrl = `${serverEnv().CAP_AWS_BUCKET_URL}/${fileKey}`; } else if (serverEnv().CAP_AWS_ENDPOINT) { - iconUrl = `${serverEnv().CAP_AWS_ENDPOINT}/${bucket.bucketName}/${fileKey}`; + iconUrl = `${serverEnv().CAP_AWS_ENDPOINT}/${bucketName}/${fileKey}`; } else { - iconUrl = `https://${bucket.bucketName}.s3.${ + iconUrl = `https://${bucketName}.s3.${ serverEnv().CAP_AWS_REGION || "us-east-1" }.amazonaws.com/${fileKey}`; }apps/web/actions/organization/upload-organization-icon.ts (1)
63-84: Yield bucketName and remove inline comments per repo guidelines
- bucket.bucketName must be yielded inside Effect.gen.
- Inline comments violate repo’s “no comments” rule.
Apply this diff:
await Effect.gen(function* () { const [bucket] = yield* S3Buckets.getBucketAccess(Option.none()); yield* bucket.putObject( fileKey, yield* Effect.promise(() => sanitizedFile.bytes()), { contentType: file.type }, ); - // Construct the icon URL - if (serverEnv().CAP_AWS_BUCKET_URL) { - // If a custom bucket URL is defined, use it - iconUrl = `${serverEnv().CAP_AWS_BUCKET_URL}/${fileKey}`; - } else if (serverEnv().CAP_AWS_ENDPOINT) { - // For custom endpoints like MinIO - iconUrl = `${serverEnv().CAP_AWS_ENDPOINT}/${bucket.bucketName}/${fileKey}`; - } else { - // Default AWS S3 URL format - iconUrl = `https://${bucket.bucketName}.s3.${ - serverEnv().CAP_AWS_REGION || "us-east-1" - }.amazonaws.com/${fileKey}`; - } + const bucketName = yield* bucket.bucketName; + if (serverEnv().CAP_AWS_BUCKET_URL) { + iconUrl = `${serverEnv().CAP_AWS_BUCKET_URL}/${fileKey}`; + } else if (serverEnv().CAP_AWS_ENDPOINT) { + iconUrl = `${serverEnv().CAP_AWS_ENDPOINT}/${bucketName}/${fileKey}`; + } else { + iconUrl = `https://${bucketName}.s3.${ + serverEnv().CAP_AWS_REGION || "us-east-1" + }.amazonaws.com/${fileKey}`; + } }).pipe(runPromise);apps/web/components/forms/server.ts (1)
71-93: Yield bucketName and strip comments
- Resolve bucketName via yield* before building URLs.
- Remove inline comments to comply with repo rules.
Apply this diff:
yield* bucket.putObject( fileKey, yield* Effect.promise(() => iconFile.bytes()), { contentType: iconFile.type }, ); - // Construct the icon URL - if (serverEnv().CAP_AWS_BUCKET_URL) { - // If a custom bucket URL is defined, use it - iconUrl = `${serverEnv().CAP_AWS_BUCKET_URL}/${fileKey}`; - } else if (serverEnv().CAP_AWS_ENDPOINT) { - // For custom endpoints like MinIO - iconUrl = `${serverEnv().CAP_AWS_ENDPOINT}/${bucket.bucketName}/${fileKey}`; - } else { - // Default AWS S3 URL format - iconUrl = `https://${bucket.bucketName}.s3.${ - serverEnv().CAP_AWS_REGION || "us-east-1" - }.amazonaws.com/${fileKey}`; - } + const bucketName = yield* bucket.bucketName; + if (serverEnv().CAP_AWS_BUCKET_URL) { + iconUrl = `${serverEnv().CAP_AWS_BUCKET_URL}/${fileKey}`; + } else if (serverEnv().CAP_AWS_ENDPOINT) { + iconUrl = `${serverEnv().CAP_AWS_ENDPOINT}/${bucketName}/${fileKey}`; + } else { + iconUrl = `https://${bucketName}.s3.${ + serverEnv().CAP_AWS_REGION || "us-east-1" + }.amazonaws.com/${fileKey}`; + }apps/web/lib/transcribe.ts (1)
76-79: S3Buckets migration here looks correctGood use of S3Buckets.getBucketAccess and running bucket ops via runPromise; URL signing and putObject are properly sequenced.
Also applies to: 83-83, 125-130
apps/web/actions/videos/get-transcript.ts (1)
52-60: LGTM: effect-based S3 access and Option handlingCorrectly obtains bucket access, fetches object, and handles Option result.
Also applies to: 62-62, 68-68
apps/web/actions/videos/download.ts (1)
38-44: Bug: missing return from inner Effect; downloadUrl will be undefinedReturn the signed URL from the inner generator.
Apply this diff:
- const downloadUrl = await Effect.gen(function* () { - const [bucket] = yield* S3Buckets.getBucketAccess( - Option.fromNullable(video.bucket), - ); - yield* bucket.getSignedObjectUrl(videoKey); - }).pipe(runPromise); + const downloadUrl = await Effect.gen(function* () { + const [bucket] = yield* S3Buckets.getBucketAccess( + Option.fromNullable(video.bucket), + ); + return yield* bucket.getSignedObjectUrl(videoKey); + }).pipe(runPromise);apps/web/actions/video/upload.ts (1)
112-133: Fix: yield bucketName (Effect) before string interpolation; harden endpoint checkbucket.bucketName is an Effect — must be yielded. Also replace brittle substring check with URL hostname check and normalize trailing slash.
Apply this diff:
- const presignedPostData = await Effect.gen(function* () { + const presignedPostData = await Effect.gen(function* () { const [bucket] = yield* S3Buckets.getBucketAccess( Option.fromNullable(customBucket?.id), ); - const presignedPostData = yield* bucket.getPresignedPostUrl(fileKey, { + const presignedPostData = yield* bucket.getPresignedPostUrl(fileKey, { Fields, Expires: 1800, }); - const customEndpoint = serverEnv().CAP_AWS_ENDPOINT; - if (customEndpoint && !customEndpoint.includes("amazonaws.com")) { - if (serverEnv().S3_PATH_STYLE) { - presignedPostData.url = `${customEndpoint}/${bucket.bucketName}`; - } else { - presignedPostData.url = customEndpoint; - } - } + const bucketName = yield* bucket.bucketName; + const customEndpoint = serverEnv().CAP_AWS_ENDPOINT; + if (customEndpoint) { + let u: URL | null = null; + try { + u = new URL(customEndpoint); + } catch {} + if (u && !u.hostname.endsWith("amazonaws.com")) { + if (serverEnv().S3_PATH_STYLE) { + presignedPostData.url = `${customEndpoint.replace(/\/$/, "")}/${bucketName}`; + } else { + presignedPostData.url = customEndpoint; + } + } + } return presignedPostData; - }).pipe(runPromise); + }).pipe(runPromise);apps/web/actions/organization/create-space.ts (1)
94-116: Fix: yield bucketName (Effect) and remove inline comments (repo policy)
- bucket.bucketName must be yielded before use.
- Remove inline comments per repository guidelines.
Apply this diff:
- await Effect.gen(function* () { + await Effect.gen(function* () { const [bucket] = yield* S3Buckets.getBucketAccess(Option.none()); yield* bucket.putObject( fileKey, yield* Effect.promise(() => iconFile.bytes()), { contentType: iconFile.type }, ); - // Construct the icon URL + const bucketName = yield* bucket.bucketName; if (serverEnv().CAP_AWS_BUCKET_URL) { - // If a custom bucket URL is defined, use it iconUrl = `${serverEnv().CAP_AWS_BUCKET_URL}/${fileKey}`; } else if (serverEnv().CAP_AWS_ENDPOINT) { - // For custom endpoints like MinIO - iconUrl = `${serverEnv().CAP_AWS_ENDPOINT}/${bucket.bucketName}/${fileKey}`; + iconUrl = `${serverEnv().CAP_AWS_ENDPOINT}/${bucketName}/${fileKey}`; } else { - // Default AWS S3 URL format - iconUrl = `https://${bucket.bucketName}.s3.${ + iconUrl = `https://${bucketName}.s3.${ serverEnv().CAP_AWS_REGION || "us-east-1" }.amazonaws.com/${fileKey}`; } - }).pipe(runPromise); + }).pipe(runPromise);apps/web/actions/videos/edit-transcript.ts (2)
55-57: Correct bucket selection for the video.Good: you now pass the video’s bucket id to S3Buckets.getBucketAccess, avoiding the default-bucket mistake for custom-bucket users.
Please verify that
videos.bucketreally stores the S3 bucket id type expected byS3Buckets.getBucketAccess. If there’s any chance it’s nullable, this call already handles it viaOption.fromNullable, which is correct.
72-76: No change required — putObject expects lowercase contentTypeS3BucketAccess.putObject accepts fields?: { contentType?: string; contentLength?: number } and sets ContentType: fields?.contentType on the S3.PutObjectCommand, so .putObject(..., { contentType: "text/vtt" }) is correct (see packages/web-backend/src/S3Buckets/S3BucketAccess.ts).
apps/web/app/api/playlist/route.ts (1)
82-95: Nice split between default vs custom bucket handling.Using
getBucketAccess(video.bucketId)and branching oncustomBucketis clean. Redirecting to a signed object URL for the default bucket keeps the fast path simple, while custom buckets generate playlists.packages/web-backend/src/Loom/ImportVideo.ts (1)
65-71: Good fix: resolve user’s bucket access instead of default.Switching to
getBucketAccessForUsercaptures custom-bucket routing correctly.packages/web-backend/src/S3Buckets/index.ts (2)
71-75: Double-check path‑style decision.If the endpoint ends with
s3.amazonaws.com, you likely wantforcePathStyle: false. The current logic sets it to true for AWS endpoints.
139-143: Verify Layer provisioning API.Confirm your Effect version supports
Effect.provide(providerLayer)directly. If not, useLayer.provideToor an equivalent helper.packages/web-backend/src/S3Buckets/S3BucketAccess.ts (2)
17-38: Confirm Promise detection approach.
instanceof Promisecan be brittle across realms/edge runtimes. Consider using anEffect.isEffectguard (if available in your version) rather than relying oninstanceof Promise.
40-44: No remainingyield* ... .bucketNameusages — change OKRepository search found multiple
.bucketNameoccurrences (e.g. apps/web/components/forms/server.ts, packages/web-backend/src/Videos/index.ts) but noyield* ... .bucketNamepatterns; callers already usebucket.bucketNamedirectly.
| const [s3Bucket] = yield* s3Buckets.getBucketAccess(customBucketId); | ||
|
|
||
| const resp = yield* http | ||
| .get(payload.loom.video.downloadUrl) | ||
| .pipe(Effect.catchAll((cause) => new LoomApiError({ cause }))); | ||
| const contentLength = Headers.get(resp.headers, "content-length").pipe( | ||
| Option.map((v) => Number(v)), | ||
| Option.getOrUndefined, | ||
| ); | ||
| yield* Effect.log(`Downloading ${contentLength} bytes`); | ||
|
|
There was a problem hiding this comment.
Propagate HTTP errors explicitly and validate status.
Currently a non-2xx response proceeds as success. Fail fast on bad status and wrap with LoomApiError.
- const resp = yield* http
- .get(payload.loom.video.downloadUrl)
- .pipe(Effect.catchAll((cause) => new LoomApiError({ cause })));
+ const resp = yield* http
+ .get(payload.loom.video.downloadUrl)
+ .pipe(
+ Effect.catchAll((cause) => Effect.fail(new LoomApiError({ cause }))),
+ );
+ if (resp.status < 200 || resp.status >= 300) {
+ yield* Effect.fail(new LoomApiError({ cause: `HTTP ${resp.status}` }));
+ }📝 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 [s3Bucket] = yield* s3Buckets.getBucketAccess(customBucketId); | |
| const resp = yield* http | |
| .get(payload.loom.video.downloadUrl) | |
| .pipe(Effect.catchAll((cause) => new LoomApiError({ cause }))); | |
| const contentLength = Headers.get(resp.headers, "content-length").pipe( | |
| Option.map((v) => Number(v)), | |
| Option.getOrUndefined, | |
| ); | |
| yield* Effect.log(`Downloading ${contentLength} bytes`); | |
| const [s3Bucket] = yield* s3Buckets.getBucketAccess(customBucketId); | |
| const resp = yield* http | |
| .get(payload.loom.video.downloadUrl) | |
| .pipe( | |
| Effect.catchAll((cause) => Effect.fail(new LoomApiError({ cause }))), | |
| ); | |
| if (resp.status < 200 || resp.status >= 300) { | |
| yield* Effect.fail(new LoomApiError({ cause: `HTTP ${resp.status}` })); | |
| } | |
| const contentLength = Headers.get(resp.headers, "content-length").pipe( | |
| Option.map((v) => Number(v)), | |
| Option.getOrUndefined, | |
| ); | |
| yield* Effect.log(`Downloading ${contentLength} bytes`); |
🤖 Prompt for AI Agents
In packages/web-backend/src/Loom/ImportVideo.ts around lines 104 to 114, the
HTTP GET currently treats any response as success; update the flow to validate
resp.status (ensure 2xx) and fail fast on non-2xx by converting the response
into a LoomApiError (including status, url and body or cause) instead of
proceeding; implement this by inspecting resp.status after the http.get Effect
resolves, returning the response on 2xx or yielding/raising an
Effect.fail/Effect.catchAll that wraps the error in new LoomApiError with
contextual details so downstream code does not treat errors as successful
downloads.
| Effect.repeat( | ||
| Effect.gen(function* () { | ||
| const bytes = yield* Effect.succeed(downloadedBytes); | ||
| yield* Effect.log(`Downloaded ${bytes} bytes`); | ||
| }), | ||
| ), | ||
| { contentLength }, | ||
| ) | ||
| .pipe( | ||
| Effect.race( | ||
| // TODO: Connect this with upload progress | ||
| Effect.repeat( | ||
| Effect.gen(function* () { | ||
| const bytes = yield* Effect.succeed(downloadedBytes); | ||
| yield* Effect.log(`Downloaded ${bytes} bytes`); | ||
| }), | ||
| Schedule.forever.pipe(Schedule.delayed(() => "2 seconds")), | ||
| ).pipe(Effect.delay("100 millis")), | ||
| ), | ||
| ); | ||
|
|
||
| yield* Effect.log( | ||
| `Uploaded video for user '${payload.cap.userId}' at key '${key}'`, | ||
| Schedule.forever.pipe(Schedule.delayed(() => "2 seconds")), | ||
| ).pipe(Effect.delay("100 millis")), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove inline comments per repo guidelines.
Inline comments are disallowed. Drop the TODO and keep logging via the schedule.
- // TODO: Connect this with upload progress
Effect.repeat(
Effect.gen(function* () {
const bytes = yield* Effect.succeed(downloadedBytes);
yield* Effect.log(`Downloaded ${bytes} bytes`);
}),
Schedule.forever.pipe(Schedule.delayed(() => "2 seconds")),📝 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.
| Effect.repeat( | |
| Effect.gen(function* () { | |
| const bytes = yield* Effect.succeed(downloadedBytes); | |
| yield* Effect.log(`Downloaded ${bytes} bytes`); | |
| }), | |
| ), | |
| { contentLength }, | |
| ) | |
| .pipe( | |
| Effect.race( | |
| // TODO: Connect this with upload progress | |
| Effect.repeat( | |
| Effect.gen(function* () { | |
| const bytes = yield* Effect.succeed(downloadedBytes); | |
| yield* Effect.log(`Downloaded ${bytes} bytes`); | |
| }), | |
| Schedule.forever.pipe(Schedule.delayed(() => "2 seconds")), | |
| ).pipe(Effect.delay("100 millis")), | |
| ), | |
| ); | |
| yield* Effect.log( | |
| `Uploaded video for user '${payload.cap.userId}' at key '${key}'`, | |
| Schedule.forever.pipe(Schedule.delayed(() => "2 seconds")), | |
| ).pipe(Effect.delay("100 millis")), | |
| Effect.repeat( | |
| Effect.gen(function* () { | |
| const bytes = yield* Effect.succeed(downloadedBytes); | |
| yield* Effect.log(`Downloaded ${bytes} bytes`); | |
| }), | |
| Schedule.forever.pipe(Schedule.delayed(() => "2 seconds")), | |
| ).pipe(Effect.delay("100 millis")), |
🤖 Prompt for AI Agents
In packages/web-backend/src/Loom/ImportVideo.ts around lines 133 to 139, there's
an inline TODO comment inside the Effect.repeat block; per repo guidelines
remove that inline comment and any TODO markers, leaving the Effect.repeat
scheduling and logging behavior intact so the schedule continues to log
downloadedBytes without commented code present. Ensure no inline comments remain
in that block and keep the existing Schedule.forever.pipe(Schedule.delayed(...))
and Effect.delay call as-is.
| import * as CloudFrontPresigner from "@aws-sdk/cloudfront-signer"; | ||
| import { decrypt } from "@cap/database/crypto"; | ||
| import { S3_BUCKET_URL } from "@cap/utils"; | ||
| import type { S3Bucket } from "@cap/web-domain"; |
There was a problem hiding this comment.
Fix broken type import (namespace needed).
S3Bucket.S3Bucket / S3Bucket.S3BucketId usage requires a namespace import. The current import type { S3Bucket } will not compile.
Apply:
-import type { S3Bucket } from "@cap/web-domain";
+import * as S3Bucket 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 { S3Bucket } from "@cap/web-domain"; | |
| import * as S3Bucket from "@cap/web-domain"; |
🤖 Prompt for AI Agents
In packages/web-backend/src/S3Buckets/index.ts around line 5, the file currently
uses a named type import which won't allow accessing nested types like
S3Bucket.S3Bucket or S3Bucket.S3BucketId; replace the named type import with a
namespace type import from "@cap/web-domain" so the code can reference
S3Bucket.S3Bucket and S3Bucket.S3BucketId (i.e., use a type-only namespace
import instead of importing a single symbol).
Brendonovich
force-pushed
the
universal-s3-access
branch
from
423c353 to
29ebf78
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/web-backend/src/Videos/index.ts (2)
36-40: Incorrect use of flatMap with catchAll; error mapping never applied.Use catchAll directly; flatMap expects a value->Effect function.
- const [video] = yield* repo - .getById(videoId) - .pipe( - Effect.flatMap(Effect.catchAll(() => new Video.NotFoundError())), - ); + const [video] = yield* repo + .getById(videoId) + .pipe(Effect.catchAll(() => Effect.fail(new Video.NotFoundError())));
70-75: Same flatMap/catchAll misuse in duplicate().Apply the same fix as above.
- const [video] = yield* repo - .getById(videoId) - .pipe( - Effect.flatMap(Effect.catchAll(() => new Video.NotFoundError())), - Policy.withPolicy(policy.isOwner(videoId)), - ); + const [video] = yield* repo + .getById(videoId) + .pipe( + Effect.catchAll(() => Effect.fail(new Video.NotFoundError())), + Policy.withPolicy(policy.isOwner(videoId)), + );apps/web/app/api/thumbnail/route.ts (1)
13-97: Refactor to @effect/platform HttpApi/HttpApiBuilder and export via apiToHandler(ApiLive)Convert apps/web/app/api/thumbnail/route.ts (lines 13–97) from NextRequest/Response to an @effect/platform HttpApi/HttpApiBuilder implementation, map domain→transport errors with HttpApiError.*, and export only the handler produced by apiToHandler(ApiLive). Offer a minimal patch converting this route with exhaustive error translation.
apps/web/app/api/upload/[...route]/multipart.ts (1)
303-321: Fix const redeclaration: result is declared twice in same scope.Rename DB update result to avoid shadowing the multipart
result.- const result = await db() + const updateResult = await db() .update(videos) @@ - if (result.rowsAffected > 0) + if (updateResult.rowsAffected > 0) await db() .delete(videoUploads) .where(eq(videoUploads.videoId, videoId));Also applies to: 318-323
🧹 Nitpick comments (7)
apps/web/app/api/thumbnail/route.ts (1)
50-58: Avoid multiple runPromise invocations.Wrap S3 operations in a single Effect.gen and call runPromise once to reduce overhead and share the same Effect environment.
Also applies to: 76-79
packages/web-backend/src/Videos/index.ts (2)
48-51: Use video.ownerId for S3 key prefix.Safer and consistent with duplicate(); removes unnecessary CurrentUser dependency.
- const user = yield* CurrentUser; - - const prefix = `${user.id}/${video.id}/`; + const prefix = `${video.ownerId}/${video.id}/`;
8-8: Remove unused import.S3BucketAccess is not used after the refactor.
apps/web/app/api/playlist/route.ts (2)
69-70: Provide S3Buckets via a Layer (per repo guidelines).Prefer Layer-based provision over manual provideService.
- Effect.provideService(S3Buckets, s3Buckets), + Effect.provide(Layer.succeed(S3Buckets, s3Buckets)),
145-148: Set Content-Type for HLS playlists.Return m3u8 with explicit Content-Type to avoid misdetection by some clients/CDNs.
- return HttpServerResponse.text(playlist, { - headers: CACHE_CONTROL_HEADERS, - }); + return HttpServerResponse.text(playlist, { + headers: { + ...CACHE_CONTROL_HEADERS, + "Content-Type": "application/x-mpegURL", + }, + });- return HttpServerResponse.text(generatedPlaylist, { - headers: CACHE_CONTROL_HEADERS, - }); + return HttpServerResponse.text(generatedPlaylist, { + headers: { + ...CACHE_CONTROL_HEADERS, + "Content-Type": "application/x-mpegURL", + }, + });- return HttpServerResponse.text(generatedPlaylist, { - headers: CACHE_CONTROL_HEADERS, - }); + return HttpServerResponse.text(generatedPlaylist, { + headers: { + ...CACHE_CONTROL_HEADERS, + "Content-Type": "application/x-mpegURL", + }, + });Also applies to: 195-198, 225-227
apps/web/app/api/upload/[...route]/signed.ts (1)
118-121: Use getBucketAccessForUser to avoid duplicate lookups.You already fetched
customBucketabove for invalidation logic; the access API can resolve user->bucket directly.- const [bucket] = yield* S3Buckets.getBucketAccess( - Option.fromNullable(customBucket?.id), - ); + const [bucket] = yield* S3Buckets.getBucketAccessForUser(user.id);apps/web/app/api/upload/[...route]/multipart.ts (1)
268-273: URL-encode CopySource key for S3 copyObject.Keys with special chars must be URL-encoded per AWS spec.
- .copyObject(`${bucket.bucketName}/${fileKey}`, fileKey, { + .copyObject(`${bucket.bucketName}/${encodeURIComponent(fileKey)}`, fileKey, { ContentType: "video/mp4", MetadataDirective: "REPLACE", })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
apps/web/actions/organization/create-space.ts(2 hunks)apps/web/actions/organization/delete-space.ts(2 hunks)apps/web/actions/organization/update-space.ts(2 hunks)apps/web/actions/organization/upload-organization-icon.ts(2 hunks)apps/web/actions/organization/upload-space-icon.ts(3 hunks)apps/web/actions/video/upload.ts(2 hunks)apps/web/actions/videos/download.ts(2 hunks)apps/web/actions/videos/edit-transcript.ts(2 hunks)apps/web/actions/videos/generate-ai-metadata.ts(2 hunks)apps/web/actions/videos/get-og-image.tsx(2 hunks)apps/web/actions/videos/get-transcript.ts(2 hunks)apps/web/app/api/desktop/[...route]/s3Config.ts(2 hunks)apps/web/app/api/desktop/[...route]/video.ts(2 hunks)apps/web/app/api/playlist/route.ts(4 hunks)apps/web/app/api/thumbnail/route.ts(3 hunks)apps/web/app/api/upload/[...route]/multipart.ts(5 hunks)apps/web/app/api/upload/[...route]/signed.ts(3 hunks)apps/web/app/embed/[videoId]/page.tsx(1 hunks)apps/web/app/layout.tsx(0 hunks)apps/web/app/s/[videoId]/page.tsx(1 hunks)apps/web/components/forms/server.ts(2 hunks)apps/web/lib/server.ts(1 hunks)apps/web/lib/transcribe.ts(3 hunks)apps/web/utils/public-env.tsx(0 hunks)apps/web/utils/s3.ts(0 hunks)packages/database/schema.ts(3 hunks)packages/web-backend/src/Loom/ImportVideo.ts(3 hunks)packages/web-backend/src/S3Buckets/S3BucketAccess.ts(2 hunks)packages/web-backend/src/S3Buckets/index.ts(8 hunks)packages/web-backend/src/Videos/index.ts(2 hunks)packages/web-backend/src/index.ts(0 hunks)
💤 Files with no reviewable changes (4)
- apps/web/app/layout.tsx
- packages/web-backend/src/index.ts
- apps/web/utils/s3.ts
- apps/web/utils/public-env.tsx
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/web/actions/videos/edit-transcript.ts
- apps/web/lib/transcribe.ts
- apps/web/app/embed/[videoId]/page.tsx
- apps/web/app/api/desktop/[...route]/s3Config.ts
- apps/web/actions/organization/update-space.ts
- apps/web/app/s/[videoId]/page.tsx
- packages/database/schema.ts
🧰 Additional context used
📓 Path-based instructions (8)
apps/web/actions/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
All Groq/OpenAI calls must be implemented in Next.js Server Actions under apps/web/actions; do not place AI calls elsewhere
Files:
apps/web/actions/videos/get-transcript.tsapps/web/actions/video/upload.tsapps/web/actions/organization/upload-organization-icon.tsapps/web/actions/videos/download.tsapps/web/actions/organization/delete-space.tsapps/web/actions/organization/upload-space-icon.tsapps/web/actions/videos/generate-ai-metadata.tsapps/web/actions/organization/create-space.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/actions/videos/get-transcript.tsapps/web/components/forms/server.tsapps/web/app/api/upload/[...route]/multipart.tsapps/web/actions/video/upload.tsapps/web/actions/videos/get-og-image.tsxapps/web/app/api/desktop/[...route]/video.tsapps/web/app/api/thumbnail/route.tsapps/web/app/api/upload/[...route]/signed.tsapps/web/actions/organization/upload-organization-icon.tsapps/web/actions/videos/download.tsapps/web/actions/organization/delete-space.tsapps/web/lib/server.tsapps/web/actions/organization/upload-space-icon.tsapps/web/actions/videos/generate-ai-metadata.tsapps/web/actions/organization/create-space.tsapps/web/app/api/playlist/route.ts
**/*.{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:
apps/web/actions/videos/get-transcript.tsapps/web/components/forms/server.tsapps/web/app/api/upload/[...route]/multipart.tsapps/web/actions/video/upload.tsapps/web/actions/videos/get-og-image.tsxpackages/web-backend/src/Videos/index.tsapps/web/app/api/desktop/[...route]/video.tsapps/web/app/api/thumbnail/route.tspackages/web-backend/src/S3Buckets/index.tsapps/web/app/api/upload/[...route]/signed.tsapps/web/actions/organization/upload-organization-icon.tsapps/web/actions/videos/download.tspackages/web-backend/src/S3Buckets/S3BucketAccess.tsapps/web/actions/organization/delete-space.tsapps/web/lib/server.tsapps/web/actions/organization/upload-space-icon.tsapps/web/actions/videos/generate-ai-metadata.tspackages/web-backend/src/Loom/ImportVideo.tsapps/web/actions/organization/create-space.tsapps/web/app/api/playlist/route.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:
apps/web/actions/videos/get-transcript.tsapps/web/components/forms/server.tsapps/web/app/api/upload/[...route]/multipart.tsapps/web/actions/video/upload.tsapps/web/actions/videos/get-og-image.tsxpackages/web-backend/src/Videos/index.tsapps/web/app/api/desktop/[...route]/video.tsapps/web/app/api/thumbnail/route.tspackages/web-backend/src/S3Buckets/index.tsapps/web/app/api/upload/[...route]/signed.tsapps/web/actions/organization/upload-organization-icon.tsapps/web/actions/videos/download.tspackages/web-backend/src/S3Buckets/S3BucketAccess.tsapps/web/actions/organization/delete-space.tsapps/web/lib/server.tsapps/web/actions/organization/upload-space-icon.tsapps/web/actions/videos/generate-ai-metadata.tspackages/web-backend/src/Loom/ImportVideo.tsapps/web/actions/organization/create-space.tsapps/web/app/api/playlist/route.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/actions/videos/get-transcript.tsapps/web/components/forms/server.tsapps/web/app/api/upload/[...route]/multipart.tsapps/web/actions/video/upload.tsapps/web/actions/videos/get-og-image.tsxpackages/web-backend/src/Videos/index.tsapps/web/app/api/desktop/[...route]/video.tsapps/web/app/api/thumbnail/route.tspackages/web-backend/src/S3Buckets/index.tsapps/web/app/api/upload/[...route]/signed.tsapps/web/actions/organization/upload-organization-icon.tsapps/web/actions/videos/download.tspackages/web-backend/src/S3Buckets/S3BucketAccess.tsapps/web/actions/organization/delete-space.tsapps/web/lib/server.tsapps/web/actions/organization/upload-space-icon.tsapps/web/actions/videos/generate-ai-metadata.tspackages/web-backend/src/Loom/ImportVideo.tsapps/web/actions/organization/create-space.tsapps/web/app/api/playlist/route.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/actions/videos/get-transcript.tsapps/web/components/forms/server.tsapps/web/app/api/upload/[...route]/multipart.tsapps/web/actions/video/upload.tsapps/web/actions/videos/get-og-image.tsxapps/web/app/api/desktop/[...route]/video.tsapps/web/app/api/thumbnail/route.tsapps/web/app/api/upload/[...route]/signed.tsapps/web/actions/organization/upload-organization-icon.tsapps/web/actions/videos/download.tsapps/web/actions/organization/delete-space.tsapps/web/lib/server.tsapps/web/actions/organization/upload-space-icon.tsapps/web/actions/videos/generate-ai-metadata.tsapps/web/actions/organization/create-space.tsapps/web/app/api/playlist/route.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/api/upload/[...route]/multipart.tsapps/web/app/api/desktop/[...route]/video.tsapps/web/app/api/thumbnail/route.tsapps/web/app/api/upload/[...route]/signed.tsapps/web/app/api/playlist/route.ts
apps/web/app/api/**/route.ts
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/app/api/**/route.ts: Place API routes only under apps/web/app/api and implement each route in a route.ts file
Construct API routes with @effect/platform HttpApi/HttpApiBuilder and export only the handler from apiToHandler(ApiLive)
Map domain errors to transport errors with HttpApiError.* and keep error translation exhaustive
Use HttpAuthMiddleware for required auth and provideOptionalAuth for guest routes; avoid duplicate session lookups
Provide dependencies via Layer.provide in API routes instead of manual provideService calls
Files:
apps/web/app/api/thumbnail/route.tsapps/web/app/api/playlist/route.ts
🧠 Learnings (2)
📚 Learning: 2025-09-24T07:24:21.429Z
Learnt from: Brendonovich
PR: CapSoftware/Cap#1060
File: apps/web/components/forms/server.ts:85-97
Timestamp: 2025-09-24T07:24:21.429Z
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:
apps/web/components/forms/server.tsapps/web/actions/video/upload.tspackages/web-backend/src/S3Buckets/index.tsapps/web/actions/organization/upload-organization-icon.tspackages/web-backend/src/S3Buckets/S3BucketAccess.tsapps/web/actions/organization/upload-space-icon.tsapps/web/actions/organization/create-space.ts
📚 Learning: 2025-09-22T14:17:47.380Z
Learnt from: CR
PR: CapSoftware/Cap#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-22T14:17:47.380Z
Learning: Applies to apps/web/app/api/**/route.ts : Provide dependencies via Layer.provide in API routes instead of manual provideService calls
Applied to files:
apps/web/lib/server.ts
🧬 Code graph analysis (19)
apps/web/actions/videos/get-transcript.ts (2)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/components/forms/server.ts (3)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)packages/env/server.ts (1)
serverEnv(83-87)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/app/api/upload/[...route]/multipart.ts (2)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/actions/video/upload.ts (2)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/actions/videos/get-og-image.tsx (2)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)apps/web/lib/server.ts (1)
runPromise(56-71)
packages/web-backend/src/Videos/index.ts (3)
packages/database/schema.ts (1)
s3Buckets(373-383)packages/web-domain/src/Policy.ts (2)
Policy(7-11)policy(27-39)packages/web-domain/src/Authentication.ts (1)
CurrentUser(7-10)
apps/web/app/api/desktop/[...route]/video.ts (3)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)packages/web-backend/src/index.ts (1)
S3Buckets(6-6)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/app/api/thumbnail/route.ts (2)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)apps/web/lib/server.ts (1)
runPromise(56-71)
packages/web-backend/src/S3Buckets/index.ts (4)
packages/web-backend/src/S3Buckets/S3BucketAccess.ts (1)
createS3BucketAccess(40-290)packages/web-domain/src/S3Bucket.ts (3)
S3Bucket(6-14)S3BucketId(3-3)S3BucketId(4-4)packages/web-backend/src/S3Buckets/S3BucketsRepo.ts (1)
S3BucketsRepo(8-73)packages/web-backend/src/Database.ts (1)
Database(9-19)
apps/web/app/api/upload/[...route]/signed.ts (2)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/actions/organization/upload-organization-icon.ts (3)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)packages/env/server.ts (1)
serverEnv(83-87)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/actions/videos/download.ts (2)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)apps/web/lib/server.ts (1)
runPromise(56-71)
packages/web-backend/src/S3Buckets/S3BucketAccess.ts (1)
packages/web-backend/src/S3Buckets/S3BucketClientProvider.ts (1)
S3BucketClientProvider(4-13)
apps/web/actions/organization/delete-space.ts (2)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/lib/server.ts (1)
packages/web-backend/src/Database.ts (1)
Database(9-19)
apps/web/actions/organization/upload-space-icon.ts (2)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/actions/videos/generate-ai-metadata.ts (3)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)packages/web-backend/src/index.ts (1)
S3Buckets(6-6)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/actions/organization/create-space.ts (2)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)apps/web/lib/server.ts (1)
runPromise(56-71)
apps/web/app/api/playlist/route.ts (1)
packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)
🪛 GitHub Check: Typecheck
apps/web/app/api/thumbnail/route.ts
[failure] 52-52:
Cannot find name 'result'.
⏰ 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 (20)
apps/web/actions/videos/generate-ai-metadata.ts (1)
124-131: LGTM on S3 access migration.Effect-based bucket access + runPromise pattern looks correct here.
apps/web/lib/server.ts (1)
47-51: Layer composition change LGTM.Including Database.Default in the provided inner merge aligns with broader Effect provisioning.
apps/web/actions/videos/get-og-image.tsx (1)
71-78: LGTM on bucket access and signed URL generation.Effect.gen + runPromise usage is clean and scoped.
apps/web/actions/video/upload.ts (1)
117-129: Yield bucketName and avoid naive endpoint substring checks.bucket.bucketName is an Effect; also replace fragile includes("amazonaws.com") logic.
const presignedPostData = await Effect.gen(function* () { const [bucket] = yield* S3Buckets.getBucketAccess( Option.fromNullable(customBucket?.id), ); const presignedPostData = yield* bucket.getPresignedPostUrl(fileKey, { Fields, Expires: 1800, }); + const bucketName = yield* bucket.bucketName; const customEndpoint = serverEnv().CAP_AWS_ENDPOINT; - if (customEndpoint && !customEndpoint.includes("amazonaws.com")) { + if (customEndpoint) { - if (serverEnv().S3_PATH_STYLE) { - presignedPostData.url = `${customEndpoint}/${bucket.bucketName}`; - } else { - presignedPostData.url = customEndpoint; - } + try { + const u = new URL(customEndpoint); + presignedPostData.url = serverEnv().S3_PATH_STYLE + ? `${u.origin}${u.pathname.replace(/\/$/, "")}/${bucketName}` + : `${u.origin}${u.pathname}`; + } catch { + // fallback: leave URL as-is + } } return presignedPostData; }).pipe(runPromise);apps/web/app/api/desktop/[...route]/video.ts (1)
226-241: LGTM: Proper Effect-based S3 cleanup implementationThe S3 deletion flow correctly uses Effect.gen with S3Buckets.getBucketAccess and properly yields the bucket operations before executing with runPromise. The error handling with try/catch ensures space deletion continues even if S3 cleanup fails.
apps/web/actions/videos/get-transcript.ts (1)
52-70: LGTM: Proper Effect-based S3 transcript retrievalThe implementation correctly:
- Uses S3Buckets.getBucketAccess with proper Option handling
- Yields the bucket.getObject effect properly
- Uses Option.isNone() to check for missing files
- Accesses vttContent.value when present
- Executes via runPromise
apps/web/actions/organization/upload-space-icon.ts (1)
78-84: Critical issue: Effects not properly handled in putObject callLines 81 and 92-96 have critical issues based on the retrieved learnings:
- Line 81:
Effect.promise(() => sanitizedFile.bytes())should be yielded withyield*before passing toputObject- Lines 92 and 94:
bucket.bucketNameis an Effect that must be yielded before use in string interpolationThis matches the exact issue flagged in the past review comments. The code needs to be fixed as previously suggested.Based on the web search results confirming that in Effect-TS, you must use
yield*with Effects to properly extract their values, and in general, if you ever find yourself with an Effect of an Effect, you're probably missing a yield* somewhere, the critical issues remain:- yield* bucket.putObject( - fileKey, - Effect.promise(() => sanitizedFile.bytes()), - { contentType: file.type }, - ); + const fileBytes = yield* Effect.promise(() => sanitizedFile.bytes()); + yield* bucket.putObject( + fileKey, + fileBytes, + { contentType: file.type }, + ); + const bucketName = yield* bucket.bucketName; if (serverEnv().CAP_AWS_BUCKET_URL) { iconUrl = `${serverEnv().CAP_AWS_BUCKET_URL}/${fileKey}`; } else if (serverEnv().CAP_AWS_ENDPOINT) { - iconUrl = `${serverEnv().CAP_AWS_ENDPOINT}/${bucket.bucketName}/${fileKey}`; + iconUrl = `${serverEnv().CAP_AWS_ENDPOINT}/${bucketName}/${fileKey}`; } else { - iconUrl = `https://${bucket.bucketName}.s3.${ + iconUrl = `https://${bucketName}.s3.${ serverEnv().CAP_AWS_REGION || "us-east-1" }.amazonaws.com/${fileKey}`; }apps/web/actions/videos/download.ts (1)
38-43: Critical bug: missing return statement causes downloadUrl to be undefinedLine 42 yields the signed URL but doesn't return it from the Effect.gen block, making
downloadUrlundefined.This matches the exact issue flagged in past review comments. Apply this fix:
const downloadUrl = await Effect.gen(function* () { const [bucket] = yield* S3Buckets.getBucketAccess( Option.fromNullable(video.bucket), ); - yield* bucket.getSignedObjectUrl(videoKey); + return yield* bucket.getSignedObjectUrl(videoKey); }).pipe(runPromise);apps/web/components/forms/server.ts (1)
71-93: Critical issue: bucketName Effect not yielded before string interpolationLines 86 and 89-91 attempt to use
bucket.bucketNamedirectly in string interpolation, butbucketNameis an Effect that must be yielded first.Based on retrieved learnings, this needs to be fixed:
await Effect.gen(function* () { const [bucket] = yield* S3Buckets.getBucketAccess(Option.none()); yield* bucket.putObject( fileKey, yield* Effect.promise(() => iconFile.bytes()), { contentType: iconFile.type }, ); + const bucketName = yield* bucket.bucketName; if (serverEnv().CAP_AWS_BUCKET_URL) { iconUrl = `${serverEnv().CAP_AWS_BUCKET_URL}/${fileKey}`; } else if (serverEnv().CAP_AWS_ENDPOINT) { - iconUrl = `${serverEnv().CAP_AWS_ENDPOINT}/${bucket.bucketName}/${fileKey}`; + iconUrl = `${serverEnv().CAP_AWS_ENDPOINT}/${bucketName}/${fileKey}`; } else { - iconUrl = `https://${bucket.bucketName}.s3.${ + iconUrl = `https://${bucketName}.s3.${ serverEnv().CAP_AWS_REGION || "us-east-1" }.amazonaws.com/${fileKey}`; } }).pipe(runPromise);apps/web/actions/organization/upload-organization-icon.ts (1)
63-84: Critical issue: bucketName Effect not yielded before string interpolationLines 77 and 80-82 use
bucket.bucketNamedirectly in template literals, butbucketNameis an Effect that must be yielded first according to retrieved learnings.Apply this fix:
await Effect.gen(function* () { const [bucket] = yield* S3Buckets.getBucketAccess(Option.none()); yield* bucket.putObject( fileKey, yield* Effect.promise(() => sanitizedFile.bytes()), { contentType: file.type }, ); + const bucketName = yield* bucket.bucketName; if (serverEnv().CAP_AWS_BUCKET_URL) { iconUrl = `${serverEnv().CAP_AWS_BUCKET_URL}/${fileKey}`; } else if (serverEnv().CAP_AWS_ENDPOINT) { - iconUrl = `${serverEnv().CAP_AWS_ENDPOINT}/${bucket.bucketName}/${fileKey}`; + iconUrl = `${serverEnv().CAP_AWS_ENDPOINT}/${bucketName}/${fileKey}`; } else { - iconUrl = `https://${bucket.bucketName}.s3.${ + iconUrl = `https://${bucketName}.s3.${ serverEnv().CAP_AWS_REGION || "us-east-1" }.amazonaws.com/${fileKey}`; } }).pipe(runPromise);apps/web/actions/organization/delete-space.ts (1)
79-89: Critical issue: potential S3 delete failure with undefined KeysThe code maps
listedObjects.Contentsdirectly to delete objects, but S3 listObjects can return entries with undefinedKeyproperties, which would cause deleteObjects to fail.Apply this fix:
- if (listedObjects.Contents?.length) { - yield* bucket.deleteObjects( - listedObjects.Contents.map((content) => ({ - Key: content.Key, - })), - ); - - console.log( - `Deleted ${listedObjects.Contents.length} objects for space ${spaceId}`, - ); - } + const objects = (listedObjects.Contents ?? []) + .filter((c) => !!c.Key) + .map((c) => ({ Key: c.Key! })); + if (objects.length > 0) { + yield* bucket.deleteObjects(objects); + console.log( + `Deleted ${objects.length} objects for space ${spaceId}`, + ); + }apps/web/actions/organization/create-space.ts (1)
94-116: Critical issue: bucketName Effect not yielded and inline comments presentTwo issues:
- Lines 109 and 112-114:
bucket.bucketNameis an Effect that must be yielded before string interpolation- Lines 103, 105, 108, 111: Inline comments violate coding guidelines
Apply this fix:
await Effect.gen(function* () { const [bucket] = yield* S3Buckets.getBucketAccess(Option.none()); yield* bucket.putObject( fileKey, yield* Effect.promise(() => iconFile.bytes()), { contentType: iconFile.type }, ); - // Construct the icon URL + const bucketName = yield* bucket.bucketName; if (serverEnv().CAP_AWS_BUCKET_URL) { - // If a custom bucket URL is defined, use it iconUrl = `${serverEnv().CAP_AWS_BUCKET_URL}/${fileKey}`; } else if (serverEnv().CAP_AWS_ENDPOINT) { - // For custom endpoints like MinIO - iconUrl = `${serverEnv().CAP_AWS_ENDPOINT}/${bucket.bucketName}/${fileKey}`; + iconUrl = `${serverEnv().CAP_AWS_ENDPOINT}/${bucketName}/${fileKey}`; } else { - // Default AWS S3 URL format - iconUrl = `https://${bucket.bucketName}.s3.${ + iconUrl = `https://${bucketName}.s3.${ serverEnv().CAP_AWS_REGION || "us-east-1" }.amazonaws.com/${fileKey}`; } }).pipe(runPromise);apps/web/app/api/upload/[...route]/multipart.ts (1)
41-71: Good migration to Effect-based S3 access.Using S3Buckets.getBucketAccessForUser + runPromise is consistent with the new model.
packages/web-backend/src/S3Buckets/index.ts (3)
92-126: LGTM: CloudFront access override wiring is solid.Nicely composes createS3BucketAccess and overrides getSignedObjectUrl when CF envs exist.
166-186: API shape and static helpers look good.Renamed methods + static helpers align with the new access model.
Also applies to: 190-196
5-6: Fix type import: need namespace for nested types.
S3Bucket.S3Bucketrequires a namespace import; current type import won’t compile.-import type { S3Bucket } from "@cap/web-domain"; +import * as S3Bucket from "@cap/web-domain";packages/web-backend/src/S3Buckets/S3BucketAccess.ts (2)
17-38: wrapS3Promise change is correct.Handling both Promise and Effect paths simplifies call sites and centralizes error mapping.
40-293: S3 access factory looks consistent and complete.API surface (signed URLs, object ops, multipart) aligns with callers; bucketName exposure is convenient.
packages/web-backend/src/Loom/ImportVideo.ts (2)
106-113: Validate HTTP status; fail fast on non-2xx.Proceeding on non-2xx leads to uploading error pages. Wrap as LoomApiError.
- const resp = yield* http - .get(payload.loom.video.downloadUrl) - .pipe(Effect.catchAll((cause) => new LoomApiError({ cause }))); + const resp = yield* http + .get(payload.loom.video.downloadUrl) + .pipe(Effect.catchAll((cause) => Effect.fail(new LoomApiError({ cause })))); + if (resp.status < 200 || resp.status >= 300) { + yield* Effect.fail(new LoomApiError({ cause: `HTTP ${resp.status}` })); + }
132-139: Remove inline TODO; comments are disallowed.Inline comments violate repo rules. Drop the TODO.
- // TODO: Connect this with upload progress Effect.repeat( Effect.gen(function* () { const bytes = yield* Effect.succeed(downloadedBytes); yield* Effect.log(`Downloaded ${bytes} bytes`); }), Schedule.forever.pipe(Schedule.delayed(() => "2 seconds")),
| const [bucket] = await S3Buckets.getBucketAccess( | ||
| Option.fromNullable(result.bucket?.id), | ||
| ).pipe(runPromise); | ||
|
|
||
| const listResponse = await bucket | ||
| .listObjects({ prefix: prefix }) | ||
| .pipe(runPromise); | ||
| const contents = listResponse.Contents || []; |
There was a problem hiding this comment.
Fix undefined identifier and minor cleanup.
Use query instead of the undefined result and simplify the prefix object.
- const [bucket] = await S3Buckets.getBucketAccess(
- Option.fromNullable(result.bucket?.id),
- ).pipe(runPromise);
+ const [bucket] = await S3Buckets.getBucketAccess(
+ Option.fromNullable(query.bucket?.id),
+ ).pipe(runPromise);
- const listResponse = await bucket
- .listObjects({ prefix: prefix })
- .pipe(runPromise);
+ const listResponse = await bucket
+ .listObjects({ prefix })
+ .pipe(runPromise);📝 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 [bucket] = await S3Buckets.getBucketAccess( | |
| Option.fromNullable(result.bucket?.id), | |
| ).pipe(runPromise); | |
| const listResponse = await bucket | |
| .listObjects({ prefix: prefix }) | |
| .pipe(runPromise); | |
| const contents = listResponse.Contents || []; | |
| const [bucket] = await S3Buckets.getBucketAccess( | |
| Option.fromNullable(query.bucket?.id), | |
| ).pipe(runPromise); | |
| const listResponse = await bucket | |
| .listObjects({ prefix }) | |
| .pipe(runPromise); | |
| const contents = listResponse.Contents || []; |
🧰 Tools
🪛 GitHub Check: Typecheck
[failure] 52-52:
Cannot find name 'result'.
🤖 Prompt for AI Agents
In apps/web/app/api/thumbnail/route.ts around lines 51 to 58, replace the
undefined "result" usage with "query" when accessing the bucket id
(Option.fromNullable(query.bucket?.id)) and simplify the listObjects call to
pass the prefix directly rather than wrapping it in an unnecessary object (use
the prefix variable as the argument expected by your S3 client). Ensure "prefix"
is a defined string beforehand and remove the old object wrapper so the call
matches the S3 client's expected parameter shape.
| let data: PresignedPost; | ||
|
|
||
| await Effect.gen(function* () { | ||
| const [bucket] = yield* S3Buckets.getBucketAccess( | ||
| Option.fromNullable(customBucket?.id), | ||
| ); | ||
|
|
||
| data = { url: presignedUrl, fields: {} }; | ||
| } | ||
| if (method === "post") { | ||
| const Fields = { | ||
| "Content-Type": contentType, | ||
| "x-amz-meta-userid": user.id, | ||
| "x-amz-meta-duration": durationInSecs | ||
| ? durationInSecs.toString() | ||
| : "", | ||
| }; | ||
|
|
||
| data = yield* bucket.getPresignedPostUrl(fileKey, { | ||
| Fields, | ||
| Expires: 1800, | ||
| }); | ||
| } else if (method === "put") { | ||
| const presignedUrl = yield* bucket.getPresignedPutUrl( | ||
| fileKey, | ||
| { | ||
| ContentType: contentType, | ||
| Metadata: { | ||
| userid: user.id, | ||
| duration: durationInSecs ? durationInSecs.toString() : "", | ||
| }, | ||
| }, | ||
| { expiresIn: 1800 }, | ||
| ); | ||
|
|
||
| data = { url: presignedUrl, fields: {} }; | ||
| } | ||
| }).pipe(runPromise); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
Avoid conflating PUT and POST shapes; keep types stable.
data: PresignedPost is reused for PUT by forging {url, fields:{}}. Split types to prevent downstream breakage.
- let data: PresignedPost;
+ let postData: PresignedPost | undefined;
+ let putUrl: string | undefined;
@@
- if (method === "post") {
+ if (method === "post") {
@@
- data = yield* bucket.getPresignedPostUrl(fileKey, {
+ postData = yield* bucket.getPresignedPostUrl(fileKey, {
Fields,
Expires: 1800,
});
- } else if (method === "put") {
+ } else if (method === "put") {
const presignedUrl = yield* bucket.getPresignedPutUrl(
fileKey,
{
ContentType: contentType,
Metadata: {
userid: user.id,
duration: durationInSecs ? durationInSecs.toString() : "",
},
},
{ expiresIn: 1800 },
);
-
- data = { url: presignedUrl, fields: {} };
+ putUrl = presignedUrl;
}
@@
- if (method === "post") return c.json({ presignedPostData: data! });
- else return c.json({ presignedPutData: data! });
+ if (method === "post") return c.json({ presignedPostData: postData! });
+ else return c.json({ presignedPutData: putUrl! });Run to confirm no callers assume presignedPutData is a string:
Also applies to: 189-190
🏁 Script executed:
#!/bin/bash
# Search for presignedPutData usage shapes
rg -n -C3 '\bpresignedPutData\b'Length of output: 564
Split POST and PUT presigned shapes; keep types stable
data is typed as PresignedPost but reused for PUT — introduce separate postData and putUrl and return them instead.
File: apps/web/app/api/upload/[...route]/signed.ts (116-151, 187-191)
- let data: PresignedPost;
+ let postData: PresignedPost | undefined;
+ let putUrl: string | undefined;
@@
- if (method === "post") {
+ if (method === "post") {
@@
- data = yield* bucket.getPresignedPostUrl(fileKey, {
+ postData = yield* bucket.getPresignedPostUrl(fileKey, {
Fields,
Expires: 1800,
});
- } else if (method === "put") {
+ } else if (method === "put") {
const presignedUrl = yield* bucket.getPresignedPutUrl(
fileKey,
{
ContentType: contentType,
Metadata: {
userid: user.id,
duration: durationInSecs ? durationInSecs.toString() : "",
},
},
{ expiresIn: 1800 },
);
-
- data = { url: presignedUrl, fields: {} };
+ putUrl = presignedUrl;
}
@@
- if (method === "post") return c.json({ presignedPostData: data! });
- else return c.json({ presignedPutData: data! });
+ if (method === "post") return c.json({ presignedPostData: postData! });
+ else return c.json({ presignedPutData: putUrl! });rg search shows presignedPutData only where it's returned in this file — safe to change.
📝 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.
| let data: PresignedPost; | |
| await Effect.gen(function* () { | |
| const [bucket] = yield* S3Buckets.getBucketAccess( | |
| Option.fromNullable(customBucket?.id), | |
| ); | |
| data = { url: presignedUrl, fields: {} }; | |
| } | |
| if (method === "post") { | |
| const Fields = { | |
| "Content-Type": contentType, | |
| "x-amz-meta-userid": user.id, | |
| "x-amz-meta-duration": durationInSecs | |
| ? durationInSecs.toString() | |
| : "", | |
| }; | |
| data = yield* bucket.getPresignedPostUrl(fileKey, { | |
| Fields, | |
| Expires: 1800, | |
| }); | |
| } else if (method === "put") { | |
| const presignedUrl = yield* bucket.getPresignedPutUrl( | |
| fileKey, | |
| { | |
| ContentType: contentType, | |
| Metadata: { | |
| userid: user.id, | |
| duration: durationInSecs ? durationInSecs.toString() : "", | |
| }, | |
| }, | |
| { expiresIn: 1800 }, | |
| ); | |
| data = { url: presignedUrl, fields: {} }; | |
| } | |
| }).pipe(runPromise); | |
| let postData: PresignedPost | undefined; | |
| let putUrl: string | undefined; | |
| await Effect.gen(function* () { | |
| const [bucket] = yield* S3Buckets.getBucketAccess( | |
| Option.fromNullable(customBucket?.id), | |
| ); | |
| if (method === "post") { | |
| const Fields = { | |
| "Content-Type": contentType, | |
| "x-amz-meta-userid": user.id, | |
| "x-amz-meta-duration": durationInSecs | |
| ? durationInSecs.toString() | |
| : "", | |
| }; | |
| postData = yield* bucket.getPresignedPostUrl(fileKey, { | |
| Fields, | |
| Expires: 1800, | |
| }); | |
| } else if (method === "put") { | |
| const presignedUrl = yield* bucket.getPresignedPutUrl( | |
| fileKey, | |
| { | |
| ContentType: contentType, | |
| Metadata: { | |
| userid: user.id, | |
| duration: durationInSecs ? durationInSecs.toString() : "", | |
| }, | |
| }, | |
| { expiresIn: 1800 }, | |
| ); | |
| putUrl = presignedUrl; | |
| } | |
| }).pipe(runPromise); | |
| if (method === "post") return c.json({ presignedPostData: postData! }); | |
| else return c.json({ presignedPutData: putUrl! }); |
🤖 Prompt for AI Agents
In apps/web/app/api/upload/[...route]/signed.ts around lines 116 to 151 (and
adjust uses at 187-191), the variable data is declared as PresignedPost but is
reused to hold a PUT URL shape; split the two shapes by introducing separate
variables (e.g., postData: PresignedPost | undefined and putUrl: { url: string;
fields: Record<string,string> } | undefined or a specific presignedPutData
type), populate postData when method === "post" and putUrl (or presignedPutData)
when method === "put", update the return shape and any downstream references to
use the appropriate variable, and keep types stable so the PUT branch never
assigns to the PresignedPost-typed variable.
| if (listedObjects.Contents?.length) { | ||
| yield* bucket.deleteObjects( | ||
| listedObjects.Contents.map((content) => ({ | ||
| Key: content.Key, | ||
| })), | ||
| ); | ||
| } |
There was a problem hiding this comment.
Filter out undefined Keys before delete.
S3 deleteObjects expects defined Keys.
- if (listedObjects.Contents?.length) {
- yield* bucket.deleteObjects(
- listedObjects.Contents.map((content) => ({
- Key: content.Key,
- })),
- );
- }
+ if (listedObjects.Contents?.length) {
+ yield* bucket.deleteObjects(
+ Array.filterMap(listedObjects.Contents, (c) =>
+ Option.map(Option.fromNullable(c.Key), (Key) => ({ Key })),
+ ),
+ );
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/web-backend/src/Videos/index.ts around lines 54 to 60, the code maps
listedObjects.Contents to delete Keys but may include undefined Key values which
S3 deleteObjects rejects; filter out entries without a defined Key before
calling bucket.deleteObjects and only call deleteObjects when the resulting
array of { Key } items is non-empty (i.e., map to Key-bearing objects, filter
where Key != null/undefined, then conditionally yield* deleteObjects).
| const allObjects = yield* bucket.listObjects({ prefix }); | ||
|
|
||
| if (allObjects.Contents) | ||
| yield* Effect.all( | ||
| Array.filterMap(allObjects.Contents, (obj) => | ||
| Option.map(Option.fromNullable(obj.Key), (key) => { | ||
| const newKey = key.replace(prefix, newPrefix); | ||
| return bucket.copyObject( | ||
| `${bucket.bucketName}/${obj.Key}`, | ||
| newKey, | ||
| ); | ||
| }), |
There was a problem hiding this comment.
bucket.bucketName must be yielded before string interpolation.
bucketName is an Effect; using it directly will break typing/runtime.
- const allObjects = yield* bucket.listObjects({ prefix });
+ const bucketName = yield* bucket.bucketName;
+ const allObjects = yield* bucket.listObjects({ prefix });
...
- return bucket.copyObject(
- `${bucket.bucketName}/${obj.Key}`,
- newKey,
- );
+ return bucket.copyObject(`${bucketName}/${obj.Key}`, newKey);📝 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 allObjects = yield* bucket.listObjects({ prefix }); | |
| if (allObjects.Contents) | |
| yield* Effect.all( | |
| Array.filterMap(allObjects.Contents, (obj) => | |
| Option.map(Option.fromNullable(obj.Key), (key) => { | |
| const newKey = key.replace(prefix, newPrefix); | |
| return bucket.copyObject( | |
| `${bucket.bucketName}/${obj.Key}`, | |
| newKey, | |
| ); | |
| }), | |
| const bucketName = yield* bucket.bucketName; | |
| const allObjects = yield* bucket.listObjects({ prefix }); | |
| if (allObjects.Contents) | |
| yield* Effect.all( | |
| Array.filterMap(allObjects.Contents, (obj) => | |
| Option.map(Option.fromNullable(obj.Key), (key) => { | |
| const newKey = key.replace(prefix, newPrefix); | |
| return bucket.copyObject( | |
| `${bucketName}/${obj.Key}`, | |
| newKey, | |
| ); | |
| }), |
🤖 Prompt for AI Agents
In packages/web-backend/src/Videos/index.ts around lines 85 to 96, the code
interpolates `${bucket.bucketName}/${obj.Key}` but bucket.bucketName is an
Effect and must be yielded first; change the flow so you yield
(run/flatMap/await via yield*) bucket.bucketName into a local const (e.g.
bucketName) before using it in string interpolation, then use
`${bucketName}/${obj.Key}` inside the copyObject call and ensure the surrounding
Effect composition uses the yielded value (move the bucketName yield above or
into the Effect chain that builds the Array of copy operations).
Summary by CodeRabbit
Refactor
Bug Fixes
Chores