2 Contributing
picklemustard edited this page 2026-09-07 15:49:27 -04:00

Contributing

Setup

cp configs/config.example.yaml configs/config.yaml

make dev     # gbb-server on :8080, Vite on :5173

The catalog of modules and Godot versions lives in Postgres, not in a file, and the server will not start without it. For development:

docker run -d --name gbb-pg -p 5432:5432 \
  -e POSTGRES_USER=gbb -e POSTGRES_PASSWORD=gbb -e POSTGRES_DB=gbb postgres:16

export GBB_DATABASE_DSN='postgres://gbb:gbb@localhost:5432/gbb?sslmode=disable'
export GBB_ADMIN_BOOTSTRAP_TOKEN=dev-token

Migrations run at startup, and the bootstrap token seeds the first admin while api_tokens is empty. Populate the catalog over the admin API — see docs/admin.md. A fresh database offers nothing, so /api/v1/modules is empty until you do.

Develop against http://localhost:5173; Vite proxies the API to the Go server.

Builds need a reachable Docker daemon. On a host running podman:

systemctl --user enable --now podman.socket
export DOCKER_HOST=unix://$XDG_RUNTIME_DIR/podman/podman.sock

Without one the server still starts and logs a warning; only builds fail.

Before pushing

make check   # gofmt, go vet, go test
cd web && npm run check && npm test

CI runs the same checks and fails on unformatted Go. It is Forgejo Actions: .forgejo/workflows/ci.yml (.forgejo/workflows/ci.yml in the repo), which runs on every push and on every pull request. The pull request run tests the merge commit, not the branch head, so a branch that passes on its own can still fail once it is merged with a moved main.

Once a branch has an open pull request, its pushes stop starting a second run: the guard job stands the push run down in favour of the pull request one, which tests the merge ref. So the check to watch after opening a PR is the PR's, not the branch's. docs/ci-cd.md has the full trigger table.

make test-cover reports the same total the README's coverage badge shows.

The two test gates

Some tests need something a laptop or a CI runner may not have, so they skip rather than fail. make check passes on a machine with neither.

Variable Enables Set in CI
GBB_TEST_DSN The internal/store tests, against a real Postgres yes
GBB_TEST_DOCKER The tests that need a real Docker daemon no
GBB_TEST_DSN='postgres://gbb:gbb@localhost:5432/gbb?sslmode=disable' go test ./internal/store/...
GBB_TEST_DOCKER=1 go test ./internal/runner/... ./internal/job/...

GBB_TEST_DOCKER takes 1 for the ambient daemon (DOCKER_HOST, or the default socket) or an endpoint of its own. A podman socket works: the tests are written against the Docker-compatible API and are run against podman.

Those tests build their own toolchain image, tagged gbbtest/linux:latest so it cannot collide with what make images produces. It is busybox with a shell script standing in for scons, built from strings in internal/runner/testing.go — there is no make target and nothing to build by hand. The first run pulls busybox and therefore needs a network; later runs do not. What the tests prove is what only a daemon can: that a non-zero SCons exit becomes a failed job, and that cancelling a build removes its container.

Each store test creates and drops a schema of its own, so they do not fight over one database. CI sets GBB_TEST_DSN, which is the only place those tests actually run; it does not set GBB_TEST_DOCKER, because there is no daemon there — the in-cluster run described in docs/deployment.md is what proves the deployed arrangement instead.

CI runs in a container, not on the host, so GBB_TEST_DSN points at the Postgres service by name (postgres:5432) rather than at localhost — under Forgejo Actions the job and its services share a network instead of being port-mapped onto a runner host.

Only tests that genuinely need a daemon or a database get a gate. The queue's own tests use fakes throughout and must never skip: they are the ones that cover the lifecycle, and a lifecycle test nobody runs is not a test.

main is protected

You cannot push to main. It is a Forgejo branch protection rule, and it is deliberate: main arrives by merge so that what CI tested is what landed.

remote: Forgejo: Not allowed to push to protected branch main

is the expected answer, not a broken remote. Open a pull request. A merge needs the Go and Frontend checks green and the branch current with main.

Releases are cut from main with the Bump patch, Bump minor and Bump major actions, which build, test, then tag and publish. See docs/ci-cd.md; scripts/protect-main.sh is what applies the rule.

Conventions

Go. Standard library first — a dependency needs a reason. No cgo: the server must stay a single static binary, which is why the Postgres driver is github.com/jackc/pgx/v5, a pure-Go one. The runtime image is debian:bookworm-slim rather than distroless, but that is because the server shells out to git and ssh — not because anything is dynamically linked, and not a reason to relax the rule. Comments explain why, not what.

Container work. Remember that mount sources are resolved by the daemon, not by us — in production that daemon is a sidecar with its own filesystem. Anything a build container needs to see belongs on a named Docker volume, or on a volume mounted at the same path in both containers. See docs/deployment.md.

Toolchain arguments belong in the image, not in internal/runner. Every toolchain image's entrypoint is build/docker/entrypoint.sh, and it adds the SCons arguments that are properties of that image — which cross-compiler, which Apple SDK osxcross was built against, whether a bundle is assembled. internal/runner/scons.go renders the spec and nothing else. The reason is that the orchestrator cannot inspect the image it is about to run, so anything it hardcoded would rot the first time an image was rebuilt. Every key the entrypoint can add is in deniedSConsKeys, so a caller cannot set one too.

Unimplemented work returns an error naming the package and function, and carries a TODO(Mn): comment pointing at its milestone in ROADMAP.md. Do not leave a silent stub.

Validation happens in internal/spec before anything is cloned or executed. Refs and SCons arguments are matched against strict patterns rather than escaped, because they end up on a command line.

Frontend. Svelte 5 runes ($state, $derived, $props). Three files mirror Go and change together with it:

  • src/lib/types.ts mirrors the types in internal/spec and internal/job.
  • src/lib/validate.ts mirrors the rules in internal/spec/validate.go — its patterns and denied-key set are copied verbatim, each citing the Go line it came from.
  • src/lib/admin-types.ts mirrors the views in internal/httpapi/adminreads.go and the request bodies in admin.go. It is separate from types.ts on purpose: that one mirrors the domain, this one mirrors the HTTP layer, and the two disagree in a load-bearing place — AdminGodotVersion carries an id that GodotVersion must not, because the public route withholds it.

Adding a mirror is a decision, not a default. Each one is a standing obligation to change two files together. validate.ts earns it because the API carries no field and the build form has ten; the admin forms do not, and they copy exactly one regex (moduleNamePattern, because a module name is the primary key and cannot be changed afterwards) and leave every other rule to the server's own sentence. The mirror is what lets a bad spec mark the field it belongs to, since every API failure is a flat {"error": "..."} with no field in it (docs/api.md). Rewording an error there means updating the matcher in validate.ts; its tests assert the exact sentences.

Tests are vitest and run with no server, no database and no daemon (npm test). There is no frontend equivalent of the two gates above and there should not be: what genuinely needs a live system is the hand checklist in docs/tickets/m4-10-verification.md.

Commits

Conventional Commits. Scope is the package or area: feat(runner):, fix(httpapi):, docs(modules):.

Never commit

  • An Apple SDK, or anything under build/docker/sdk/. See docs/macos-sdk.md.
  • Real configs/*.yaml. Only the *.example.yaml files are tracked.
  • Build output, node_modules, or the contents of internal/httpapi/dist beyond the tracked placeholder.