Claim the bot's name only when it is free !135

merged merged by cmc on 2026-08-31 16:33 UTC · krz/gitbay:bot-name-guard into main

3 files changed, +73 −2

Layout: unified · split

internal/deps/worker.go +5 −1

 	}
 	author, err := w.St.UserByUsername(store.BotUsername)
 	if err != nil {
-		return fmt.Errorf("loading %s: %w", store.BotUsername, err)
+		// Migration 0028 leaves the account uncreated when the name was
+		// already taken, which is the one case worth spelling out: the
+		// feature is stuck until an operator frees the name.
+		return fmt.Errorf("no %s account to author the issue (the name was taken when this instance upgraded): %w",
+			store.BotUsername, err)
 	}
 	number, err := w.St.CreateIssue(repo.ID, author.ID, IssueTitle, body, "md")
 	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	-- The account dependency issues are authored by. Keyless and mailless: it	23	-- The account dependency issues are authored by. Keyless and mailless: it
24	-- authors, it never authenticates.	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');