Move the cursor idle logic into useCursorIdle and use it from both the video
page and the compare page, which carried its own copy. Only pointer activity
wakes the cursor now: a pause/play pair the element emits on its own (a
rebuffer, a source switch) leaves the idle state alone instead of bringing
the chrome back for a second. The fullscreen-while-paused arming is gone,
nothing rendered it. The scrub test now leaves the player before pressing
the timeline, as the real layout forces.
The idle countdown that hides the cursor and the play/pause overlay was only
started from mousemove. A cursor that stayed still over the player while a
click, a key or a scrub release started playback never got a countdown, so
the overlay stayed on the video until the mouse moved again.
Arm the timer from one place and rerun it whenever playback or fullscreen
changes, keeping the cursor-over-player state in a ref so the same rule
applies from every entry point.
Bunny and direct downloads are pulled through fetch() so we can save them
under our own filename. The browser does not treat that as a download, so
closing the tab discarded everything received so far without a word.
Register a reference counted beforeunload guard while those transfers are
in flight, and while a project manifest is being pulled file by file.
Browser owned downloads (same-origin proxy, the over-10GB fallback, asset
downloads) survive a tab close on their own and stay unguarded.
NEXT_PUBLIC_BUNNY_CDN_URL is inlined into the client bundle at build time, and
the published image is built by CI without it, so the browser had no host to
build a playlist URL from no matter what the operator set in .env.docker. The
player read the empty URL as a stream that had not finished encoding and sat on
'Video Is Processing', retrying forever.
The server knows the value on every request, so the root layout now serialises
the public settings into a JSON script tag and the browser reads them from
there, falling back to the build-time variable for source builds. The
direct-download allow list came through the same broken path and moves with it.
Closes#60
A comment held one image, and the paste handler took the first item off the
clipboard and dropped the rest. Reviewing a cut usually means several
screenshots about the same moment, which meant one comment per screenshot or
one screenshot and a paragraph describing the others. Editing a comment could
not attach anything at all: the edit box had no paste handler, no file picker
and no way to remove what was already there.
A comment now carries up to five images, in the composer, in a reply and in
the editor. One paste stages every image on the clipboard, the file picker
takes a multiple selection, and a drop lands on whichever editor is open. Over
the cap the extras are refused out loud rather than dropped quietly. A single
image still fills the width; several tile into a grid, and either opens full
screen on click.
The images move into their own table. `comments.imageUrl` stays and follows the
first of them, so a reader that has not been updated keeps working, and the
migration copies the existing attachments across so the new table is complete
from the first read. Every path that resolves a URL back to a comment now asks
the new table: R2 cleanup, the orphan sweep, the storage accounting and the
reference checks that decide whether an object can be deleted. Left on the old
column they would have treated images two through five as unreferenced and
swept them.
Detaching an image while editing only breaks the link. The file stays in R2 and
in the assets pane, which is where it is deleted from and where its bytes are
already billed.
The attachment goes up before the comment does, so a full account fails on
the image and never reaches the comment at all. Both that failure and a
rejected comment came out as "Failed to add comment", which tells the
uploader to try again, and trying again is the one thing that cannot work
when there is no room left.
Both now read out what the server said. A network fault, which has no message
anybody wants to see, still falls back to the old line.
A Bunny init asked the quota whether it could store zero bytes, which is a
question with only one answer. Nothing an upload was about to consume was
visible to the next request, so every init inside the same window read the
same total and every one of them passed, and an upload that could never
fit was only refused after it had been sent.
The client now declares the size up front. It is checked against the
account's remaining room before Bunny is asked for anything, and held as
a reservation the next init has to see. The declaration is a claim rather
than proof, so it is signed into the upload token: the same token already
binds the video id, which is what makes the reservation safe to release
on a caller's say-so, since releasing it costs them the video it belongs
to.
The declared size is then written onto the version or asset row and the
reservation is dropped in the same transaction, because Bunny reports no
size at all for a video until it has finished encoding it. On a half hour
of footage that is most of an hour during which the upload did not appear
on the uploader's own storage page and did not count against the next
upload. Per-video accounting now takes the larger of what Bunny reports
and what was declared, so the estimate stands in until the real figure
arrives and Bunny's wins once it does.
Two smaller things came out of the same reading. The asset route's
in-transaction fallback compared against the plan limit, so a caller
quoting a reservation that no longer existed was measured against 200 GiB
even on a trial worth three. And the guest branch reserves without being
able to release early, because a guest grant is bound to our video id and
the caller's network context rather than to the Bunny video, which would
let the reservation be dropped while the upload it stands for carried on.
The recording clock counted setInterval ticks, which a background tab
throttles away: a recording that kept going looked frozen at 13 seconds
and was saved with that length. It now reads the wall clock instead.
MediaRecorder also writes WebM with no usable duration. Chrome omits the
element entirely, Firefox reserves a Duration of 0.0 it never fills in,
so players had no length to show and played past the end of the seek bar.
lib/webm-duration.ts stamps the recorded length into Segment > Info when
the recording stops, in place where the browser reserved room for it.
The suite that landed in #43/#44 was written against existing behaviour, so a
number of tests pinned bugs rather than asserting correct behaviour. This fixes
the production code and moves each of those tests onto the fixed behaviour in
the same change.
Security:
- project-download: derive the archive entry extension from the last path
segment and restrict it to a short alphanumeric run, so an extensionless
allowlisted url can no longer contribute a path separator; validate the r2
branch against the strict proxy-path pattern instead of a `startsWith`, which
let `/api/upload/video/clip.mp4/../../etc/passwd` through verbatim.
- rate-limit: hash a key or action wider than its column instead of skipping the
query. Both the guard and the failing INSERT used to answer "allowed", so the
limit stopped applying entirely. Warn at startup when TRUSTED_PROXY_MODE is
unset in production.
- video uploads: the file name decides the content type; a client-declared video
mime no longer makes `payload.exe` acceptable.
- email templates: escape in the helpers rather than relying on every caller,
with an explicit `rawEmailHtml()` opt-out for the one call site that builds
markup. `escapeHtml` now covers the single quote.
- CSP: allow loopback object storage outside production only.
- route-access: reach the billing redirect only for the workspace owner. Keying
it off the owner's billing status alone made the redirect target an oracle for
whose subscription had lapsed, and sent members to a page they cannot act on.
- search: carry the same billing condition every other read path carries.
- logger: check `err.name` as well as `err.constructor.name`, so a re-thrown,
deserialised or minified Prisma error is still redacted.
- upload tokens: resolve the signing secret outside the try, so a server booted
without one fails loudly instead of reporting every grant as a forgery.
- invitations: never downgrade an existing membership, and report a scoped
invitation that points at nothing as not_found rather than accepted.
- auth: resolve the workspace role for every signed-in caller, so
checkProjectAccess and computeProjectAccess stop disagreeing about the owner
who also owns the workspace. The `intent` option is gone with it.
- r2-media-proxy: validate the object key inside the proxy so the guard travels
with the function; delete the unused, unanchored `mediaUrlToR2Key`.
- r2: sign the content type into presigned PUT grants.
Correctness:
- frame rate snapping picks the nearest standard, not the first within
tolerance, so 24, 30 and 60 fps are reachable at all.
- a version upload registers its Bunny cleanup as soon as bunny-init answers, so
a failed tus upload no longer leaves a billed video behind.
- deleting videos clears storage before the rows, so a refused DELETE leaves a
retryable row rather than an orphaned object.
- an expired upload session can be cancelled, which is what releases its quota.
- `voice/` joins the delete allowlist, so a voice note can be removed by the
module that wrote it.
- a failed CORS write propagates instead of being mistaken for an empty config
and replacing the bucket's rules.
- filtering projects by workspace no longer hides projects the unfiltered call
returns.
- upload retries skip aborts and permanent 4xx; progress no longer divides by
zero.
- reply edits no longer clear the comment's tag; optimistic resolve rolls back
to the state it replaced; the delete snapshot is captured once.
- assorted UI fixes: duplicate React keys, double-click guards reading stale
closures, the tag list fetched twice per load, a failed member list rendering
as an empty one, a stale "Initializing upload..." beside a failure, and a
registration banner pointing at an email that never arrives.
Consistency and access:
- the two download routes answer 404 for an id belonging to another tenant, as
the comment export route already did. A caller who does belong still gets 403.
- accessible names for the share-link password field, the guest name gates, the
version dialog inputs and the comment-tag controls.
Repository health:
- the runner image installs production dependencies only.
- a setup file for the unit project restores stubbed env centrally.
- native tsconfig path resolution replaces vite-tsconfig-paths.
- `uploadBytesWithProgress` exists once.
- admin stats bill Bunny storage to the workspace owner like every other
quota, gate on the configured flag, wire up the single-flight guard and count
the statuses that belonged to no bucket.
- `r2Client.destroy()` releases the presign client too.
- `prepare` tolerates a production install, where husky is absent.
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.
yusufipek
yusufipk