fix: prevent inherited BEADS_DIR from causing prefix mismatch (#321)
- Fix beads.run() to always explicitly set BEADS_DIR based on the working directory or explicit override - This prevents inherited environment variables (e.g., from mayor session with BEADS_DIR=/home/erik/gt/.beads) from causing prefix mismatch errors when creating agent beads for rigs - Update polecat manager to use NewWithBeadsDir for explicitness - Add comprehensive test coverage for BEADS_DIR routing and validation - Add SessionLister interface for deterministic orphan session testing Root cause: When BEADS_DIR was set in the parent environment, all bd commands used the town database (hq- prefix) instead of the rig database (gt- prefix), causing "prefix mismatch: database uses 'hq' but you specified 'gt'" errors during polecat spawn. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
gastown/crew/joe
committed by
Steve Yegge
Steve Yegge
parent
ea84079f8b
commit
598a39e708
@@ -1,9 +1,22 @@
|
||||
package doctor
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"reflect"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// mockSessionLister allows deterministic testing of orphan session detection.
|
||||
type mockSessionLister struct {
|
||||
sessions []string
|
||||
err error
|
||||
}
|
||||
|
||||
func (m *mockSessionLister) ListSessions() ([]string, error) {
|
||||
return m.sessions, m.err
|
||||
}
|
||||
|
||||
func TestNewOrphanSessionCheck(t *testing.T) {
|
||||
check := NewOrphanSessionCheck()
|
||||
|
||||
@@ -132,3 +145,264 @@ func TestOrphanSessionCheck_IsValidSession(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestOrphanSessionCheck_IsValidSession_EdgeCases tests edge cases that have caused
|
||||
// false positives in production - sessions incorrectly detected as orphans.
|
||||
func TestOrphanSessionCheck_IsValidSession_EdgeCases(t *testing.T) {
|
||||
check := NewOrphanSessionCheck()
|
||||
validRigs := []string{"gastown", "niflheim", "grctool", "7thsense", "pulseflow"}
|
||||
mayorSession := "hq-mayor"
|
||||
deaconSession := "hq-deacon"
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
session string
|
||||
want bool
|
||||
reason string
|
||||
}{
|
||||
// Crew sessions with various name formats
|
||||
{
|
||||
name: "crew_simple_name",
|
||||
session: "gt-gastown-crew-max",
|
||||
want: true,
|
||||
reason: "simple crew name should be valid",
|
||||
},
|
||||
{
|
||||
name: "crew_with_numbers",
|
||||
session: "gt-niflheim-crew-codex1",
|
||||
want: true,
|
||||
reason: "crew name with numbers should be valid",
|
||||
},
|
||||
{
|
||||
name: "crew_alphanumeric",
|
||||
session: "gt-grctool-crew-grc1",
|
||||
want: true,
|
||||
reason: "alphanumeric crew name should be valid",
|
||||
},
|
||||
{
|
||||
name: "crew_short_name",
|
||||
session: "gt-7thsense-crew-ss1",
|
||||
want: true,
|
||||
reason: "short crew name should be valid",
|
||||
},
|
||||
{
|
||||
name: "crew_pf1",
|
||||
session: "gt-pulseflow-crew-pf1",
|
||||
want: true,
|
||||
reason: "pf1 crew name should be valid",
|
||||
},
|
||||
|
||||
// Polecat sessions (any name after rig should be accepted)
|
||||
{
|
||||
name: "polecat_hash_style",
|
||||
session: "gt-gastown-abc123def",
|
||||
want: true,
|
||||
reason: "polecat with hash-style name should be valid",
|
||||
},
|
||||
{
|
||||
name: "polecat_descriptive",
|
||||
session: "gt-niflheim-fix-auth-bug",
|
||||
want: true,
|
||||
reason: "polecat with descriptive name should be valid",
|
||||
},
|
||||
|
||||
// Sessions that should be detected as orphans
|
||||
{
|
||||
name: "unknown_rig_witness",
|
||||
session: "gt-unknownrig-witness",
|
||||
want: false,
|
||||
reason: "unknown rig should be orphan",
|
||||
},
|
||||
{
|
||||
name: "malformed_too_short",
|
||||
session: "gt-only",
|
||||
want: false,
|
||||
reason: "malformed session (too few parts) should be orphan",
|
||||
},
|
||||
|
||||
// Edge case: rig name with hyphen would be tricky
|
||||
// Current implementation uses SplitN with limit 3
|
||||
// gt-my-rig-witness would parse as rig="my" role="rig-witness"
|
||||
// This is a known limitation documented here
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got := check.isValidSession(tt.session, validRigs, mayorSession, deaconSession)
|
||||
if got != tt.want {
|
||||
t.Errorf("isValidSession(%q) = %v, want %v: %s", tt.session, got, tt.want, tt.reason)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestOrphanSessionCheck_GetValidRigs verifies rig detection from filesystem.
|
||||
func TestOrphanSessionCheck_GetValidRigs(t *testing.T) {
|
||||
check := NewOrphanSessionCheck()
|
||||
townRoot := t.TempDir()
|
||||
|
||||
// Setup: create mayor directory (required for getValidRigs to proceed)
|
||||
if err := os.MkdirAll(filepath.Join(townRoot, "mayor"), 0755); err != nil {
|
||||
t.Fatalf("failed to create mayor dir: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(townRoot, "mayor", "rigs.json"), []byte("{}"), 0644); err != nil {
|
||||
t.Fatalf("failed to create rigs.json: %v", err)
|
||||
}
|
||||
|
||||
// Create some rigs with polecats/crew directories
|
||||
createRigDir := func(name string, hasCrew, hasPolecats bool) {
|
||||
rigPath := filepath.Join(townRoot, name)
|
||||
os.MkdirAll(rigPath, 0755)
|
||||
if hasCrew {
|
||||
os.MkdirAll(filepath.Join(rigPath, "crew"), 0755)
|
||||
}
|
||||
if hasPolecats {
|
||||
os.MkdirAll(filepath.Join(rigPath, "polecats"), 0755)
|
||||
}
|
||||
}
|
||||
|
||||
createRigDir("gastown", true, true)
|
||||
createRigDir("niflheim", true, false)
|
||||
createRigDir("grctool", false, true)
|
||||
createRigDir("not-a-rig", false, false) // No crew or polecats
|
||||
|
||||
rigs := check.getValidRigs(townRoot)
|
||||
|
||||
// Should find gastown, niflheim, grctool but not "not-a-rig"
|
||||
expected := map[string]bool{
|
||||
"gastown": true,
|
||||
"niflheim": true,
|
||||
"grctool": true,
|
||||
}
|
||||
|
||||
for _, rig := range rigs {
|
||||
if !expected[rig] {
|
||||
t.Errorf("unexpected rig %q in result", rig)
|
||||
}
|
||||
delete(expected, rig)
|
||||
}
|
||||
|
||||
for rig := range expected {
|
||||
t.Errorf("expected rig %q not found in result", rig)
|
||||
}
|
||||
}
|
||||
|
||||
// TestOrphanSessionCheck_FixProtectsCrewSessions verifies that Fix() never kills crew sessions.
|
||||
func TestOrphanSessionCheck_FixProtectsCrewSessions(t *testing.T) {
|
||||
check := NewOrphanSessionCheck()
|
||||
|
||||
// Simulate cached orphan sessions including a crew session
|
||||
check.orphanSessions = []string{
|
||||
"gt-gastown-crew-max", // Crew - should be protected
|
||||
"gt-unknown-witness", // Not crew - would be killed
|
||||
"gt-niflheim-crew-codex1", // Crew - should be protected
|
||||
}
|
||||
|
||||
// Verify isCrewSession correctly identifies crew sessions
|
||||
for _, sess := range check.orphanSessions {
|
||||
if sess == "gt-gastown-crew-max" || sess == "gt-niflheim-crew-codex1" {
|
||||
if !isCrewSession(sess) {
|
||||
t.Errorf("isCrewSession(%q) should return true for crew session", sess)
|
||||
}
|
||||
} else {
|
||||
if isCrewSession(sess) {
|
||||
t.Errorf("isCrewSession(%q) should return false for non-crew session", sess)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestIsCrewSession_ComprehensivePatterns tests the crew session detection pattern thoroughly.
|
||||
func TestIsCrewSession_ComprehensivePatterns(t *testing.T) {
|
||||
tests := []struct {
|
||||
session string
|
||||
want bool
|
||||
reason string
|
||||
}{
|
||||
// Valid crew patterns
|
||||
{"gt-gastown-crew-joe", true, "standard crew session"},
|
||||
{"gt-beads-crew-max", true, "different rig crew session"},
|
||||
{"gt-niflheim-crew-codex1", true, "crew with numbers in name"},
|
||||
{"gt-grctool-crew-grc1", true, "crew with alphanumeric name"},
|
||||
{"gt-7thsense-crew-ss1", true, "rig starting with number"},
|
||||
{"gt-a-crew-b", true, "minimal valid crew session"},
|
||||
|
||||
// Invalid crew patterns
|
||||
{"gt-gastown-witness", false, "witness is not crew"},
|
||||
{"gt-gastown-refinery", false, "refinery is not crew"},
|
||||
{"gt-gastown-polecat-abc", false, "polecat is not crew"},
|
||||
{"hq-deacon", false, "deacon is not crew"},
|
||||
{"hq-mayor", false, "mayor is not crew"},
|
||||
{"gt-gastown-crew", false, "missing crew name"},
|
||||
{"gt-crew-max", false, "missing rig name"},
|
||||
{"crew-gastown-max", false, "wrong prefix"},
|
||||
{"other-session", false, "not a gt session"},
|
||||
{"", false, "empty string"},
|
||||
{"gt", false, "just prefix"},
|
||||
{"gt-", false, "prefix with dash"},
|
||||
{"gt-gastown", false, "rig only"},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.session, func(t *testing.T) {
|
||||
got := isCrewSession(tt.session)
|
||||
if got != tt.want {
|
||||
t.Errorf("isCrewSession(%q) = %v, want %v: %s", tt.session, got, tt.want, tt.reason)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestOrphanSessionCheck_Run_Deterministic tests the full Run path with a mock session
|
||||
// lister, ensuring deterministic behavior without depending on real tmux state.
|
||||
func TestOrphanSessionCheck_Run_Deterministic(t *testing.T) {
|
||||
townRoot := t.TempDir()
|
||||
mayorDir := filepath.Join(townRoot, "mayor")
|
||||
if err := os.MkdirAll(mayorDir, 0o755); err != nil {
|
||||
t.Fatalf("create mayor dir: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(mayorDir, "rigs.json"), []byte("{}"), 0o644); err != nil {
|
||||
t.Fatalf("create rigs.json: %v", err)
|
||||
}
|
||||
|
||||
// Create rig directories to make them "valid"
|
||||
if err := os.MkdirAll(filepath.Join(townRoot, "gastown", "polecats"), 0o755); err != nil {
|
||||
t.Fatalf("create gastown rig: %v", err)
|
||||
}
|
||||
if err := os.MkdirAll(filepath.Join(townRoot, "beads", "crew"), 0o755); err != nil {
|
||||
t.Fatalf("create beads rig: %v", err)
|
||||
}
|
||||
|
||||
lister := &mockSessionLister{
|
||||
sessions: []string{
|
||||
"gt-gastown-witness", // valid: gastown rig exists
|
||||
"gt-gastown-polecat1", // valid: gastown rig exists
|
||||
"gt-beads-refinery", // valid: beads rig exists
|
||||
"gt-unknown-witness", // orphan: unknown rig doesn't exist
|
||||
"gt-missing-crew-joe", // orphan: missing rig doesn't exist
|
||||
"random-session", // ignored: doesn't match gt-* pattern
|
||||
},
|
||||
}
|
||||
check := NewOrphanSessionCheckWithSessionLister(lister)
|
||||
result := check.Run(&CheckContext{TownRoot: townRoot})
|
||||
|
||||
if result.Status != StatusWarning {
|
||||
t.Fatalf("expected StatusWarning, got %v: %s", result.Status, result.Message)
|
||||
}
|
||||
if result.Message != "Found 2 orphaned session(s)" {
|
||||
t.Fatalf("unexpected message: %q", result.Message)
|
||||
}
|
||||
if result.FixHint == "" {
|
||||
t.Fatal("expected FixHint to be set for orphan sessions")
|
||||
}
|
||||
|
||||
expectedOrphans := []string{"gt-unknown-witness", "gt-missing-crew-joe"}
|
||||
if !reflect.DeepEqual(check.orphanSessions, expectedOrphans) {
|
||||
t.Fatalf("cached orphans = %v, want %v", check.orphanSessions, expectedOrphans)
|
||||
}
|
||||
|
||||
expectedDetails := []string{"Orphan: gt-unknown-witness", "Orphan: gt-missing-crew-joe"}
|
||||
if !reflect.DeepEqual(result.Details, expectedDetails) {
|
||||
t.Fatalf("details = %v, want %v", result.Details, expectedDetails)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user