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 <julianknutsen@users.noreply.github>
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Julian Knutsen
GitHub
@@ -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())
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user