Commit 0413b92f9a
0413b92f9a8896bfedeb9e73df244553033d4143
parent: 7ede0c7904
Verified · cmc ci/build: success ci/test: success
cmc <hello@cleberg.net> · 2026-08-31T16:30:27Z
Claim the bot's name only when it is free
Migration 0028 inserted gitbay-bot unconditionally. The name was not
reserved before that migration, so an instance upgrading from v1.0.x may
already have a user holding it: the insert hits the UNIQUE constraint, the
migration aborts, and the daemon does not start. An org of the same name is
the quieter version — the insert succeeds and two owners share one name.
Editing 0028 rather than adding 0029 because 0028 is unreleased, and a
later migration cannot rescue an earlier one that aborts.
The worker now says why it cannot open an issue when the account is absent,
which lands in the repo's check state instead of anywhere else.
Closes #64
internal/deps/worker.go
+5 −1
| @@ -194,7 +194,11 @@ func (w *Worker) reconcile(repo store.Repo, behind []store.DepReport) error { |
| 194 | 194 | } |
| 195 | 195 | author, err := w.St.UserByUsername(store.BotUsername) |
| 196 | 196 | if err != nil { |
| 197 | | return fmt.Errorf("loading %s: %w", store.BotUsername, err) |
| 197 | // Migration 0028 leaves the account uncreated when the name was |
| 198 | // already taken, which is the one case worth spelling out: the |
| 199 | // feature is stuck until an operator frees the name. |
| 200 | return fmt.Errorf("no %s account to author the issue (the name was taken when this instance upgraded): %w", |
| 201 | store.BotUsername, err) |
| 198 | 202 | } |
| 199 | 203 | number, err := w.St.CreateIssue(repo.ID, author.ID, IssueTitle, body, "md") |
| 200 | 204 | if err != nil { |
internal/store/deps_test.go
added
+59
| @@ -0,0 +1,59 @@ |
| 1 | package store |
| 2 | |
| 3 | import "testing" |
| 4 | |
| 5 | // The bot's name was not reserved before migration 0028, so an instance |
| 6 | // upgrading from v1.0.x may already have an owner holding it. The migration |
| 7 | // has to survive that: a daemon that will not start is worse than a |
| 8 | // dependency check that cannot open an issue. |
| 9 | func TestBotNameCollisionDoesNotBlockMigration(t *testing.T) { |
| 10 | for _, tc := range []struct { |
| 11 | name string |
| 12 | occupy func(*Store) error |
| 13 | }{ |
| 14 | {"user", func(s *Store) error { |
| 15 | _, err := s.DB.Exec("INSERT INTO users (username, is_admin) VALUES (?, 0)", BotUsername) |
| 16 | return err |
| 17 | }}, |
| 18 | {"org", func(s *Store) error { |
| 19 | _, err := s.DB.Exec("INSERT INTO orgs (name) VALUES (?)", BotUsername) |
| 20 | return err |
| 21 | }}, |
| 22 | } { |
| 23 | t.Run(tc.name, func(t *testing.T) { |
| 24 | s := open(t) |
| 25 | if err := s.MigrateTo(27); err != nil { |
| 26 | t.Fatal(err) |
| 27 | } |
| 28 | if err := tc.occupy(s); err != nil { |
| 29 | t.Fatal(err) |
| 30 | } |
| 31 | if err := s.MigrateTo(28); err != nil { |
| 32 | t.Fatalf("migration 0028 failed with %s %q present: %v", tc.name, BotUsername, err) |
| 33 | } |
| 34 | var n int |
| 35 | if err := s.DB.QueryRow("SELECT count(*) FROM users WHERE username = ?", BotUsername).Scan(&n); err != nil { |
| 36 | t.Fatal(err) |
| 37 | } |
| 38 | want := 0 |
| 39 | if tc.name == "user" { |
| 40 | want = 1 // the pre-existing account, not a second one |
| 41 | } |
| 42 | if n != want { |
| 43 | t.Errorf("users named %q = %d, want %d", BotUsername, n, want) |
| 44 | } |
| 45 | }) |
| 46 | } |
| 47 | } |
| 48 | |
| 49 | // On a clean instance the account is created, which is what every other |
| 50 | // dependency test assumes. |
| 51 | func TestBotAccountCreatedWhenNameIsFree(t *testing.T) { |
| 52 | s := open(t) |
| 53 | if err := s.MigrateUp(); err != nil { |
| 54 | t.Fatal(err) |
| 55 | } |
| 56 | if _, err := s.UserByUsername(BotUsername); err != nil { |
| 57 | t.Fatalf("no %s account after a clean migration: %v", BotUsername, err) |
| 58 | } |
| 59 | } |
internal/store/migrations/0028_deps.up.sql
+9 −1
| @@ -22,4 +22,12 @@ CREATE TABLE dep_reports ( |
| 22 | 22 | |
| 23 | 23 | -- The account dependency issues are authored by. Keyless and mailless: it |
| 24 | 24 | -- authors, it never authenticates. |
| 25 | | INSERT INTO users (username, is_admin) VALUES ('gitbay-bot', 0); |
| 25 | -- |
| 26 | -- The name was not reserved before this migration, so an instance upgrading |
| 27 | -- from v1.0.x may already have a user or an org holding it. Claim it only if |
| 28 | -- it is free: a daemon that will not start is a far worse outcome than a |
| 29 | -- dependency check that reports why it cannot open an issue. |
| 30 | INSERT INTO users (username, is_admin) |
| 31 | SELECT 'gitbay-bot', 0 |
| 32 | WHERE NOT EXISTS (SELECT 1 FROM users WHERE username = 'gitbay-bot') |
| 33 | AND NOT EXISTS (SELECT 1 FROM orgs WHERE name = 'gitbay-bot'); |