A Bunny guid is stored twice per row: in `VideoVersion.videoId` and
`VideoAsset.providerVideoId` on its own, and inside `originalUrl` / `sourceUrl`
as `https://iframe.mediadelivery.net/embed/<library>/<guid>`. The lookup read
only the id columns.
They are written together so they normally agree, but this query decides what
gets deleted. A row whose id column was left empty or drifted while its url
still carried the guid would present a live video as an orphan, and the script
would delete media the product is still serving. Reading both makes a
disagreement harmless instead of destructive.
Bunny rows are now read in one pass rather than filtered per candidate id: the
url match is a substring test, and there are only as many of these rows as there
are Bunny videos in the product, so one small scan beats a LIKE per id.
Two problems with running these unattended, both found while wiring the Bunny
cleanup up to a Coolify scheduled task against production.
A dry run reported a count and nothing else. "Orphaned: 31" is not something
anyone can approve: it says how many objects would go, never which. Both scripts
now list every orphan they would delete, and print the same list when deleting,
so a real run is auditable afterwards too.
Each line carries who the object belongs to, as far as each provider can answer:
- R2 reads the owner out of `videoUploadSession`, which keeps `objectKey`
alongside the initiating and billed user and survives an upload that never
became a video. That is the case producing orphans, so this is an answer
rather than a guess.
- Bunny has no equivalent. `bunny-init` sends the provider a title and nothing
else, and an orphan by definition has no row pointing at it, so there is
nothing authoritative to look up. The title is matched against titles still in
the database instead, which catches the common shape (a version upload that
failed and was retried successfully leaves a live row with the same title).
A hit prints as "possibly", because it is a hint.
The grace periods were also too short to be safe:
- Bunny counted a video abandoned after 24 hours.
- R2 counted an object abandoned after 15 minutes, which is shorter than a slow
multipart upload of a large file. An object still being written, or written
but not yet finalised into a row, looked abandoned and could be deleted out
from under the upload creating it.
Both are seven days now: long enough that no upload, retry or delayed
finalisation can still be in flight.
bun loads a plain `.env` into process.env before anything runs, and
tests/helpers/env.ts read that as a deliberate export, so it beat `.env.test`
outright. `scripts/test.sh api` therefore pointed the api suites at whatever
deployment `.env` describes: `prisma db push --accept-data-loss` for the
schema, then a truncate of every table between tests. The e2e half was worse,
because playwright.config.ts built and started the app with that DATABASE_URL
and those R2 credentials, then wrote fixtures into it. CI never saw any of
this: a runner has no `.env`.
Three changes, in order of what each one catches:
- helpers/dev-env.ts drops the values bun copied out of a development env file,
leaving `.env.test` to fill them. Only values that match the file
character-for-character go, so a real export still wins and the per-suite
databases of a parallel api run keep working.
- helpers/test-database.ts refuses a DATABASE_URL whose database name is not
marked as a test one, at the single point every path into the setup passes
through. This is the backstop, not the fix.
- playwright.config.ts blanks the variables a development env file defines and
the config does not. Dropping them from process.env is not enough there:
`next build` and `next start` run @next/env themselves and read the files
again. That is also why a local e2e run could not build at all (a set
DISABLE_RATE_LIMIT throws in lib/rate-limit.ts under NODE_ENV=production) and
why auth.spec.ts failed on a machine with SMTP configured.
`scripts/test.sh` now creates `.env.test` from the committed example instead of
asking for a one-line copy, so the guard above is something nobody has to meet.
`.dockerignore` excludes `tests`, so the production build context carries
scripts/test-db-bootstrap.ts without the tests/setup/db-global module it
imports. tsconfig includes `**/*.ts`, so the `prebuild` typecheck fails on the
missing module and every deploy since the test suites landed has died there.
CI never saw it because tests/ exists on a runner.
The script is test-only, so it belongs in the tree that is already ignored.
Second pass over the suite, driven by the inventory in the gaps document. Nine
agents wrote suites in parallel against private databases, then a tenth read all
of it adversarially and five of its findings were fixed.
unit + component 2076 -> 2079 (+888 over the round)
api 647 -> 1015
e2e 18 -> 29
What was closed:
- lib/route-access.ts, the page-level authorization layer, went from zero tests
to 48. Every API route was guarded and none of the pages were.
- The five media proxy routes now have a real 2xx beside every 403. The blocker
was the positive control, solved by stubbing r2Client.send() and leaving
lib/r2-media-proxy.ts itself real.
- Every remaining server-side lib module: invitations, email verification, the
upload tokens, the logger, request origin, the whole R2 and Bunny lifecycle,
notifications and admin stats.
- Six video-page hooks, and the chunking arithmetic extracted out of
lib/client/r2-video-upload.ts as a pure module.
- Five end-to-end flows: workspace members, bulk operations, the admin area,
player interaction and failure recovery.
Three things about the harness itself turned out to be wrong:
- Two @/lib/r2 stubs in tests/setup/api.ts had the wrong return shape, so every
route reaching finalizeR2VideoUpload silently took the "not a valid video"
branch and no test noticed.
- The auth matrix asserted only "not 2xx", which two entries satisfied without
their guard existing. It now requires 401 or 403, which makes both
load-bearing, and all 60 routes pass the stricter form.
- Both admin API routes had no positive control anywhere: replacing their guard
with an unconditional refusal left the entire suite green. Found by the
adversarial review, now covered.
Process:
- bun run test:mutation runs StrykerJS over the authorization and validation
modules. Diagnostic, not a gate, weekly in CI rather than on a push.
- playwright.config.ts gains an opt-in webkit project for the player spec.
- AGENTS.md now requires a batch of new tests to be reviewed by somebody who
did not write them.
Only two production files change, both deliberate: lib/auth.ts loses a verbatim
copy of its own permission formulas, and lib/client/r2-video-upload.ts calls the
extracted arithmetic. No behaviour change in either.
The repo had no automated tests. Every change was verified by hand.
Adds four layers, 2023 tests in total, runnable with one command:
- 1191 unit tests over the pure logic in lib/, including the full
computeProjectAccess permission matrix and the billing gate
- 167 component and hook tests in jsdom, covering the hooks that hold
real logic rather than presentational wrappers
- 647 API integration tests against a real Postgres, with only auth()
mocked, including a data-driven sweep asserting that none of the 60
route modules answers 2xx to an unauthenticated caller
- 18 Playwright specs driving a real browser against a real build
Infrastructure: vitest.config.ts with three projects, a disposable
Postgres and MinIO in docker-compose.test.yml, factories and helpers
under tests/, scripts/test.sh as the single entry point, a pre-push
hook running bun run verify, and CI split into check, test and e2e jobs.
The test database is built with prisma db push plus a replay of the
hand-written SQL, because prisma migrate deploy cannot build this schema
from empty: the migration history has no captured baseline. This mirrors
what scripts/docker-db-bootstrap.ts already does in production, and
tests/setup/db-global.ts carries a drift guard so a new migration fails
the run until someone reviews it.
Production code is unchanged apart from one pure-function extraction out
of use-video-player.ts, which was too large to test in jsdom.
Several tests pin behaviour that looks wrong, each marked KNOWN BUG in
place. TESTING.md section 12 records where the plan turned out to be
wrong, and AGENTS.md now states which layer a change needs a test in.
The audio proxy resolved ownership only through voice comments, so
R2_AUDIO video assets (which store the same proxy path in
videoAsset.sourceUrl) always got 403s. Resolve ownership the way the
image proxy does: query comments and video assets, merge into a
unique-owning-video map, deny on ambiguity.
r2-orphan-cleanup marked videoAsset.sourceUrl as referenced but not
videoAsset.thumbnailUrl, so every R2_VIDEO asset thumbnail older than
the TTL was deleted as an orphan. Widen the query to both columns.
Co-Authored-By: Claude Fable 5 <[email protected]>
- Added support for self-hosted S3 video uploads with new environment variables: OPENFRAME_ENABLE_S3_VIDEO_UPLOADS and OPENFRAME_MAX_VIDEO_UPLOAD_BYTES.
- Updated .env.example and .env.docker.example to reflect new configuration options.
- Enhanced Content Security Policy to include origins for S3-compatible storage.
- Updated dependencies for AWS SDK to support new features.
- Refactored upload logic to accommodate both Bunny and S3 upload providers.
- Updated documentation to clarify the usage of direct uploads and S3 configurations.
- Closes#11
- Added storage quota enforcement for audio and image uploads in the respective routes.
- Introduced reservation system to manage concurrent uploads and prevent quota overages.
- Enhanced comment creation to account for audio and image attachment sizes against user quotas.
- Created new UploadReservation model to track in-flight upload reservations.
- Backfilled existing video assets with size information from R2.
- Added progress component for UI feedback during uploads.
- Updated API responses to include reservation IDs for better quota management.
- Adjusted error handling to return appropriate storage limit exceeded messages.
- Introduced a new logger utility (`logError`) to standardize error logging.
- Replaced all instances of `console.error` with `logError` in various API routes and libraries.
- Enhanced error logging to sanitize sensitive information, particularly for Prisma and Stripe errors.
- Ensured consistent error handling and logging practices throughout the codebase.
- Added billing-related fields to the User model in the database.
- Implemented functions for managing billing access, including trial periods and subscription statuses.
- Created new billing utility functions for Stripe integration.
- Updated onboarding page to include billing overview and workspace creation eligibility.
- Enhanced route access checks to require billing access for certain actions.
- Implemented cleanup scripts for expired billing workspaces and associated media.
- Updated header component to conditionally show app navigation based on billing access.
- Added new migrations for billing-related database changes.
yusufipk