bvandeusenandClaude Opus 5
dbc4e8b0c6
CI / lint (push) Successful in 2s
CI / extension-version (push) Successful in 3s
Build images / sign-extension (push) Successful in 4s
Build images / build-ml (push) Successful in 6s
Build images / build-agent (push) Successful in 8s
Build images / build-web (push) Successful in 7s
CI / frontend-build (push) Successful in 20s
CI / backend-lint-and-test (push) Successful in 31s
CI / integration (push) Successful in 3m52s
The repo had no LICENSE, which meant all rights reserved by default: nobody could legally run or modify it, and "public availability" was a contradiction no amount of install documentation could fix. This is the hard blocker in milestone 328; everything else in it is quality. AGPL-3.0, at the operator's explicit choice, for the reason the operator gave: this should not become something another party runs as a hosted proprietary service. Section 13 is what makes it fit — for a self-hosted web app, distribution otherwise never happens, so the GPL's obligation would never actually bite. AGPL reaches the case that matters here: running a MODIFIED copy as a service for others. LICENSE is the FSF text fetched from gnu.org and verified byte-identical (34,523 bytes, 661 lines, §13 "Remote Network Interaction" present), not retyped. README's old "Personal project; use at your own discretion" said nothing legally and is replaced with what the licence actually asks — including the part worth being clear about, that running an unmodified copy for yourself carries no obligation whatsoever. SECURITY.md names what this software actually holds, because that is what makes a report serious here: live third-party session cookies for accounts with payment methods attached, the extension API key, the multi-user sharing ACL, and arbitrary downloaded media that gets decoded and fed to models. It also states the plain-HTTP posture up front, so "served over HTTP" and "no HSTS" are understood as the documented design rather than filed as findings. There is no private disclosure channel yet, so the reporting instruction is to open an issue containing NOTHING but the fact that a report exists, and wait for a private contact. Awkward on purpose: an issue tracker is public the moment it is written to, and every self-hosted instance stays vulnerable until its operator can update. Worth replacing with a real contact address — that decision is the operator's, since it publishes one. CONTRIBUTING records the two things that actually catch people: ruff's order-by-type import sorting, and that a model change and its migration belong in the same commit. The second is not style — the models and the chain silently diverged for a long time (#3275) and autogenerate was unsafe as a result. Pre-publication scan, since the repo is about to get attention: .env.example is placeholders only (`changeme_*`), `.env` is gitignored with an `!.env.example` exception, no credential-shaped literals are committed, and there are no private IPs or operator home paths. The only hostnames are the project's own forge, which is public by design. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017QHszn9H8VBvx5Ke8x1hvw
78 lines
3.2 KiB
Markdown
78 lines
3.2 KiB
Markdown
# Contributing
|
|
|
|
FabledCurator is developed by a single maintainer for their own use, and
|
|
published because it may be useful to others. That shapes what contribution
|
|
looks like here.
|
|
|
|
**Issues are welcome** — bug reports, and questions about running it, are
|
|
genuinely useful and often the fastest way to find out that something is
|
|
broken outside the one environment it was built in.
|
|
|
|
**Open an issue before writing a pull request.** Not as a formality: the
|
|
project has opinions that are not obvious from the code, and it is unpleasant
|
|
for everyone when a finished patch turns out to conflict with one. A short
|
|
issue first costs you nothing and may save you an evening.
|
|
|
|
**Contributions are licensed under the AGPL-3.0**, like the rest of the
|
|
project. By submitting one you agree it ships under that licence. There is no
|
|
CLA and no copyright assignment.
|
|
|
|
## Running it for development
|
|
|
|
```bash
|
|
docker compose up -d # UI on http://localhost:8080
|
|
```
|
|
|
|
The dev override (`docker-compose.override.yml`) is auto-merged and builds the
|
|
app images locally from source, so this needs no `.env` and no registry
|
|
access. Postgres and Redis ports are exposed on the host.
|
|
|
|
## What CI checks
|
|
|
|
Every push runs these, and they are the definition of done for a change:
|
|
|
|
```bash
|
|
ruff check backend/ tests/ alembic/ agent/ scripts/ # lint (and import order)
|
|
pytest tests/ -m "not integration" # backend unit tests
|
|
pytest tests/ -m integration # needs pgvector + redis
|
|
cd frontend && npm run test:unit && npm run build # frontend
|
|
```
|
|
|
|
The integration lane builds its schema by running the real migrations
|
|
(`alembic upgrade head`), never from ORM metadata — so a migration that does
|
|
not apply cleanly fails CI rather than being discovered later.
|
|
|
|
Note for the linter: ruff's isort runs with `order-by-type`, which sorts
|
|
ALL-CAPS names ahead of CamelCase. `from sqlalchemy import JSON, DateTime, ...`
|
|
is correct; putting `JSON` alphabetically between `Integer` and `String` is
|
|
not. This catches people out.
|
|
|
|
## Database changes
|
|
|
|
The ORM models and the migration chain must agree. This is enforced, and it is
|
|
enforced because they silently diverged for a long time and nobody noticed
|
|
until they were compared: the models were missing indexes, defaults and
|
|
uniqueness guarantees that only ever existed inside a migration, which made
|
|
`alembic revision --autogenerate` actively unsafe to run.
|
|
|
|
So: if you change a model, write the migration; if you write a migration,
|
|
change the model to match. Both, in the same commit.
|
|
|
|
Adding a value to a CHECK-constrained column means swapping the constraint in
|
|
the same change — the constraint is not documentation, and a new value without
|
|
it fails at insert time.
|
|
|
|
## Branch model
|
|
|
|
`dev` is where work happens. `main` is production and is only reached by a
|
|
merge from `dev`, never pushed to directly. If you are sending a pull request,
|
|
target `dev`.
|
|
|
|
## Style
|
|
|
|
Match the surrounding code. The one convention worth stating explicitly is
|
|
that comments here explain *why*, especially where a choice looks wrong at a
|
|
glance — a comment recording which migration a constraint came from, or why a
|
|
default is a `text()` rather than a string, is the kind that has repeatedly
|
|
turned out to be worth its space.
|