feat: per-verse audio recordings (upload/serve for mobile) - #224
Conversation
The dynamic import('@/app') pays every route module's transform cost under
parallel collect; the 5s default started flaking when the app graph gained
@azure/storage-blob. Verified against origin/main baseline (38.5s vs 44.8s
aggregate collect).
|
@coderabbitai full review |
There was a problem hiding this comment.
Pull request overview
Adds a new verse-audio domain to support per-verse translator audio recordings for the Fluent mobile app, including upload/replace, playback via short-lived SAS URLs, and deletion, backed by Azure Blob Storage and a new database table.
Changes:
- Introduces
verse-audioroutes + auth middleware and service/repository layers for upload/get/list/delete flows. - Adds Azure Blob storage integration (
src/lib/audio-storage.ts) and a DB table + migration (verse_audio_recordings) with a per-(projectUnitId,bibleTextId) uniqueness constraint. - Updates tooling/test harness (doc route test timeout; eslint/prettier ignores; env example + env schema) and adds Azure SDK dependency.
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/routes/doc.route.test.ts | Increases /doc OpenAPI test timeout to reduce flakiness from larger import graph. |
| src/lib/types.ts | Adds error codes/messages/statuses for verse-audio and upload validation. |
| src/lib/audio-storage.ts | Implements Azure Blob upload/delete and 15-minute read-only SAS URL generation for verse audio. |
| src/lib/audio-storage.test.ts | Adds unit tests for blob naming and unconfigured-storage behavior. |
| src/env.ts | Adds optional AZURE_STORAGE_CONNECTION_STRING and AUDIO_CONTAINER env vars. |
| src/domains/verse-audio/verse-audio.types.ts | Defines limits, content-type allowlist, domain types, and OpenAPI schemas. |
| src/domains/verse-audio/verse-audio.service.ts | Implements upload/get/list/delete orchestration (blob + DB) and URL attachment. |
| src/domains/verse-audio/verse-audio.service.test.ts | Adds service-level tests with storage/repo mocked. |
| src/domains/verse-audio/verse-audio.route.ts | Adds PUT/GET/DELETE /verse-audio/{projectUnitId}/{bibleTextId} and chapter listing GET /verse-audio. |
| src/domains/verse-audio/verse-audio.repository.ts | Adds Drizzle queries for get/list/upsert/remove of verse-audio metadata. |
| src/domains/verse-audio/verse-audio-auth.middleware.ts | Enforces project membership / chapter assignment policy and masks forbidden as 404. |
| src/db/schema.ts | Adds verse_audio_recordings table definition to Drizzle schema. |
| src/db/migrations/0015_add_verse_audio_recordings.sql | Creates the verse_audio_recordings table and unique index. |
| src/db/migrations/meta/0015_snapshot.json | Drizzle migration snapshot for the new table. |
| src/db/migrations/meta/_journal.json | Registers migration 0015_add_verse_audio_recordings. |
| src/app.ts | Registers the new verse-audio routes into the app. |
| package.json | Adds @azure/storage-blob dependency. |
| package-lock.json | Locks Azure SDK dependency graph. |
| eslint.config.mjs | Ignores .superpowers/** during linting. |
| .prettierignore | Ignores .superpowers during formatting. |
| .env.example | Documents new env vars for verse-audio storage configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
kaseywright
left a comment
kaseywright
left a comment
There was a problem hiding this comment.
The decision has been made to use CloudFlare R2 as cloud storage for the project. We should ensure that the GDPR jurisdiction is set so that the files at rest in the R2 buckets are located in known regions in Europe to start with. Some of these settings might be out of scope for this PR while integrating CloudFlare.
| ALTER TABLE "verse_audio_recordings" ADD CONSTRAINT "verse_audio_recordings_project_unit_id_project_units_id_fk" FOREIGN KEY ("project_unit_id") REFERENCES "public"."project_units"("id") ON DELETE cascade ON UPDATE cascade;--> statement-breakpoint | ||
| ALTER TABLE "verse_audio_recordings" ADD CONSTRAINT "verse_audio_recordings_bible_text_id_bible_texts_id_fk" FOREIGN KEY ("bible_text_id") REFERENCES "public"."bible_texts"("id") ON DELETE no action ON UPDATE no action;--> statement-breakpoint | ||
| ALTER TABLE "verse_audio_recordings" ADD CONSTRAINT "verse_audio_recordings_uploaded_by_users_id_fk" FOREIGN KEY ("uploaded_by") REFERENCES "public"."users"("id") ON DELETE no action ON UPDATE no action;--> statement-breakpoint | ||
| CREATE UNIQUE INDEX "uq_verse_audio_per_bible_text" ON "verse_audio_recordings" USING btree ("project_unit_id","bible_text_id"); No newline at end of file |
There was a problem hiding this comment.
project_unit_id cascades on delete (good), but bible_text_id has ON DELETE no action. Deleting a bible_texts row that has recordings will fail with a FK violation. If that's intentional (preserve recordings), the Azure blob is still orphaned since the DB row blocks cleanup. If verses can be deleted, consider ON DELETE cascade for bible_text_id too — but note the blob won't be cleaned up automatically either way (no DB-triggered blob deletion). Worth documenting the intended lifecycle.
Typically, bible verses will not be deleted but we should implement a mechanism to avoid orphaning recordings. I am open to a separate table to track recordings and their cloud storage locations - this can be modifying a current table or a new one. Whichever makes the most sense.
There was a problem hiding this comment.
agreed, and to state the current lifecycle plainly: bible_texts rows aren't deleted in practice so the no-action FK is really just a guard, but deleting a project_unit does cascade the rows away and leaves the blobs sitting under unit-{id}/, so today storage can definitely orphan. cascades alone can't fix that since nothing in postgres deletes a blob, so yeah, a tracking mechanism is the right call rather than leaning on FK behavior. i'd rather land it together with the azure to r2 migration this PR needs anyway per your other review, so that whatever we track records the r2 bucket and key from the start instead of azure containers we're about to drop. two directions i see: a storage_objects table (bucket, key, created_at, deleted_at) that verse_audio_recordings references and deliberately does not cascade, so a surviving row is the marker a sweeper uses to reclaim the blob, or a delete-tombstone queue written by a db trigger, which has the advantage of firing on cascades too, drained by a worker. which would you rather have? happy to build either, just want your call before i wire it up.
There was a problem hiding this comment.
A storage_objects table is probably the best way here. Although I like DBMS features, triggers usually get lost because they are not visible in the application code. Without a dedicated DB admin, it's best to stick with highly visible changes.
There was a problem hiding this comment.
that settles it, and agreed on the reasoning, a trigger that fires on cascade is great right up until someone greps the app code for what deletes blobs and finds nothing, which with no dedicated dba is exactly how it rots. so it'll be a storage_objects table holding bucket, key, created_at, deleted_at, referenced by verse_audio_recordings and deliberately not cascading, so when a recording row goes away the storage row survives as the marker a reclaim sweeper picks up. it lands together with the azure to r2 swap this PR still needs, so the very first rows written record r2 bucket and key instead of azure containers we're about to drop.
There was a problem hiding this comment.
built it, 3a9013f.
storage_objects is in (bucket, key, created_at, deleted_at) with verse_audio_recordings referencing it and deliberately no cascade, so when a project unit gets dropped the recording rows go but the storage rows stay behind as the markers. a sweep on an interval deletes those objects and stamps deleted_at, and if r2 is unconfigured it doesn't even start. one detail i went with: the storage row is claimed before the object is written, so a crash between the two leaves a reclaimable marker rather than bytes nothing knows about.
same commit moves this branch off azure blob onto r2, eu jurisdiction endpoint, presigned playback urls still 15 min. env vars now match the export pr (R2_ACCOUNT_ID / R2_ACCESS_KEY_ID / R2_SECRET_ACCESS_KEY / R2_JURISDICTION), with a separate R2_AUDIO_BUCKET since exports get ttl-swept and recordings shouldn't share a bucket with that. creating the bucket in the eu jurisdiction is still the one bit code can't do, it's in .env.example as the infra step.
193 tests green.
Every other error response on these routes returns { message } via
createMessageObjectSchema; the storage-unavailable 503 returned
{ error, details }, so clients had to handle two error shapes from the
same four routes. The shape was inherited from usfm.route.ts, which uses
{ error, details } for all of its errors and is internally consistent —
verse-audio was not. Status and "storage not configured" semantics are
unchanged; only the body shape and its OpenAPI schema move.
|
Warning Review limit reached
Next review available in: 24 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughReplaces Azure Blob Storage with Cloudflare R2 for verse audio. Adds storage-object persistence, orphan reclamation, authorized audio routes, updated audio contracts and errors, and startup cleanup scheduling. ChangesVerse audio
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant VerseAudioRoutes
participant AccessMiddleware
participant VerseAudioService
participant R2
participant Database
Client->>VerseAudioRoutes: Send verse audio request
VerseAudioRoutes->>AccessMiddleware: Authenticate and authorize scope
AccessMiddleware-->>VerseAudioRoutes: Allow request
VerseAudioRoutes->>VerseAudioService: Execute upload, read, list, or delete
VerseAudioService->>R2: Store, remove, or presign audio object
VerseAudioService->>Database: Claim or update storage and recording metadata
Database-->>VerseAudioService: Return persistence result
VerseAudioService-->>VerseAudioRoutes: Return service result
VerseAudioRoutes-->>Client: Return HTTP response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/domains/verse-audio/verse-audio.service.ts`:
- Around line 38-61: In src/domains/verse-audio/verse-audio.service.ts lines
38-61, serialize upload mutations per (projectUnitId, bibleTextId) and persist a
recoverable upload state before blob changes so a failed repo.upsert can be
reconciled or retried. In src/domains/verse-audio/verse-audio.service.ts lines
97-114, make deletion conditional on the recording version/state observed by the
request, persist a durable deletion state before removing the blob, and retry
cleanup without deleting newer uploads. Add coverage for concurrent
upload/delete interleavings and database failures after blob operations.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a3968741-d8a6-470c-8d43-6476f2d0ad10
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (21)
.env.example.gitignore.prettierignoreeslint.config.mjspackage.jsonsrc/app.tssrc/db/migrations/0015_add_verse_audio_recordings.sqlsrc/db/migrations/meta/0015_snapshot.jsonsrc/db/migrations/meta/_journal.jsonsrc/db/schema.tssrc/domains/verse-audio/verse-audio-auth.middleware.tssrc/domains/verse-audio/verse-audio.repository.tssrc/domains/verse-audio/verse-audio.route.tssrc/domains/verse-audio/verse-audio.service.test.tssrc/domains/verse-audio/verse-audio.service.tssrc/domains/verse-audio/verse-audio.types.tssrc/env.tssrc/lib/audio-storage.test.tssrc/lib/audio-storage.tssrc/lib/types.tssrc/routes/doc.route.test.ts
Agent tooling checks out other branches into .claude/worktrees/<branch>/ — full repo copies whose suites vitest collected, reporting another branch's 29 failures as ours. Excluded there and gitignored; unscoped npm test is 183/183 again. CI unaffected (clean checkouts have no .claude/).
Storage: swaps Azure Blob for R2 (S3-compatible) behind the same exported surface, pinned to the EU jurisdiction endpoint for GDPR data-at-rest. The presigner is async, so generateAudioDownloadUrl now returns a promise. Orphan reclaim (reviewer call on #224): a storage_objects table records every object written (bucket, key, timestamps) and verse_audio_recordings references it WITHOUT cascading. Dropping a project unit cascades the recordings away but Postgres cannot delete an object, so the storage row survives as the marker a periodic sweep uses to delete the bytes and stamp deletedAt. Chosen over a DB trigger because triggers are invisible from application code. The claim is written before the upload, so a crash between the two leaves a reclaimable marker rather than untracked bytes.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/domains/verse-audio/verse-audio.service.ts`:
- Around line 116-139: Make deletion and reclamation mutually exclusive with
claim/re-upload: in src/domains/verse-audio/verse-audio.service.ts:116-139,
perform a durable recoverable unlink/delete transition via the repository before
deleting object bytes; in
src/domains/verse-audio/storage-objects.repository.ts:24-44, make claim honor an
in-progress reclamation lease/state; in
src/domains/verse-audio/storage-objects.repository.ts:69-89, atomically reserve
orphan rows; and in src/domains/verse-audio/verse-audio.service.ts:149-169,
delete only objects held by that reservation. Add coverage for database-remove
failure and reclaim-versus-reupload interleavings.
In `@src/env.ts`:
- Around line 60-76: The storage configuration lacks a way to target MinIO. In
src/env.ts lines 60-76, add an optional URL-validated S3 endpoint setting; in
src/lib/audio-storage.ts lines 54-57, update the client configuration to prefer
that endpoint when provided and otherwise retain the derived R2 endpoint.
In `@src/index.ts`:
- Around line 48-57: Update startup initialization around initializeAudioStorage
and audioReclaimInterval to await the R2 probe before scheduling the reclaim
worker. Only enable the interval after the probe succeeds, while preserving the
existing disabled behavior when audio storage is unconfigured; allow invalid
credentials, bucket names, or jurisdictions to fail during startup.
In `@src/lib/audio-storage.ts`:
- Around line 70-77: Configure the S3Client construction in audio storage with a
requestHandler that enforces finite connection and socket timeouts for all R2
operations. Preserve the existing region, endpoint, path-style addressing, and
credentials, and use the AWS Node HTTP handler appropriate for the project’s SDK
version.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 44332c46-6ad8-4975-931a-7db266bd43d2
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (17)
.env.example.gitignorepackage.jsonsrc/db/migrations/0016_add_storage_objects.sqlsrc/db/migrations/meta/0016_snapshot.jsonsrc/db/migrations/meta/_journal.jsonsrc/db/schema.tssrc/domains/verse-audio/storage-objects.repository.tssrc/domains/verse-audio/verse-audio.repository.tssrc/domains/verse-audio/verse-audio.service.test.tssrc/domains/verse-audio/verse-audio.service.tssrc/domains/verse-audio/verse-audio.types.tssrc/env.tssrc/index.tssrc/lib/audio-storage.test.tssrc/lib/audio-storage.tsvitest.config.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- .gitignore
Review follow-ups on the storage-objects work: - deleteRecording now removes the row BEFORE the object. A failed row write destroys nothing and can be retried; a failed object delete leaves an orphan the sweep collects. The old order could strand a recording pointing at bytes that were already gone, which no sweep can repair. - The reclaim sweep skips rows younger than AUDIO_RECLAIM_GRACE_MS, and claim() refreshes createdAt on revive. An upload claims its row before writing the object, so without this a sweep landing mid-upload could delete live bytes. - initializeAudioStorage() is now actually called: the sweep only starts once the bucket answers. Audio is optional, so a failed probe logs and leaves the API serving rather than killing startup. - S3 client gets connection/request timeouts (the SDK defaults to none, so an unreachable endpoint would hang uploads and the sweep indefinitely). - R2_ENDPOINT override makes the documented MinIO local-dev path real. - Drops @aws-sdk/lib-storage: recordings are capped at 30 MB and buffered, so PutObject is used and the multipart helper was never imported.
|
This looks good. I'm happy to merge as soon as we have the r2 configuration in place. |
# Conflicts: # package-lock.json # package.json # src/index.ts
kaseywright
left a comment
kaseywright
left a comment
There was a problem hiding this comment.
Request changes: bucket-name defaults risk silently mixing/overwriting user audio across environments
Since #212 landed, R2_ACCOUNT_ID / R2_ACCESS_KEY_ID / R2_SECRET_ACCESS_KEY are now shared between the exports and verse-audio R2 integrations (src/env.ts), which is a good consolidation. But each feature still has its own bucket var with a hardcoded literal default:
R2_EXPORTS_BUCKET: z.string().default('usfm-exports'),
R2_AUDIO_BUCKET: z.string().default('verse-audio'),The problem: if the same R2 account is ever reused across environments (dev/staging/prod — a common cost-saving setup) and an environment's deploy config omits R2_AUDIO_BUCKET, that environment doesn't fail to start — it silently falls back to the literal bucket name "verse-audio". If another environment is also missing the override (or was configured before this var existed), both now point at the same real bucket. Because verse-audio object keys are deterministic (unit-{id}/text-{id} in audio-storage.ts), this isn't a "wrong bucket, 404" failure — it's two environments silently reading and overwriting the same objects. A dev/staging upload would clobber production audio for the same verse, with no error anywhere.
This is worse than a missing-bucket error because it's silent and destructive rather than loud and safe.
Compounding gap: even when the bucket genuinely doesn't exist, the failure mode is soft. initializeAudioStorage()'s HeadBucketCommand check in src/index.ts is wrapped in try/catch — a failure just logs and disables the reclaim sweep; it doesn't stop the server or gate the routes. The PUT/GET/DELETE verse-audio handlers only check isAudioStorageConfigured() (credential presence), never bucket reachability, so a bad bucket name surfaces as scattered per-request 500s instead of a clean boot-time refusal.
Requested change: drop the hardcoded .default(...) on R2_AUDIO_BUCKET and R2_EXPORTS_BUCKET. Require both explicitly whenever R2 credentials are configured (e.g. EnvSchema.superRefine, or an explicit check at startup) so a missing bucket var fails the boot loudly instead of quietly resolving to a name another environment might already own. Whatever the specific mechanism, no environment should be able to silently inherit a bucket name it didn't explicitly choose — that's the only thing that actually closes the collision risk, independent of which string is picked as a "default."
Everything else in this pass (the credential consolidation itself) looks correct and I don't have other blocking concerns.
R2 credentials are account-level, so environments pointed at the same account
are separated by nothing but their bucket names. R2_EXPORTS_BUCKET and
R2_AUDIO_BUCKET each carried a hardcoded default, so an environment whose
deploy config omitted one still booted, on the fallback name. Audio keys are
deterministic (unit-{id}/text-{id}), so two environments landing on the same
name would read and OVERWRITE each other's recordings rather than miss and
404 — silent and destructive instead of loud and safe (kaseywright, #224).
Both vars are now optional in the schema and required by a superRefine as soon
as any R2 credential is present, so a missing one fails env validation before
anything starts. audioBucket()/exportsBucket() are the single accessors that
narrow the type, with that invariant documented in place rather than asserted
at each call site.
An unreachable bucket stays non-fatal, per the #212 call that R2 trouble must
not take the whole API down: initializeAudioStorage now records its probe
result and the verse-audio routes read it through isAudioStorageAvailable, so
a bad bucket answers one clean 503 instead of a 500 per request.
.env.example documents the real per-environment bucket names, since the
invented ones were exactly the trap.
Refs: #224
Claude-Session: https://claude.ai/code/session_016ej1G6uozJyR5TYwypKxfz
|
dropped both defaults in d6f3190. on the compounding gap, I kept your #212 call intact rather than trading one for the other: a missing env var now fails boot, but an unreachable bucket still does not take the API down.
|
kaseywright
left a comment
kaseywright
left a comment
There was a problem hiding this comment.
Approving — the bucket-default issue is resolved
Commit d6f3190 directly fixes the silent cross-environment collision risk from my previous review:
R2_EXPORTS_BUCKET/R2_AUDIO_BUCKETno longer have hardcoded defaults — both are optional in the base schema, enforced by asuperRefine(requireExplicitR2Buckets) that fails env validation at boot if any R2 credential is set but a bucket name is missing or blank. An environment can no longer silently inherit another environment's bucket by omission; it fails loudly instead..env.examplenow documents real per-environment bucket names instead of inviting-looking placeholders.- Bonus fix for the compounding gap I flagged as secondary: the verse-audio routes previously only checked credential presence, so an unreachable bucket surfaced as scattered 500s.
initializeAudioStorage()now records its boot-time probe result, and the routes check a newisAudioStorageAvailable()so a bad bucket now answers a clean 503 instead. - Good, targeted test coverage:
src/env.test.ts(new) exercises the superRefine directly — missing creds+bucket, blank bucket, partial credentials, fully-specified — andaudio-storage.test.tscovers the availability-flag behavior.
Verified locally: env.test.ts, audio-storage.test.ts, verse-audio.service.test.ts, and blob-storage.test.ts all pass, and tsc --noEmit is clean.
No remaining concerns from me. Nice, precise fix.
Summary
Backend support for per-verse translator audio recordings, built for the Fluent mobile app: upload through the API into Cloudflare R2, metadata in Postgres, playback via short-lived presigned URLs.
PUT /verse-audio/{projectUnitId}/{bibleTextId}CONTENT_UPDATE+ChapterAssignmentPolicy.edit(same gate as editing the verse text)file(+ optionaldurationSeconds); create-or-replace; returns metadata +downloadUrlGET /verse-audio/{projectUnitId}/{bibleTextId}PROJECT_VIEW+ project membershipdownloadUrlGET /verse-audio?projectUnitId=&bookId=&chapterNumber=DELETE /verse-audio/{projectUnitId}/{bibleTextId}Design decisions (spec'd before implementation):
verse_audio_recordingsunique on(project_unit_id, bible_text_id); re-upload replaces in place. Blob names are deterministic (unit-{id}/text-{id}), so replacement never orphans a blob.bodyLimit), 8-type audio MIME allowlist, blob-first-then-row write ordering (drift windows documented in code); playback streams directly from storage, no audio bytes proxied through the API.translated-versesexactly — forbidden access masked as 404 per repo convention; IDs travel in path/query only, never the multipart body; all routes declare 401/403 (and reachable 400s) in their OpenAPI response maps. Mobile BetterAuth sessions work unchanged.Screenshots
1) New "Verse Audio" group in the API reference (
/reference)All four endpoints registered in Scalar: upload (PUT), get (GET), delete (DEL), and the chapter listing (GET
/verse-audio).2) Full PUT upload contract
Path params, multipart body (
file+ optionaldurationSeconds), and every declared response — 200 withdownloadUrl, 400/401/403/404, 413 (30 MB cap), 500, 503 (storage unconfigured).3) Chapter-listing endpoint contract
Query params (
projectUnitId,bookId,chapterNumber) and the verse-ordereditemsresponse the mobile app uses for one-call chapter playback.4) SAS URL streaming in the browser's native player
The
downloadUrlfrom a real upload, opened directly: Chrome's built-in audio player streaming the 3-second recording (shown mid-playback at 0:01/0:03) straight from Azurite — no bytes proxied through the API.Config & deploy
R2_ACCOUNT_ID/R2_ACCESS_KEY_ID/R2_SECRET_ACCESS_KEY(new, optional) — when any is unset the audio routes return503 {message}and nothing else is affected.R2_JURISDICTION(defaulteu) — pins the endpoint to{account}.eu.r2.cloudflarestorage.comso data at rest stays in the EU. Creating the bucket in that jurisdiction is an infra step code cannot do (dashboard/IaC); documented in.env.example.R2_AUDIO_BUCKET(defaultverse-audio) — deliberately not the exports bucket, whose contents are TTL-swept. Recordings are permanent.AUDIO_RECLAIM_INTERVAL_MS(default 1h) — how often orphaned objects are reclaimed..env.example).0015_add_verse_audio_recordings— additive only, no backfill, trivially reversible.Related work
feat/196-async-export-hardening— the async-export storage work, also now on R2. Still untouched by this branch: audio keeps its ownsrc/lib/audio-storage.ts(distinct filename ⇒ clean merges in either order) because the two have different buckets and lifecycles. Consolidating a shared R2 module is a follow-up once both land.ft/audio-record-sync) — a parallel approach to recording sync (POST /recordings/sync,.m4a→ Cloudflare R2, standalonerecordingstable; its 0012/0013 migrations now conflict with main's journal). This PR differs deliberately: verse-anchored data model with a per-verse uniqueness invariant, chapter-assignment policy gating, full CRUD + chapter listing, and tracked storage objects with orphan reclaim. Both are now on R2, so the remaining question is which implementation wins — happy to help reconcile either way.Storage objects and orphan reclaim
Postgres cannot delete an object in a bucket, so cascades alone can never keep storage clean: dropping a project unit cascades its recordings away and would strand their audio. Following @kaseywright's call on the migration thread (a visible table beats a DB trigger, which rots without a dedicated DBA):
storage_objects(bucket,key,created_at,deleted_at) records every object written.verse_audio_recordings.storage_object_idreferences it without cascading, so when the recording row disappears the storage row survives as the marker.deleted_at. It never starts when R2 is unconfigured, and a failed delete leaves the row unstamped so the next pass retries.Tooling riders
eslint.config.mjs+.prettierignorenow skip the gitignored.superpowers/scratch dir (both tools walk the disk regardless of gitignore).doc.route.test.tsper-test timeout 5s → 20s, with an explanatory comment: its whole-appimport('@/app')pays every route module's transform cost under parallel collect, and the graph now includes the AWS S3 SDK (verified against an origin/main baseline before touching the test).Out of scope (per design) / follow-ups
Blob sweep on project-unit cascade delete— done: see storage objects below.audio/webm;codecs=opus(web MediaRecorder) are rejected — normalize before comparing if/when web recording lands.audio-storage.tsandblob-storage.tsonto one R2 module once feat(usfm): move async export to Azure Blob with SAS downloads and reliable jobs #212 lands.Testing
verse-audio.service.test.ts— 12 tests: MIME/empty-file validation, blob-then-row ordering and failure paths, playback-URL attachment, delete ordering (blob before row).audio-storage.test.ts— blob naming, unconfigured behavior.npm run precheckgreen; CIvalidate✅.set-auth-token) → three real mp3 uploads (200 each) → chapter listing returns all three verse-ordered with sizes →downloadUrlstreamsaudio/mpegin Chrome's native player (screenshot 4) →video/mp4upload correctly rejected 400. Local-dev note: Azurite must run with--skipApiVersionCheckfor SDK ≥ 12.33.Summary by CodeRabbit