diff --git a/internal/cmd/doctor.go b/internal/cmd/doctor.go index b95f9aee..ee6ff0d7 100644 --- a/internal/cmd/doctor.go +++ b/internal/cmd/doctor.go @@ -96,6 +96,9 @@ func runDoctor(cmd *cobra.Command, args []string) error { // Lifecycle hygiene checks d.Register(doctor.NewLifecycleHygieneCheck()) + // Hook attachment checks + d.Register(doctor.NewHookAttachmentValidCheck()) + // Run checks var report *doctor.Report if doctorFix { diff --git a/internal/doctor/hook_check.go b/internal/doctor/hook_check.go new file mode 100644 index 00000000..4b6b2385 --- /dev/null +++ b/internal/doctor/hook_check.go @@ -0,0 +1,186 @@ +package doctor + +import ( + "fmt" + "os/exec" + "path/filepath" + "strings" + + "github.com/steveyegge/gastown/internal/beads" +) + +// HookAttachmentValidCheck verifies that attached molecules exist and are not closed. +// This detects when a hook's attached_molecule field points to a non-existent or +// closed issue, which can leave agents with stale work assignments. +type HookAttachmentValidCheck struct { + FixableCheck + invalidAttachments []invalidAttachment +} + +type invalidAttachment struct { + pinnedBeadID string + pinnedBeadDir string // Directory where the pinned bead was found + moleculeID string + reason string // "not_found" or "closed" +} + +// NewHookAttachmentValidCheck creates a new hook attachment validation check. +func NewHookAttachmentValidCheck() *HookAttachmentValidCheck { + return &HookAttachmentValidCheck{ + FixableCheck: FixableCheck{ + BaseCheck: BaseCheck{ + CheckName: "hook-attachment-valid", + CheckDescription: "Verify attached molecules exist and are not closed", + }, + }, + } +} + +// Run checks all pinned beads for invalid molecule attachments. +func (c *HookAttachmentValidCheck) Run(ctx *CheckContext) *CheckResult { + c.invalidAttachments = nil + + var details []string + + // Check town-level beads + townBeadsDir := filepath.Join(ctx.TownRoot, ".beads") + townInvalid := c.checkBeadsDir(townBeadsDir, "town") + for _, inv := range townInvalid { + details = append(details, c.formatInvalid(inv)) + } + c.invalidAttachments = append(c.invalidAttachments, townInvalid...) + + // Check rig-level beads + rigDirs := c.findRigBeadsDirs(ctx.TownRoot) + for _, rigDir := range rigDirs { + rigName := filepath.Base(filepath.Dir(rigDir)) + rigInvalid := c.checkBeadsDir(rigDir, rigName) + for _, inv := range rigInvalid { + details = append(details, c.formatInvalid(inv)) + } + c.invalidAttachments = append(c.invalidAttachments, rigInvalid...) + } + + if len(c.invalidAttachments) == 0 { + return &CheckResult{ + Name: c.Name(), + Status: StatusOK, + Message: "All hook attachments are valid", + } + } + + return &CheckResult{ + Name: c.Name(), + Status: StatusError, + Message: fmt.Sprintf("Found %d invalid hook attachment(s)", len(c.invalidAttachments)), + Details: details, + FixHint: "Run 'gt doctor --fix' to detach invalid molecules, or 'gt mol detach ' manually", + } +} + +// checkBeadsDir checks all pinned beads in a directory for invalid attachments. +func (c *HookAttachmentValidCheck) checkBeadsDir(beadsDir, location string) []invalidAttachment { + var invalid []invalidAttachment + + b := beads.New(filepath.Dir(beadsDir)) + + // List all pinned beads + pinnedBeads, err := b.List(beads.ListOptions{ + Status: beads.StatusPinned, + Priority: -1, + }) + if err != nil { + // Can't list pinned beads - silently skip this directory + return nil + } + + for _, pinnedBead := range pinnedBeads { + // Parse attachment fields from the pinned bead + attachment := beads.ParseAttachmentFields(pinnedBead) + if attachment == nil || attachment.AttachedMolecule == "" { + continue // No attachment, skip + } + + // Verify the attached molecule exists and is not closed + molecule, err := b.Show(attachment.AttachedMolecule) + if err != nil { + // Molecule not found + invalid = append(invalid, invalidAttachment{ + pinnedBeadID: pinnedBead.ID, + pinnedBeadDir: beadsDir, + moleculeID: attachment.AttachedMolecule, + reason: "not_found", + }) + continue + } + + if molecule.Status == "closed" { + invalid = append(invalid, invalidAttachment{ + pinnedBeadID: pinnedBead.ID, + pinnedBeadDir: beadsDir, + moleculeID: attachment.AttachedMolecule, + reason: "closed", + }) + } + } + + return invalid +} + +// findRigBeadsDirs finds all rig-level .beads directories. +func (c *HookAttachmentValidCheck) findRigBeadsDirs(townRoot string) []string { + var dirs []string + + // Look for .beads directories in rig subdirectories + // Pattern: //.beads (but NOT /.beads which is town-level) + cmd := exec.Command("find", townRoot, "-maxdepth", "2", "-type", "d", "-name", ".beads") + output, err := cmd.Output() + if err != nil { + return nil + } + + for _, line := range strings.Split(strings.TrimSpace(string(output)), "\n") { + if line == "" { + continue + } + // Skip town-level .beads + if line == filepath.Join(townRoot, ".beads") { + continue + } + // Skip mayor directory + if strings.Contains(line, "/mayor/") { + continue + } + dirs = append(dirs, line) + } + + return dirs +} + +// formatInvalid formats an invalid attachment for display. +func (c *HookAttachmentValidCheck) formatInvalid(inv invalidAttachment) string { + reasonText := "not found" + if inv.reason == "closed" { + reasonText = "is closed" + } + return fmt.Sprintf("%s: attached molecule %s %s", inv.pinnedBeadID, inv.moleculeID, reasonText) +} + +// Fix detaches all invalid molecule attachments. +func (c *HookAttachmentValidCheck) Fix(ctx *CheckContext) error { + var errors []string + + for _, inv := range c.invalidAttachments { + b := beads.New(filepath.Dir(inv.pinnedBeadDir)) + + _, err := b.DetachMolecule(inv.pinnedBeadID) + if err != nil { + errors = append(errors, fmt.Sprintf("failed to detach from %s: %v", inv.pinnedBeadID, err)) + } + } + + if len(errors) > 0 { + return fmt.Errorf("%s", strings.Join(errors, "; ")) + } + return nil +} diff --git a/internal/doctor/hook_check_test.go b/internal/doctor/hook_check_test.go new file mode 100644 index 00000000..e2a8e770 --- /dev/null +++ b/internal/doctor/hook_check_test.go @@ -0,0 +1,123 @@ +package doctor + +import ( + "os" + "path/filepath" + "testing" +) + +func TestNewHookAttachmentValidCheck(t *testing.T) { + check := NewHookAttachmentValidCheck() + + if check.Name() != "hook-attachment-valid" { + t.Errorf("expected name 'hook-attachment-valid', got %q", check.Name()) + } + + if check.Description() != "Verify attached molecules exist and are not closed" { + t.Errorf("unexpected description: %q", check.Description()) + } + + if !check.CanFix() { + t.Error("expected CanFix to return true") + } +} + +func TestHookAttachmentValidCheck_NoBeadsDir(t *testing.T) { + tmpDir := t.TempDir() + + check := NewHookAttachmentValidCheck() + ctx := &CheckContext{TownRoot: tmpDir} + + result := check.Run(ctx) + + // No beads dir means nothing to check, should be OK + if result.Status != StatusOK { + t.Errorf("expected StatusOK when no beads dir, got %v", result.Status) + } +} + +func TestHookAttachmentValidCheck_EmptyBeadsDir(t *testing.T) { + tmpDir := t.TempDir() + beadsDir := filepath.Join(tmpDir, ".beads") + if err := os.MkdirAll(beadsDir, 0755); err != nil { + t.Fatal(err) + } + + check := NewHookAttachmentValidCheck() + ctx := &CheckContext{TownRoot: tmpDir} + + result := check.Run(ctx) + + // Empty beads dir means no pinned beads, should be OK + // Note: This may error if bd CLI is not available, but should still handle gracefully + if result.Status != StatusOK && result.Status != StatusError { + t.Errorf("expected StatusOK or graceful error, got %v", result.Status) + } +} + +func TestHookAttachmentValidCheck_FormatInvalid(t *testing.T) { + check := NewHookAttachmentValidCheck() + + tests := []struct { + inv invalidAttachment + expected string + }{ + { + inv: invalidAttachment{ + pinnedBeadID: "hq-123", + moleculeID: "gt-456", + reason: "not_found", + }, + expected: "hq-123: attached molecule gt-456 not found", + }, + { + inv: invalidAttachment{ + pinnedBeadID: "hq-123", + moleculeID: "gt-789", + reason: "closed", + }, + expected: "hq-123: attached molecule gt-789 is closed", + }, + } + + for _, tt := range tests { + result := check.formatInvalid(tt.inv) + if result != tt.expected { + t.Errorf("formatInvalid() = %q, want %q", result, tt.expected) + } + } +} + +func TestHookAttachmentValidCheck_FindRigBeadsDirs(t *testing.T) { + tmpDir := t.TempDir() + + // Create town-level .beads (should be excluded) + townBeads := filepath.Join(tmpDir, ".beads") + if err := os.MkdirAll(townBeads, 0755); err != nil { + t.Fatal(err) + } + + // Create rig-level .beads + rigBeads := filepath.Join(tmpDir, "myrig", ".beads") + if err := os.MkdirAll(rigBeads, 0755); err != nil { + t.Fatal(err) + } + + check := NewHookAttachmentValidCheck() + dirs := check.findRigBeadsDirs(tmpDir) + + // Should find the rig-level beads but not town-level + found := false + for _, dir := range dirs { + if dir == townBeads { + t.Error("findRigBeadsDirs should not include town-level .beads") + } + if dir == rigBeads { + found = true + } + } + + if !found && len(dirs) > 0 { + t.Logf("Found dirs: %v", dirs) + } +}