fix: Detect and auto-remove circular redirect files in beads (gt-csbjj)
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
}
|
||||
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user