Adopt golib/pg for migrations and pool construction #3

2026-09-02T00:17:30+10:00

unkin-agent commented

2026-09-02 00:17:30 +10:00

Why

forgebot applied its schema by executing one inline DDL string on every boot: no
version table, no lock, and no place to put the next schema change. Two API
replicas starting together both ran it. golib/pg already owns that mechanic
for the estate (adopted by arrproxy in #10), so forgebot should use it and keep
owning only the SQL.

How

Schema moves to migrations/0001_init.sql, embedded via migrations.FS. The
DDL is verbatim from internal/database/migrations.go, which is deleted.
The four legacy-status UPDATEs stay in 0001, unchanged. They are
idempotent: each reads only retired statuses (pending, failed, running,
succeeded, cancelled) and writes only current ones, and no current status
appears as a source, so nothing cascades when golib replays 0001 once against
the live database. TestMigration0001_StatusRewritesAreIdempotent pins that
property against models.TaskStatus, so a future rewrite whose target is also
a source fails the build rather than the deploy.
database.New builds the pool with pg.NewMigrated and
LockName "forgebot-migrations": replicas serialize on the advisory lock,
schema_migrations records what ran, and a migration failure closes the pool
and fails startup instead of serving a half-migrated database. database.New
and apiserver.New take a context and logger for it.
Config.DatabaseDSN renders through pg.DSN, which percent-escapes the
credentials the fmt.Sprintf builder pasted in raw — byte-identical for
values without reserved characters.
The deployed env contract is unchanged. LoadConfig still reads the
environment itself rather than calling pg.DSNFromEnv: the library has no
defaults for user and database name, where forgebot defaults both to
forgebot, and it would newly honour DATABASE_URL and the PG* variables.
TestLoadConfig_DatabaseEnvContract sets DATABASE_URL and PGHOST and
asserts they are ignored.
Tests: embedded-set vs migrations/ drift guard, the lock-key pin
(pg.LockKey("forgebot-migrations")), an additive/idempotent SQL guard, and a
columns-are-selected guard tying the tasks table to the queries in
tasks.go. This is the repo's first test package.
GOPRIVATE=git.unkin.net for the first cross-repo Go dependency: exported by
the Makefile, set in both Dockerfiles and the woodpecker Go steps,
documented in the README. No go env -w state needed.
Drive-by: gofmt on the four files that were already unformatted on main
(agentpool_types.go, providerqueue_types.go, gitea/webhook.go,
tui/app.go) — the pre-commit step runs test -z "$(gofmt -l .)" and could
not pass without it. Whitespace only.

golib pulls pgx v5.7.4 -> v5.9.2 and the usual golang.org/x bumps.

## Why forgebot applied its schema by executing one inline DDL string on every boot: no version table, no lock, and no place to put the next schema change. Two API replicas starting together both ran it. `golib/pg` already owns that mechanic for the estate (adopted by arrproxy in #10), so forgebot should use it and keep owning only the SQL. ## How - Schema moves to `migrations/0001_init.sql`, embedded via `migrations.FS`. The DDL is verbatim from `internal/database/migrations.go`, which is deleted. - **The four legacy-status `UPDATE`s stay in 0001, unchanged.** They are idempotent: each reads only retired statuses (`pending`, `failed`, `running`, `succeeded`, `cancelled`) and writes only current ones, and no current status appears as a source, so nothing cascades when golib replays 0001 once against the live database. `TestMigration0001_StatusRewritesAreIdempotent` pins that property against `models.TaskStatus`, so a future rewrite whose target is also a source fails the build rather than the deploy. - `database.New` builds the pool with `pg.NewMigrated` and `LockName "forgebot-migrations"`: replicas serialize on the advisory lock, `schema_migrations` records what ran, and a migration failure closes the pool and fails startup instead of serving a half-migrated database. `database.New` and `apiserver.New` take a context and logger for it. - `Config.DatabaseDSN` renders through `pg.DSN`, which percent-escapes the credentials the `fmt.Sprintf` builder pasted in raw — byte-identical for values without reserved characters. - **The deployed env contract is unchanged.** `LoadConfig` still reads the environment itself rather than calling `pg.DSNFromEnv`: the library has no defaults for user and database name, where forgebot defaults both to `forgebot`, and it would newly honour `DATABASE_URL` and the `PG*` variables. `TestLoadConfig_DatabaseEnvContract` sets `DATABASE_URL` and `PGHOST` and asserts they are ignored. - Tests: embedded-set vs `migrations/` drift guard, the lock-key pin (`pg.LockKey("forgebot-migrations")`), an additive/idempotent SQL guard, and a columns-are-selected guard tying the `tasks` table to the queries in `tasks.go`. This is the repo's first test package. - `GOPRIVATE=git.unkin.net` for the first cross-repo Go dependency: exported by the `Makefile`, set in both Dockerfiles and the woodpecker Go steps, documented in the README. No `go env -w` state needed. - Drive-by: `gofmt` on the four files that were already unformatted on `main` (`agentpool_types.go`, `providerqueue_types.go`, `gitea/webhook.go`, `tui/app.go`) — the pre-commit step runs `test -z "$(gofmt -l .)"` and could not pass without it. Whitespace only. `golib` pulls pgx v5.7.4 -> v5.9.2 and the usual `golang.org/x` bumps.

unkin-agent added 1 commit 2026-09-02 00:17:30 +10:00

Adopt golib/pg for migrations and pool construction

ci/woodpecker/pr/build Pipeline was successful

Details

ci/woodpecker/pr/pre-commit Pipeline was successful

Details

ci/woodpecker/pr/test Pipeline was successful

Details

8c796f4087

forgebot applied its schema by executing an inline DDL string on every boot,
with no version tracking and no lock, so two API replicas starting together
raced and the SQL had nowhere to grow. golib/pg already owns that mechanic for
the estate; take it and keep owning the SQL.

- Move the schema into migrations/0001_init.sql, embedded via migrations.FS.
  The DDL is verbatim.
- The four legacy-status UPDATEs move into 0001 unchanged. Each reads only
  retired statuses (pending/failed/running/succeeded/cancelled) and writes only
  current ones, and no current status is a source, so replaying 0001 once
  against the live database is a no-op. A test pins that property.
- Build the pool with pg.NewMigrated, LockName "forgebot-migrations": the
  advisory lock serializes replicas, schema_migrations records what ran, and a
  migration failure fails startup instead of half-migrating. database.New and
  apiserver.New take a context and logger for it.
- Render the DSN with pg.DSN, which percent-escapes the credentials the
  fmt.Sprintf builder pasted in raw. LoadConfig still reads the environment
  itself: pg.DSNFromEnv has no defaults for user and database name, where
  forgebot defaults both to "forgebot", and would newly honour DATABASE_URL and
  PG*. The deployed DBHOST/DBPORT/DBUSER/DBPASS/DBNAME/DBSSL contract and its
  defaults are unchanged, and pinned by a test.
- Plumb GOPRIVATE=git.unkin.net for the first cross-repo Go dependency:
  exported by the Makefile, set in both Dockerfiles and the woodpecker Go
  steps, documented in the README.
- gofmt the four files that were already unformatted on main, so the
  pre-commit step can pass.

benvin merged commit fd556d8d2a into main

2026-09-05 11:15:31 +10:00

benvin deleted branch benvin/adopt-golib-pg

2026-09-05 11:15:31 +10:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: unkin/forgebot#3