mirror of
https://github.com/yusufipk/OpenFrame.git
synced 2026-09-11 17:46:06 +00:00
yusufipk
0187db5dc7
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.
119 lines
4.2 KiB
Markdown
119 lines
4.2 KiB
Markdown
# Contributing to OpenFrame
|
|
|
|
Thanks for taking the time to contribute.
|
|
This guide covers setup, PR expectations, and required conventions.
|
|
|
|
## Local setup
|
|
|
|
1. Install dependencies.
|
|
|
|
```bash
|
|
bun install
|
|
```
|
|
|
|
2. Copy environment variables.
|
|
|
|
```bash
|
|
cp .env.example .env
|
|
```
|
|
|
|
3. Generate Prisma client.
|
|
|
|
```bash
|
|
bun run db:generate
|
|
```
|
|
|
|
4. Run validation.
|
|
|
|
```bash
|
|
bun run check
|
|
```
|
|
|
|
## Running the tests
|
|
|
|
The testing stack, layout, and conventions live in [TESTING.md](TESTING.md). Read it before adding a test, and see the "When a change needs a test" table in [AGENTS.md](AGENTS.md) for which layer your change belongs in. A bug fix always needs a test that fails before the fix.
|
|
|
|
| Command | Runs | Needs the test database |
|
|
| ------------------ | -------------------------------------------------- | ----------------------- |
|
|
| `bun run test` | unit and component suites | no |
|
|
| `bun run test:api` | API integration suites | yes |
|
|
| `bun run test:e2e` | Playwright end-to-end specs | yes |
|
|
| `bun run verify` | `bun run check` plus the unit and component suites | no |
|
|
|
|
`scripts/test.sh mutation` is the other one worth knowing about. It runs StrykerJS over the authorization and validation modules, breaking one line at a time to find tests that pass either way. It takes minutes rather than seconds, so it is not in `all` and CI runs it weekly; reach for it after writing a batch of tests. It needs node rather than bun, which the script handles.
|
|
|
|
The test database is a disposable Postgres defined in `docker-compose.test.yml`, on port `55432` so it cannot collide with your dev stack.
|
|
|
|
```bash
|
|
bun run test:db:up
|
|
bun run test:api
|
|
bun run test:db:down
|
|
```
|
|
|
|
`scripts/test.sh <unit|api|e2e|all>` does all of that in one command. It runs each suite inside a container, so no package manager runs on your host, and it starts the test database first when the suite needs one.
|
|
|
|
```bash
|
|
./scripts/test.sh unit
|
|
./scripts/test.sh all
|
|
```
|
|
|
|
The `pre-push` hook runs `bun run verify`, so lint, format, typecheck, and the unit and component suites have to pass before a push leaves your machine. `bun run test:api` is deliberately not in the hook, because it needs the database container. Run it yourself when you change an API route.
|
|
|
|
## Contribution workflow
|
|
|
|
1. Fork and create a branch from `master`.
|
|
2. Keep changes focused on one logical concern.
|
|
3. Follow repository conventions in this file.
|
|
4. Run required checks locally.
|
|
5. Open a PR with a clear summary and checklist.
|
|
|
|
## Branch naming
|
|
|
|
- `feature/<short-topic>`
|
|
- `fix/<short-topic>`
|
|
- `docs/<short-topic>`
|
|
- `refactor/<short-topic>`
|
|
- `chore/<short-topic>`
|
|
|
|
## Commit and PR title standard
|
|
|
|
Use Conventional Commits with this pattern:
|
|
|
|
```text
|
|
type(scope): short summary
|
|
```
|
|
|
|
## Required checks before opening a PR
|
|
|
|
- Run `bun run check`.
|
|
- Run `bun run verify`, or let the `pre-push` hook run it for you.
|
|
- If you changed an API route, also run `bun run test:api` with the test database up.
|
|
- If `prisma/schema.prisma` changed, run `bun run db:generate`.
|
|
- Ensure no unrelated file changes are included.
|
|
- Ensure no secrets or private keys are committed.
|
|
- Update docs when behavior changes.
|
|
|
|
## Project conventions (must follow)
|
|
|
|
- Use Bun commands only.
|
|
- Server-side session reads must use `auth()` from [lib/auth.ts](lib/auth.ts).
|
|
- Access control should use `checkProjectAccess()` / `checkWorkspaceAccess()`.
|
|
- API responses should use `successResponse` / `apiErrors` from [lib/api-response.ts](lib/api-response.ts).
|
|
- In App Router dynamic routes, keep `params` typed as `Promise<...>` and use `await params`.
|
|
- For multi-step DB writes, use Prisma transactions.
|
|
- Prefer backward-compatible API changes unless a breaking change is explicitly requested.
|
|
- Prefer `@/` imports when available.
|
|
|
|
## Security issues
|
|
|
|
Do not open public issues for vulnerabilities.
|
|
Follow [SECURITY.md](SECURITY.md).
|
|
|
|
## Code of conduct
|
|
|
|
Follow [CODE_OF_CONDUCT.md](CODE_OF_CONDUCT.md).
|
|
|
|
## Need help?
|
|
|
|
If you are unsure where to start, open an issue with context and a proposed approach.
|