From 995476a9c08b5dd00cdc80559e334c8f48c18b93 Mon Sep 17 00:00:00 2001 From: Julian Knutsen Date: Mon, 12 Jan 2026 09:45:21 +0000 Subject: [PATCH] fix(install): add gt:role label to role beads during creation (#383) Role beads created by gt install were missing the gt:role label required by GetRoleConfig(), causing witness startup to fail with: "bead hq-witness-role is not a role bead (missing gt:role label)" This regression was introduced in 96970071 which migrated from type-based to label-based bead classification. The install code used raw exec.Command instead of the beads API, so it wasn't updated to add labels. Changes: - Use bd.CreateWithID() API which auto-converts Type:"role" to gt:role label - Add RoleLabelCheck doctor migration to fix existing installations - Add comprehensive unit tests with mocked dependencies Co-authored-by: julianknutsen Co-authored-by: Claude Opus 4.5 --- internal/cmd/doctor.go | 1 + internal/cmd/install.go | 22 +-- internal/doctor/beads_check.go | 138 +++++++++++++ internal/doctor/beads_check_test.go | 292 ++++++++++++++++++++++++++++ 4 files changed, 442 insertions(+), 11 deletions(-) diff --git a/internal/cmd/doctor.go b/internal/cmd/doctor.go index 1ab77148..6052f30c 100644 --- a/internal/cmd/doctor.go +++ b/internal/cmd/doctor.go @@ -126,6 +126,7 @@ func runDoctor(cmd *cobra.Command, args []string) error { d.Register(doctor.NewBootHealthCheck()) d.Register(doctor.NewBeadsDatabaseCheck()) d.Register(doctor.NewCustomTypesCheck()) + d.Register(doctor.NewRoleLabelCheck()) d.Register(doctor.NewFormulaCheck()) d.Register(doctor.NewBdDaemonCheck()) d.Register(doctor.NewPrefixConflictCheck()) diff --git a/internal/cmd/install.go b/internal/cmd/install.go index 42c322dc..fd1d6b99 100644 --- a/internal/cmd/install.go +++ b/internal/cmd/install.go @@ -492,18 +492,18 @@ func initTownAgentBeads(townPath string) error { continue // Already exists } - // Create role bead using bd create --type=role - cmd := exec.Command("bd", "create", - "--type=role", - "--id="+role.id, - "--title="+role.title, - "--description="+role.desc, - ) - cmd.Dir = townPath - if output, err := cmd.CombinedOutput(); err != nil { + // Create role bead using the beads API + // CreateWithID with Type: "role" automatically adds gt:role label + _, err := bd.CreateWithID(role.id, beads.CreateOptions{ + Title: role.title, + Type: "role", + Description: role.desc, + Priority: -1, // No priority + }) + if err != nil { // Log but continue - role beads are optional - fmt.Printf(" %s Could not create role bead %s: %s\n", - style.Dim.Render("⚠"), role.id, strings.TrimSpace(string(output))) + fmt.Printf(" %s Could not create role bead %s: %v\n", + style.Dim.Render("⚠"), role.id, err) continue } fmt.Printf(" ✓ Created role bead: %s\n", role.id) diff --git a/internal/doctor/beads_check.go b/internal/doctor/beads_check.go index dde13d31..f1e922c0 100644 --- a/internal/doctor/beads_check.go +++ b/internal/doctor/beads_check.go @@ -436,3 +436,141 @@ func saveRigsConfig(path string, cfg *rigsConfigFile) error { return os.WriteFile(path, data, 0644) } + +// beadShower is an interface for fetching bead information. +// Allows mocking in tests. +type beadShower interface { + Show(id string) (*beads.Issue, error) +} + +// labelAdder is an interface for adding labels to beads. +// Allows mocking in tests. +type labelAdder interface { + AddLabel(townRoot, id, label string) error +} + +// realLabelAdder implements labelAdder using bd command. +type realLabelAdder struct{} + +func (r *realLabelAdder) AddLabel(townRoot, id, label string) error { + cmd := exec.Command("bd", "label", "add", id, label) + cmd.Dir = townRoot + if output, err := cmd.CombinedOutput(); err != nil { + return fmt.Errorf("adding %s label to %s: %s", label, id, strings.TrimSpace(string(output))) + } + return nil +} + +// RoleLabelCheck verifies that role beads have the gt:role label. +// This label is required for GetRoleConfig to recognize role beads. +// Role beads created before the label migration may be missing this label. +type RoleLabelCheck struct { + FixableCheck + missingLabel []string // Role bead IDs missing gt:role label + townRoot string // Cached for Fix + + // Injected dependencies for testing + beadShower beadShower + labelAdder labelAdder +} + +// NewRoleLabelCheck creates a new role label check. +func NewRoleLabelCheck() *RoleLabelCheck { + return &RoleLabelCheck{ + FixableCheck: FixableCheck{ + BaseCheck: BaseCheck{ + CheckName: "role-bead-labels", + CheckDescription: "Check that role beads have gt:role label", + CheckCategory: CategoryConfig, + }, + }, + labelAdder: &realLabelAdder{}, + } +} + +// roleBeadIDs returns the list of role bead IDs to check. +func roleBeadIDs() []string { + return []string{ + beads.MayorRoleBeadIDTown(), + beads.DeaconRoleBeadIDTown(), + beads.DogRoleBeadIDTown(), + beads.WitnessRoleBeadIDTown(), + beads.RefineryRoleBeadIDTown(), + beads.PolecatRoleBeadIDTown(), + beads.CrewRoleBeadIDTown(), + } +} + +// Run checks if role beads have the gt:role label. +func (c *RoleLabelCheck) Run(ctx *CheckContext) *CheckResult { + // Check if bd command is available (skip if testing with mock) + if c.beadShower == nil { + if _, err := exec.LookPath("bd"); err != nil { + return &CheckResult{ + Name: c.Name(), + Status: StatusOK, + Message: "beads not installed (skipped)", + } + } + } + + // Check if .beads directory exists at town level + townBeadsDir := filepath.Join(ctx.TownRoot, ".beads") + if _, err := os.Stat(townBeadsDir); os.IsNotExist(err) { + return &CheckResult{ + Name: c.Name(), + Status: StatusOK, + Message: "No beads database (skipped)", + } + } + + // Use injected beadShower or create real one + shower := c.beadShower + if shower == nil { + shower = beads.New(ctx.TownRoot) + } + + var missingLabel []string + for _, roleID := range roleBeadIDs() { + issue, err := shower.Show(roleID) + if err != nil { + // Bead doesn't exist - that's OK, install will create it + continue + } + + // Check if it has the gt:role label + if !beads.HasLabel(issue, "gt:role") { + missingLabel = append(missingLabel, roleID) + } + } + + // Cache for Fix + c.missingLabel = missingLabel + c.townRoot = ctx.TownRoot + + if len(missingLabel) == 0 { + return &CheckResult{ + Name: c.Name(), + Status: StatusOK, + Message: "All role beads have gt:role label", + } + } + + return &CheckResult{ + Name: c.Name(), + Status: StatusWarning, + Message: fmt.Sprintf("%d role bead(s) missing gt:role label", len(missingLabel)), + Details: missingLabel, + FixHint: "Run 'gt doctor --fix' to add missing labels", + } +} + +// Fix adds the gt:role label to role beads that are missing it. +func (c *RoleLabelCheck) Fix(ctx *CheckContext) error { + for _, roleID := range c.missingLabel { + if err := c.labelAdder.AddLabel(c.townRoot, roleID, "gt:role"); err != nil { + return err + } + } + return nil +} diff --git a/internal/doctor/beads_check_test.go b/internal/doctor/beads_check_test.go index 667c879c..05579c79 100644 --- a/internal/doctor/beads_check_test.go +++ b/internal/doctor/beads_check_test.go @@ -4,6 +4,8 @@ import ( "os" "path/filepath" "testing" + + "github.com/steveyegge/gastown/internal/beads" ) func TestNewBeadsDatabaseCheck(t *testing.T) { @@ -315,3 +317,293 @@ func TestPrefixMismatchCheck_Fix(t *testing.T) { t.Errorf("expected prefix 'gt' after fix, got %q", cfg.Rigs["gastown"].BeadsConfig.Prefix) } } + +func TestNewRoleLabelCheck(t *testing.T) { + check := NewRoleLabelCheck() + + if check.Name() != "role-bead-labels" { + t.Errorf("expected name 'role-bead-labels', got %q", check.Name()) + } + + if !check.CanFix() { + t.Error("expected CanFix to return true") + } +} + +func TestRoleLabelCheck_NoBeadsDir(t *testing.T) { + tmpDir := t.TempDir() + + check := NewRoleLabelCheck() + ctx := &CheckContext{TownRoot: tmpDir} + + result := check.Run(ctx) + + if result.Status != StatusOK { + t.Errorf("expected StatusOK when no .beads dir, got %v", result.Status) + } + if result.Message != "No beads database (skipped)" { + t.Errorf("unexpected message: %s", result.Message) + } +} + +// mockBeadShower implements beadShower for testing +type mockBeadShower struct { + beads map[string]*beads.Issue +} + +func (m *mockBeadShower) Show(id string) (*beads.Issue, error) { + if issue, ok := m.beads[id]; ok { + return issue, nil + } + return nil, beads.ErrNotFound +} + +// mockLabelAdder implements labelAdder for testing +type mockLabelAdder struct { + calls []labelAddCall +} + +type labelAddCall struct { + townRoot string + id string + label string +} + +func (m *mockLabelAdder) AddLabel(townRoot, id, label string) error { + m.calls = append(m.calls, labelAddCall{townRoot, id, label}) + return nil +} + +func TestRoleLabelCheck_AllBeadsHaveLabel(t *testing.T) { + tmpDir := t.TempDir() + beadsDir := filepath.Join(tmpDir, ".beads") + if err := os.MkdirAll(beadsDir, 0755); err != nil { + t.Fatal(err) + } + + // Create mock with all role beads having gt:role label + mock := &mockBeadShower{ + beads: map[string]*beads.Issue{ + "hq-mayor-role": {ID: "hq-mayor-role", Labels: []string{"gt:role"}}, + "hq-deacon-role": {ID: "hq-deacon-role", Labels: []string{"gt:role"}}, + "hq-dog-role": {ID: "hq-dog-role", Labels: []string{"gt:role"}}, + "hq-witness-role": {ID: "hq-witness-role", Labels: []string{"gt:role"}}, + "hq-refinery-role": {ID: "hq-refinery-role", Labels: []string{"gt:role"}}, + "hq-polecat-role": {ID: "hq-polecat-role", Labels: []string{"gt:role"}}, + "hq-crew-role": {ID: "hq-crew-role", Labels: []string{"gt:role"}}, + }, + } + + check := NewRoleLabelCheck() + check.beadShower = mock + ctx := &CheckContext{TownRoot: tmpDir} + + result := check.Run(ctx) + + if result.Status != StatusOK { + t.Errorf("expected StatusOK when all beads have label, got %v: %s", result.Status, result.Message) + } + if result.Message != "All role beads have gt:role label" { + t.Errorf("unexpected message: %s", result.Message) + } +} + +func TestRoleLabelCheck_MissingLabel(t *testing.T) { + tmpDir := t.TempDir() + beadsDir := filepath.Join(tmpDir, ".beads") + if err := os.MkdirAll(beadsDir, 0755); err != nil { + t.Fatal(err) + } + + // Create mock with witness-role missing the gt:role label (the regression case) + mock := &mockBeadShower{ + beads: map[string]*beads.Issue{ + "hq-mayor-role": {ID: "hq-mayor-role", Labels: []string{"gt:role"}}, + "hq-deacon-role": {ID: "hq-deacon-role", Labels: []string{"gt:role"}}, + "hq-dog-role": {ID: "hq-dog-role", Labels: []string{"gt:role"}}, + "hq-witness-role": {ID: "hq-witness-role", Labels: []string{}}, // Missing gt:role! + "hq-refinery-role": {ID: "hq-refinery-role", Labels: []string{"gt:role"}}, + "hq-polecat-role": {ID: "hq-polecat-role", Labels: []string{"gt:role"}}, + "hq-crew-role": {ID: "hq-crew-role", Labels: []string{"gt:role"}}, + }, + } + + check := NewRoleLabelCheck() + check.beadShower = mock + ctx := &CheckContext{TownRoot: tmpDir} + + result := check.Run(ctx) + + if result.Status != StatusWarning { + t.Errorf("expected StatusWarning when label missing, got %v", result.Status) + } + if result.Message != "1 role bead(s) missing gt:role label" { + t.Errorf("unexpected message: %s", result.Message) + } + if len(result.Details) != 1 || result.Details[0] != "hq-witness-role" { + t.Errorf("expected details to contain hq-witness-role, got %v", result.Details) + } +} + +func TestRoleLabelCheck_MultipleMissingLabels(t *testing.T) { + tmpDir := t.TempDir() + beadsDir := filepath.Join(tmpDir, ".beads") + if err := os.MkdirAll(beadsDir, 0755); err != nil { + t.Fatal(err) + } + + // Create mock with multiple beads missing the gt:role label + mock := &mockBeadShower{ + beads: map[string]*beads.Issue{ + "hq-mayor-role": {ID: "hq-mayor-role", Labels: []string{}}, // Missing + "hq-deacon-role": {ID: "hq-deacon-role", Labels: []string{}}, // Missing + "hq-dog-role": {ID: "hq-dog-role", Labels: []string{"gt:role"}}, + "hq-witness-role": {ID: "hq-witness-role", Labels: []string{}}, // Missing + "hq-refinery-role": {ID: "hq-refinery-role", Labels: []string{}}, // Missing + "hq-polecat-role": {ID: "hq-polecat-role", Labels: []string{"gt:role"}}, + "hq-crew-role": {ID: "hq-crew-role", Labels: []string{"gt:role"}}, + }, + } + + check := NewRoleLabelCheck() + check.beadShower = mock + ctx := &CheckContext{TownRoot: tmpDir} + + result := check.Run(ctx) + + if result.Status != StatusWarning { + t.Errorf("expected StatusWarning, got %v", result.Status) + } + if result.Message != "4 role bead(s) missing gt:role label" { + t.Errorf("unexpected message: %s", result.Message) + } + if len(result.Details) != 4 { + t.Errorf("expected 4 details, got %d: %v", len(result.Details), result.Details) + } +} + +func TestRoleLabelCheck_BeadNotFound(t *testing.T) { + tmpDir := t.TempDir() + beadsDir := filepath.Join(tmpDir, ".beads") + if err := os.MkdirAll(beadsDir, 0755); err != nil { + t.Fatal(err) + } + + // Create mock with only some beads existing (others return ErrNotFound) + mock := &mockBeadShower{ + beads: map[string]*beads.Issue{ + "hq-mayor-role": {ID: "hq-mayor-role", Labels: []string{"gt:role"}}, + "hq-deacon-role": {ID: "hq-deacon-role", Labels: []string{"gt:role"}}, + // Other beads don't exist - should be skipped, not reported as errors + }, + } + + check := NewRoleLabelCheck() + check.beadShower = mock + ctx := &CheckContext{TownRoot: tmpDir} + + result := check.Run(ctx) + + // Should be OK - missing beads are not an error (install will create them) + if result.Status != StatusOK { + t.Errorf("expected StatusOK when beads don't exist, got %v: %s", result.Status, result.Message) + } +} + +func TestRoleLabelCheck_Fix(t *testing.T) { + tmpDir := t.TempDir() + beadsDir := filepath.Join(tmpDir, ".beads") + if err := os.MkdirAll(beadsDir, 0755); err != nil { + t.Fatal(err) + } + + // Create mock with witness-role missing the label + mockShower := &mockBeadShower{ + beads: map[string]*beads.Issue{ + "hq-mayor-role": {ID: "hq-mayor-role", Labels: []string{"gt:role"}}, + "hq-witness-role": {ID: "hq-witness-role", Labels: []string{}}, // Missing gt:role + }, + } + mockAdder := &mockLabelAdder{} + + check := NewRoleLabelCheck() + check.beadShower = mockShower + check.labelAdder = mockAdder + ctx := &CheckContext{TownRoot: tmpDir} + + // First run to detect the issue + result := check.Run(ctx) + if result.Status != StatusWarning { + t.Fatalf("expected StatusWarning, got %v", result.Status) + } + + // Now fix + if err := check.Fix(ctx); err != nil { + t.Fatalf("Fix() failed: %v", err) + } + + // Verify the correct bd label add command was called + if len(mockAdder.calls) != 1 { + t.Fatalf("expected 1 AddLabel call, got %d", len(mockAdder.calls)) + } + call := mockAdder.calls[0] + if call.townRoot != tmpDir { + t.Errorf("expected townRoot %q, got %q", tmpDir, call.townRoot) + } + if call.id != "hq-witness-role" { + t.Errorf("expected id 'hq-witness-role', got %q", call.id) + } + if call.label != "gt:role" { + t.Errorf("expected label 'gt:role', got %q", call.label) + } +} + +func TestRoleLabelCheck_FixMultiple(t *testing.T) { + tmpDir := t.TempDir() + beadsDir := filepath.Join(tmpDir, ".beads") + if err := os.MkdirAll(beadsDir, 0755); err != nil { + t.Fatal(err) + } + + // Create mock with multiple beads missing the label + mockShower := &mockBeadShower{ + beads: map[string]*beads.Issue{ + "hq-mayor-role": {ID: "hq-mayor-role", Labels: []string{}}, // Missing + "hq-deacon-role": {ID: "hq-deacon-role", Labels: []string{"gt:role"}}, + "hq-witness-role": {ID: "hq-witness-role", Labels: []string{}}, // Missing + "hq-refinery-role": {ID: "hq-refinery-role", Labels: []string{}}, // Missing + }, + } + mockAdder := &mockLabelAdder{} + + check := NewRoleLabelCheck() + check.beadShower = mockShower + check.labelAdder = mockAdder + ctx := &CheckContext{TownRoot: tmpDir} + + // First run to detect the issues + result := check.Run(ctx) + if result.Status != StatusWarning { + t.Fatalf("expected StatusWarning, got %v", result.Status) + } + if len(result.Details) != 3 { + t.Fatalf("expected 3 missing, got %d", len(result.Details)) + } + + // Now fix + if err := check.Fix(ctx); err != nil { + t.Fatalf("Fix() failed: %v", err) + } + + // Verify all 3 beads got the label added + if len(mockAdder.calls) != 3 { + t.Fatalf("expected 3 AddLabel calls, got %d", len(mockAdder.calls)) + } + + // Verify each call has the correct label + for _, call := range mockAdder.calls { + if call.label != "gt:role" { + t.Errorf("expected label 'gt:role', got %q", call.label) + } + } +}