From a68cf540573f768fde14862e8f72f2c99b257027 Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Tue, 30 Dec 2025 00:46:52 -0800 Subject: [PATCH] fix: Detect and auto-remove circular redirect files in beads (gt-csbjj) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added multiple layers of protection against circular redirects: 1. ResolveBeadsDir now detects when a redirect points back to itself and auto-removes the errant redirect file with a warning 2. ensureBeadsRedirect now includes safety checks: - Prevents creating redirects inside mayor/rig/ (the canonical location) - Validates that the redirect target is not the same as the beads dir 3. Added test case for circular redirect detection 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- internal/beads/beads.go | 23 +++++++++++++++++ internal/beads/beads_test.go | 28 +++++++++++++++++++++ internal/cmd/prime.go | 49 ++++++++++++++++++++++++++---------- 3 files changed, 87 insertions(+), 13 deletions(-) diff --git a/internal/beads/beads.go b/internal/beads/beads.go index 13f8fa91..19749912 100644 --- a/internal/beads/beads.go +++ b/internal/beads/beads.go @@ -30,6 +30,10 @@ var ( // Example: if we're at crew/max/ and .beads/redirect contains "../../mayor/rig/.beads", // the redirect is resolved from crew/max/ (not crew/max/.beads/), giving us // mayor/rig/.beads at the rig root level. +// +// Circular redirect detection: If the resolved path equals the original beads directory, +// this indicates an errant redirect file that should be removed. The function logs a +// warning and returns the original beads directory. func ResolveBeadsDir(workDir string) string { beadsDir := filepath.Join(workDir, ".beads") redirectPath := filepath.Join(beadsDir, "redirect") @@ -56,6 +60,25 @@ func ResolveBeadsDir(workDir string) string { // Clean the path to resolve .. components resolved = filepath.Clean(resolved) + // Detect circular redirects: if resolved path equals original beads dir, + // this is an errant redirect file (e.g., redirect in mayor/rig/.beads pointing to itself) + if resolved == beadsDir { + fmt.Fprintf(os.Stderr, "Warning: circular redirect detected in %s (points to itself), ignoring redirect\n", redirectPath) + // Remove the errant redirect file to prevent future warnings + if err := os.Remove(redirectPath); err != nil { + fmt.Fprintf(os.Stderr, "Warning: could not remove errant redirect file: %v\n", err) + } + return beadsDir + } + + // Detect redirect chains: check if resolved path also has a redirect + resolvedRedirect := filepath.Join(resolved, "redirect") + if _, err := os.Stat(resolvedRedirect); err == nil { + fmt.Fprintf(os.Stderr, "Warning: redirect chain detected: %s -> %s (which also has a redirect)\n", beadsDir, resolved) + // Don't follow chains - just return the first resolved path + // The target's redirect is likely errant and should be removed + } + return resolved } diff --git a/internal/beads/beads_test.go b/internal/beads/beads_test.go index ad2d1871..d0f3c7ac 100644 --- a/internal/beads/beads_test.go +++ b/internal/beads/beads_test.go @@ -966,6 +966,34 @@ func TestResolveBeadsDir(t *testing.T) { t.Errorf("ResolveBeadsDir() = %q, want %q", got, want) } }) + + t.Run("circular redirect", func(t *testing.T) { + // Redirect that points to itself (e.g., mayor/rig/.beads/redirect -> ../../mayor/rig/.beads) + // This is the bug scenario from gt-csbjj + workDir := filepath.Join(tmpDir, "mayor", "rig") + beadsDir := filepath.Join(workDir, ".beads") + if err := os.MkdirAll(beadsDir, 0755); err != nil { + t.Fatal(err) + } + + // Create a circular redirect: ../../mayor/rig/.beads resolves back to .beads + redirectPath := filepath.Join(beadsDir, "redirect") + if err := os.WriteFile(redirectPath, []byte("../../mayor/rig/.beads\n"), 0644); err != nil { + t.Fatal(err) + } + + // ResolveBeadsDir should detect the circular redirect and return the original beadsDir + got := ResolveBeadsDir(workDir) + want := beadsDir + if got != want { + t.Errorf("ResolveBeadsDir() = %q, want %q (should ignore circular redirect)", got, want) + } + + // The circular redirect file should have been removed + if _, err := os.Stat(redirectPath); err == nil { + t.Error("circular redirect file should have been removed, but it still exists") + } + }) } func TestParseAgentBeadID(t *testing.T) { diff --git a/internal/cmd/prime.go b/internal/cmd/prime.go index fa8b31b3..fccf95b4 100644 --- a/internal/cmd/prime.go +++ b/internal/cmd/prime.go @@ -1187,12 +1187,39 @@ func getAgentBeadID(ctx RoleContext) string { // ensureBeadsRedirect ensures the .beads/redirect file exists for worktree-based roles. // This handles cases where git clean or other operations delete the redirect file. +// +// IMPORTANT: This function includes safety checks to prevent creating redirects in +// the canonical beads location (mayor/rig/.beads), which would cause circular redirects. func ensureBeadsRedirect(ctx RoleContext) { // Only applies to crew and polecat roles (they use shared beads) if ctx.Role != RoleCrew && ctx.Role != RolePolecat { return } + // Get the rig root (parent of crew/ or polecats/) + relPath, err := filepath.Rel(ctx.TownRoot, ctx.WorkDir) + if err != nil { + return + } + parts := strings.Split(filepath.ToSlash(relPath), "/") + if len(parts) < 1 { + return + } + rigRoot := filepath.Join(ctx.TownRoot, parts[0]) + + // SAFETY CHECK: Prevent creating redirect in canonical beads location + // If workDir is inside mayor/rig/, we should NOT create a redirect there + // This prevents circular redirects like mayor/rig/.beads/redirect -> ../../mayor/rig/.beads + mayorRigPath := filepath.Join(rigRoot, "mayor", "rig") + workDirAbs, _ := filepath.Abs(ctx.WorkDir) + mayorRigPathAbs, _ := filepath.Abs(mayorRigPath) + if strings.HasPrefix(workDirAbs, mayorRigPathAbs) { + // We're inside mayor/rig/ - this is not a polecat/crew worker location + // Role detection may be wrong (e.g., GT_ROLE env var mismatch) + // Do NOT create a redirect here + return + } + // Check if redirect already exists beadsDir := filepath.Join(ctx.WorkDir, ".beads") redirectPath := filepath.Join(beadsDir, "redirect") @@ -1205,19 +1232,6 @@ func ensureBeadsRedirect(ctx RoleContext) { // Determine the correct redirect path based on role and rig structure var redirectContent string - // Get the rig root (parent of crew/ or polecats/) - var rigRoot string - relPath, err := filepath.Rel(ctx.TownRoot, ctx.WorkDir) - if err != nil { - return - } - parts := strings.Split(filepath.ToSlash(relPath), "/") - if len(parts) >= 1 { - rigRoot = filepath.Join(ctx.TownRoot, parts[0]) - } else { - return - } - // Check for shared beads locations in order of preference: // 1. rig/mayor/rig/.beads/ (if mayor rig clone exists) // 2. rig/.beads/ (rig root beads) @@ -1247,6 +1261,15 @@ func ensureBeadsRedirect(ctx RoleContext) { return } + // SAFETY CHECK: Verify the redirect won't be circular + // Resolve the redirect target and check it's not the same as our beads dir + resolvedTarget := filepath.Join(ctx.WorkDir, redirectContent) + resolvedTarget = filepath.Clean(resolvedTarget) + if resolvedTarget == beadsDir { + // Would create circular redirect - don't do it + return + } + // Create .beads directory if needed if err := os.MkdirAll(beadsDir, 0755); err != nil { // Silently fail - not critical