Audio sync with cloudfare - #188
Draft
Joel-Joseph-George wants to merge 5 commits into
Draft
Conversation
Joel-Joseph-George
marked this pull request as draft
June 17, 2026 15:14
Joel-Joseph-George
marked this pull request as ready for review
June 18, 2026 05:39
Contributor
📝 WalkthroughWalkthroughAdds a ChangesAudio Recording Sync with Cloudflare R2
Sequence Diagram(s)sequenceDiagram
participant Client as Mobile Client
participant Route as POST /recordings/sync
participant syncRecording as syncRecording (service)
participant uploadToR2 as uploadToR2 (lib)
participant R2 as Cloudflare R2
participant upsertRecording as upsertRecording (repository)
participant DB as PostgreSQL
Client->>Route: multipart/form-data (file, project_unit_id, bible_text_id, relative_path)
Route->>Route: parse & validate fields (400 on failure)
Route->>syncRecording: SyncRecordingInput
syncRecording->>DB: SELECT project_units WHERE id = projectUnitId
DB-->>syncRecording: exists / not found (throws NOT_FOUND)
syncRecording->>syncRecording: buildR2Key (throws INVALID_PATH)
syncRecording->>uploadToR2: buffer, r2Key, contentType
uploadToR2->>R2: PUT /{bucket}/{r2Key} with SigV4 Authorization
R2-->>uploadToR2: 200 OK
uploadToR2-->>syncRecording: void
syncRecording->>upsertRecording: UpsertRecordingData
upsertRecording->>DB: INSERT INTO recordings ON CONFLICT UPDATE
DB-->>upsertRecording: { relativePath }
upsertRecording-->>syncRecording: { relativePath }
syncRecording-->>Route: { transformedRelativePath }
Route-->>Client: 200 { success: true, data: { relative_path } }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/db/schema.ts`:
- Around line 563-568: The metadata field in insertRecordingsSchema lacks
runtime validation, allowing invalid metadata objects to be stored despite
compile-time typing. In the createInsertSchema call for the recordings schema,
add a validation rule for the metadata field that enforces its expected runtime
structure and shape, similar to how other fields like projectUnitId and
relativePath are validated, to ensure the jsonb column only accepts payloads
that match the defined contract.
In `@src/domains/recordings/recordings.repository.ts`:
- Around line 25-30: The onConflictDoUpdate block in the recordings repository
is incorrectly mutating the createdAt field by resetting it to now() on
conflict, which breaks creation audit semantics since createdAt should represent
the original creation time. Additionally, recordedByUserId is not being updated
in the conflict resolution, causing stale uploader attribution on re-syncs.
Remove the createdAt line from the set object in onConflictDoUpdate and add
recordedByUserId to the set to update it with the excluded.recordedByUserId
value, ensuring the uploader attribution stays current. If you need to track
when records were last modified, consider adding an updated_at column to
schema.recordings instead of mutating createdAt.
In `@src/domains/recordings/recordings.route.ts`:
- Around line 142-145: The error response in the return statement of the error
handler is exposing the internal error?.message to the client, which could leak
sensitive implementation details. Replace the dynamic error?.message with a
static generic message in the json response returned at the end of the error
handler block, keeping only the generic 'An unexpected error occurred.' message
since the full error details are already being logged server-side earlier in the
handler.
- Around line 128-133: In the recordings route error handling, change the HTTP
status code returned for the 'NOT_FOUND' error condition from
HttpStatusCodes.BAD_REQUEST to HttpStatusCodes.NOT_FOUND to better align with
REST conventions where 404 is semantically appropriate for missing resources.
Additionally, update the OpenAPI schema definition in
syncRecordingRoute.responses to include a 404 response specification to reflect
this change in the API documentation.
In `@src/domains/recordings/recordings.service.ts`:
- Around line 64-80: The `userId` parameter in the `verifyProjectUnitExists`
function is not being used in the query logic. Either remove this unused
parameter from the function signature or, if it's intended for future
authorization validation (as suggested by the commented-out code), add a TODO
comment above the function explaining its intended purpose for upcoming
assignment validation checks. Choose the approach that best aligns with the
team's immediate plans for this function.
- Around line 35-43: The buildR2Key function validates that at least one slash
exists in the path but doesn't ensure that the rest segment (the part after the
first slash) is non-empty, allowing paths like "projectName/" to create keys
with trailing slashes. Add validation to the buildR2Key function to return null
if the rest substring is empty, or add validation in the route handler to reject
such invalid paths. Additionally, create unit tests for buildR2Key covering edge
cases including empty rest segments, paths ending with slashes, and missing
slash separators to prevent future regressions.
In `@src/lib/r2-upload.ts`:
- Around line 169-186: The fetch call in the uploadToR2 function lacks an
explicit timeout mechanism, which can cause socket stalls to hang indefinitely.
Add an AbortController with a timeout to the fetch request in the try block
where the PUT request is made to the R2 URL. Set up the AbortController to abort
the request after a reasonable timeout duration (such as several seconds), and
pass the abort signal in the fetch options alongside the existing method,
headers, and body configuration. This ensures requests that exceed the timeout
are automatically cancelled rather than hanging indefinitely.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3cb259a7-2c02-4645-a193-07d3482e507a
📒 Files selected for processing (15)
.env.example.env.testsrc/app.tssrc/db/migrations/0012_add_recordings.sqlsrc/db/migrations/0013_add_metadata_to_recordings.sqlsrc/db/migrations/meta/0012_snapshot.jsonsrc/db/migrations/meta/0013_snapshot.jsonsrc/db/migrations/meta/_journal.jsonsrc/db/schema.tssrc/domains/recordings/recordings.repository.tssrc/domains/recordings/recordings.route.tssrc/domains/recordings/recordings.service.tssrc/domains/recordings/recordings.types.tssrc/env.tssrc/lib/r2-upload.ts
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/domains/recordings/recordings.route.ts (1)
113-127:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReturn the canonical transformed path from the service, not the raw client path.
Line 113 drops the
syncRecordingresult, and Line 127 responds with the untransformedrelativePath. The service uploads/upserts usingtransformedRelativePath, so this response can point clients to the wrong object key.Proposed fix
- await service.syncRecording({ + const { transformedRelativePath } = await service.syncRecording({ projectUnitId, bibleTextId, relativePath, file, fileSizeRaw, recordedAtRaw, userId: user.id, }); @@ - data: { relative_path: relativePath }, + data: { relative_path: transformedRelativePath },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/domains/recordings/recordings.route.ts` around lines 113 - 127, The syncRecording service call result is not being captured and the response is returning the raw client-provided relativePath instead of the canonical transformed path used by the service. Modify the code to capture the return value from the syncRecording function call and use the transformed path returned from that service call in the JSON response data object where relative_path is set, rather than using the untransformed relativePath variable from the request.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/domains/recordings/recordings.route.ts`:
- Around line 113-127: The syncRecording service call result is not being
captured and the response is returning the raw client-provided relativePath
instead of the canonical transformed path used by the service. Modify the code
to capture the return value from the syncRecording function call and use the
transformed path returned from that service call in the JSON response data
object where relative_path is set, rather than using the untransformed
relativePath variable from the request.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f06d9d72-834a-4830-8a22-812f85b46076
📒 Files selected for processing (1)
src/domains/recordings/recordings.route.ts
Joel-Joseph-George
marked this pull request as draft
June 22, 2026 10:26
This was referenced Jun 28, 2026
Closed
21 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pr is made in response with the recorded audio being synced with the server first. then it will be synced to the cloudfare and keeps a record of it in our database .
Summary by CodeRabbit
Release Notes
New Features
POST /recordings/syncendpoint to upload.m4arecordings (multipart form) and persist them to the backend.Chores