refactor(config): remove BEADS_DIR from agent environment and add doctor check (#455)

* fix(sling_test): update test for cook dir change The cook command no longer needs database context and runs from cwd, not the target rig directory. Update test to match this behavior change from bd2a5ab5. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(tests): skip tests requiring missing binaries, handle --allow-stale - Add skipIfAgentBinaryMissing helper to skip tests when codex/gemini binaries aren't available in the test environment - Update rig manager test stub to handle --allow-stale flag Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor(config): remove BEADS_DIR from agent environment Stop exporting BEADS_DIR in AgentEnv - agents should use beads redirect mechanism instead of relying on environment variable. This prevents prefix mismatches when agents operate across different beads databases. Changes: - Remove BeadsDir field from AgentEnvConfig - Remove BEADS_DIR from env vars set on agent sessions - Update doctor env_check to not expect BEADS_DIR - Update all manager Start() calls to not pass BeadsDir Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(doctor): detect BEADS_DIR in tmux session environment Add a doctor check that warns when BEADS_DIR is set in any Gas Town tmux session. BEADS_DIR in the environment overrides prefix-based routing and breaks multi-rig lookups - agents should use the beads redirect mechanism instead. The check: - Iterates over all Gas Town tmux sessions (gt-* and hq-*) - Checks if BEADS_DIR is set in the session environment - Returns a warning with fix hint to restart sessions Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: julianknutsen <julianknutsen@users.noreply.github> Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
2026-01-14 06:13:57 +00:00
parent 3cf77b2e8b
commit e7ca4908dc
19 changed files with 252 additions and 126 deletions
@@ -2,10 +2,8 @@ package doctor

 import (
 	"fmt"
-	"path/filepath"
 	"strings"

-	"github.com/steveyegge/gastown/internal/beads"
 	"github.com/steveyegge/gastown/internal/config"
 	"github.com/steveyegge/gastown/internal/session"
 	"github.com/steveyegge/gastown/internal/tmux"
@@ -89,6 +87,7 @@ func (c *EnvVarsCheck) Run(ctx *CheckContext) *CheckResult {
 	}

 	var mismatches []string
+	var beadsDirWarnings []string
 	checkedCount := 0

 	for _, sess := range gtSessions {
@@ -98,21 +97,12 @@ func (c *EnvVarsCheck) Run(ctx *CheckContext) *CheckResult {
 			continue
 		}

-		// Get expected env vars based on role, emulating real call sites
-		// Town-level roles use TownRoot for beads, rig-level roles use rig path
-		var beadsDir string
-		if identity.Rig != "" {
-			rigPath := filepath.Join(ctx.TownRoot, identity.Rig)
-			beadsDir = beads.ResolveBeadsDir(rigPath)
-		} else {
-			beadsDir = beads.ResolveBeadsDir(ctx.TownRoot)
-		}
+		// Get expected env vars based on role
 		expected := config.AgentEnv(config.AgentEnvConfig{
 			Role:      string(identity.Role),
 			Rig:       identity.Rig,
 			AgentName: identity.Name,
 			TownRoot:  ctx.TownRoot,
-			BeadsDir:  beadsDir,
 		})

 		// Get actual tmux env vars
@@ -133,6 +123,31 @@ func (c *EnvVarsCheck) Run(ctx *CheckContext) *CheckResult {
 				mismatches = append(mismatches, fmt.Sprintf("%s: %s=%q (expected %q)", sess, key, actualVal, expectedVal))
 			}
 		}
+
+		// Check for BEADS_DIR - this breaks routing-based lookups
+		if beadsDir, exists := actual["BEADS_DIR"]; exists && beadsDir != "" {
+			beadsDirWarnings = append(beadsDirWarnings, fmt.Sprintf("%s: BEADS_DIR=%q (breaks prefix routing)", sess, beadsDir))
+		}
+	}
+
+	// Check for BEADS_DIR issues first (higher priority warning)
+	if len(beadsDirWarnings) > 0 {
+		details := beadsDirWarnings
+		if len(mismatches) > 0 {
+			details = append(details, "", "Other env var issues:")
+			details = append(details, mismatches...)
+		}
+		details = append(details,
+			"",
+			"BEADS_DIR overrides prefix-based routing and breaks multi-rig lookups.",
+		)
+		return &CheckResult{
+			Name:    c.Name(),
+			Status:  StatusWarning,
+			Message: fmt.Sprintf("Found BEADS_DIR set in %d session(s)", len(beadsDirWarnings)),
+			Details: details,
+			FixHint: "Remove BEADS_DIR from session environment: gt shutdown && gt up",
+		}
 	}

 	if len(mismatches) == 0 {
@@ -2,6 +2,7 @@ package doctor

 import (
 	"errors"
+	"strings"
 	"testing"

 	"github.com/steveyegge/gastown/internal/config"
@@ -41,21 +42,12 @@ func (m *mockEnvReader) GetAllEnvironment(session string) (map[string]string, er
 const testTownRoot = "/town"

 // expectedEnv generates expected env vars matching what the check generates.
-// For town-level roles (mayor, deacon), beadsDir is /town/.beads
-// For rig-level roles, beadsDir is /town/rigName/.beads
 func expectedEnv(role, rig, agentName string) map[string]string {
-	var beadsDir string
-	if rig != "" {
-		beadsDir = testTownRoot + "/" + rig + "/.beads"
-	} else {
-		beadsDir = testTownRoot + "/.beads"
-	}
 	return config.AgentEnv(config.AgentEnvConfig{
 		Role:      role,
 		Rig:       rig,
 		AgentName: agentName,
 		TownRoot:  testTownRoot,
-		BeadsDir:  beadsDir,
 	})
 }

@@ -348,3 +340,101 @@ func TestEnvVarsCheck_HyphenatedRig(t *testing.T) {
 		t.Errorf("Status = %v, want StatusOK", result.Status)
 	}
 }
+
+func TestEnvVarsCheck_BeadsDirWarning(t *testing.T) {
+	// BEADS_DIR being set breaks prefix-based routing
+	expected := expectedEnv("witness", "myrig", "")
+	expected["BEADS_DIR"] = "/some/path/.beads" // This shouldn't be set!
+	reader := &mockEnvReader{
+		sessions: []string{"gt-myrig-witness"},
+		sessionEnvs: map[string]map[string]string{
+			"gt-myrig-witness": expected,
+		},
+	}
+	check := NewEnvVarsCheckWithReader(reader)
+	result := check.Run(testCtx())
+
+	if result.Status != StatusWarning {
+		t.Errorf("Status = %v, want StatusWarning", result.Status)
+	}
+	if !strings.Contains(result.Message, "BEADS_DIR") {
+		t.Errorf("Message should mention BEADS_DIR, got: %q", result.Message)
+	}
+	if !strings.Contains(result.FixHint, "gt shutdown") {
+		t.Errorf("FixHint should mention restart, got: %q", result.FixHint)
+	}
+}
+
+func TestEnvVarsCheck_BeadsDirEmptyIsOK(t *testing.T) {
+	// Empty BEADS_DIR should not warn
+	expected := expectedEnv("witness", "myrig", "")
+	expected["BEADS_DIR"] = "" // Empty is fine
+	reader := &mockEnvReader{
+		sessions: []string{"gt-myrig-witness"},
+		sessionEnvs: map[string]map[string]string{
+			"gt-myrig-witness": expected,
+		},
+	}
+	check := NewEnvVarsCheckWithReader(reader)
+	result := check.Run(testCtx())
+
+	if result.Status != StatusOK {
+		t.Errorf("Status = %v, want StatusOK for empty BEADS_DIR", result.Status)
+	}
+}
+
+func TestEnvVarsCheck_BeadsDirMultipleSessions(t *testing.T) {
+	// Multiple sessions, only one has BEADS_DIR
+	witnessEnv := expectedEnv("witness", "myrig", "")
+	polecatEnv := expectedEnv("polecat", "myrig", "Toast")
+	polecatEnv["BEADS_DIR"] = "/bad/path" // This shouldn't be set!
+
+	reader := &mockEnvReader{
+		sessions: []string{"gt-myrig-witness", "gt-myrig-Toast"},
+		sessionEnvs: map[string]map[string]string{
+			"gt-myrig-witness": witnessEnv,
+			"gt-myrig-Toast":   polecatEnv,
+		},
+	}
+	check := NewEnvVarsCheckWithReader(reader)
+	result := check.Run(testCtx())
+
+	if result.Status != StatusWarning {
+		t.Errorf("Status = %v, want StatusWarning", result.Status)
+	}
+	if !strings.Contains(result.Message, "1 session") {
+		t.Errorf("Message should mention 1 session with BEADS_DIR, got: %q", result.Message)
+	}
+}
+
+func TestEnvVarsCheck_BeadsDirWithOtherMismatches(t *testing.T) {
+	// Session has BEADS_DIR AND other mismatches - both should be reported
+	reader := &mockEnvReader{
+		sessions: []string{"gt-myrig-witness"},
+		sessionEnvs: map[string]map[string]string{
+			"gt-myrig-witness": {
+				"GT_ROLE":   "witness",
+				"GT_RIG":    "wrongrig", // Mismatch
+				"BEADS_DIR": "/bad/path",
+			},
+		},
+	}
+	check := NewEnvVarsCheckWithReader(reader)
+	result := check.Run(testCtx())
+
+	if result.Status != StatusWarning {
+		t.Errorf("Status = %v, want StatusWarning", result.Status)
+	}
+	// BEADS_DIR takes priority in message
+	if !strings.Contains(result.Message, "BEADS_DIR") {
+		t.Errorf("Message should prioritize BEADS_DIR, got: %q", result.Message)
+	}
+	// But details should include both
+	detailsStr := strings.Join(result.Details, "\n")
+	if !strings.Contains(detailsStr, "BEADS_DIR") {
+		t.Errorf("Details should mention BEADS_DIR")
+	}
+	if !strings.Contains(detailsStr, "Other env var issues") {
+		t.Errorf("Details should mention other issues")
+	}
+}