mirror of
https://github.com/boshu2/agentops.git
synced 2026-09-14 15:08:13 +08:00
fix(rpi): avoid detached-branch sprawl and add branch pruning control
This commit is contained in:
+136
-14
@@ -6,6 +6,7 @@ import (
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/spf13/cobra"
|
||||
@@ -15,6 +16,7 @@ var (
|
||||
cleanupRunID string
|
||||
cleanupAll bool
|
||||
cleanupPruneWorktrees bool
|
||||
cleanupPruneBranches bool
|
||||
cleanupDryRun bool
|
||||
cleanupStaleAfter time.Duration
|
||||
)
|
||||
@@ -38,6 +40,7 @@ Examples:
|
||||
}
|
||||
cleanupCmd.Flags().StringVar(&cleanupRunID, "run-id", "", "Clean up a specific run by ID")
|
||||
cleanupCmd.Flags().BoolVar(&cleanupAll, "all", false, "Clean up all stale runs")
|
||||
cleanupCmd.Flags().BoolVar(&cleanupPruneBranches, "prune-branches", false, "Delete legacy RPI branches (rpi/*, codex/auto-rpi-*)")
|
||||
cleanupCmd.Flags().BoolVar(&cleanupPruneWorktrees, "prune-worktrees", false, "Run 'git worktree prune' after cleanup")
|
||||
cleanupCmd.Flags().BoolVar(&cleanupDryRun, "dry-run", false, "Show what would be done without making changes")
|
||||
cleanupCmd.Flags().DurationVar(&cleanupStaleAfter, "stale-after", 0, "Only clean runs older than this age (0 disables age filtering)")
|
||||
@@ -50,10 +53,10 @@ func runRPICleanup(cmd *cobra.Command, args []string) error {
|
||||
return fmt.Errorf("get working directory: %w", err)
|
||||
}
|
||||
|
||||
return executeRPICleanup(cwd, cleanupRunID, cleanupAll, cleanupPruneWorktrees, cleanupDryRun, cleanupStaleAfter)
|
||||
return executeRPICleanup(cwd, cleanupRunID, cleanupAll, cleanupPruneWorktrees, cleanupPruneBranches, cleanupDryRun, cleanupStaleAfter)
|
||||
}
|
||||
|
||||
func executeRPICleanup(cwd, runID string, all, prune, dryRun bool, staleAfter time.Duration) error {
|
||||
func executeRPICleanup(cwd, runID string, all, prune, pruneBranches bool, dryRun bool, staleAfter time.Duration) error {
|
||||
if !all && runID == "" {
|
||||
return fmt.Errorf("specify --all or --run-id <id>")
|
||||
}
|
||||
@@ -80,6 +83,11 @@ func executeRPICleanup(cwd, runID string, all, prune, dryRun bool, staleAfter ti
|
||||
|
||||
if len(staleRuns) == 0 {
|
||||
fmt.Println("No stale runs found.")
|
||||
if pruneBranches {
|
||||
if err := cleanupLegacyRPIBranches(cwd, runID, all, dryRun); err != nil {
|
||||
fmt.Fprintf(os.Stderr, "Warning: legacy branch cleanup failed: %v\n", err)
|
||||
}
|
||||
}
|
||||
if prune && !dryRun {
|
||||
return pruneWorktrees(cwd)
|
||||
}
|
||||
@@ -130,9 +138,121 @@ func executeRPICleanup(cwd, runID string, all, prune, dryRun bool, staleAfter ti
|
||||
}
|
||||
}
|
||||
|
||||
if pruneBranches {
|
||||
if err := cleanupLegacyRPIBranches(cwd, runID, all, dryRun); err != nil {
|
||||
fmt.Fprintf(os.Stderr, "Warning: legacy branch cleanup failed: %v\n", err)
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// cleanupLegacyRPIBranches removes legacy RPI branches for the selected scope.
|
||||
func cleanupLegacyRPIBranches(cwd, runID string, all, dryRun bool) error {
|
||||
runID = strings.TrimSpace(runID)
|
||||
if runID == "" && !all {
|
||||
return fmt.Errorf("specify --all or --run-id to prune branches")
|
||||
}
|
||||
if strings.TrimSpace(cwd) == "" {
|
||||
return fmt.Errorf("cleanup branch command missing repository path")
|
||||
}
|
||||
|
||||
candidates, err := collectLegacyRPIBranches(cwd, runID, all)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if len(candidates) == 0 {
|
||||
fmt.Println("No legacy RPI branches found for cleanup.")
|
||||
return nil
|
||||
}
|
||||
|
||||
activeBranches, err := checkedOutBranchSet(cwd)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
for _, name := range candidates {
|
||||
if activeBranches[name] {
|
||||
fmt.Printf("Skipping active branch: %s\n", name)
|
||||
continue
|
||||
}
|
||||
if dryRun {
|
||||
fmt.Printf("[dry-run] Would delete branch: %s\n", name)
|
||||
continue
|
||||
}
|
||||
|
||||
cmd := exec.Command("git", "branch", "-D", name)
|
||||
cmd.Dir = cwd
|
||||
if err := cmd.Run(); err != nil {
|
||||
fmt.Fprintf(os.Stderr, "Warning: failed to delete branch %s: %v\n", name, err)
|
||||
continue
|
||||
}
|
||||
fmt.Printf("Deleted branch: %s\n", name)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
func collectLegacyRPIBranches(cwd, runID string, all bool) ([]string, error) {
|
||||
branchPatterns := []string{}
|
||||
if all {
|
||||
branchPatterns = append(branchPatterns, "rpi/*", "codex/auto-rpi-*")
|
||||
} else {
|
||||
branchPatterns = append(branchPatterns, "rpi/"+runID)
|
||||
}
|
||||
|
||||
seen := map[string]struct{}{}
|
||||
var branches []string
|
||||
|
||||
for _, pattern := range branchPatterns {
|
||||
refPattern := "refs/heads/" + pattern
|
||||
cmd := exec.Command("git", "for-each-ref", "--format=%(refname:short)", refPattern)
|
||||
cmd.Dir = cwd
|
||||
out, err := cmd.CombinedOutput()
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("list branches (%s): %w", pattern, err)
|
||||
}
|
||||
|
||||
for _, raw := range strings.Split(string(out), "\n") {
|
||||
name := strings.TrimSpace(raw)
|
||||
if name == "" {
|
||||
continue
|
||||
}
|
||||
if _, ok := seen[name]; !ok {
|
||||
seen[name] = struct{}{}
|
||||
branches = append(branches, name)
|
||||
}
|
||||
}
|
||||
}
|
||||
return branches, nil
|
||||
}
|
||||
|
||||
func checkedOutBranchSet(cwd string) (map[string]bool, error) {
|
||||
cmd := exec.Command("git", "worktree", "list", "--porcelain")
|
||||
cmd.Dir = cwd
|
||||
out, err := cmd.CombinedOutput()
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("git worktree list: %w", err)
|
||||
}
|
||||
|
||||
active := map[string]bool{}
|
||||
for _, line := range strings.Split(string(out), "\n") {
|
||||
line = strings.TrimSpace(line)
|
||||
const prefix = "branch "
|
||||
if !strings.HasPrefix(line, prefix) {
|
||||
continue
|
||||
}
|
||||
ref := strings.TrimPrefix(line, prefix)
|
||||
ref = strings.TrimSpace(ref)
|
||||
const refsHeads = "refs/heads/"
|
||||
if strings.HasPrefix(ref, refsHeads) {
|
||||
active[strings.TrimPrefix(ref, refsHeads)] = true
|
||||
}
|
||||
}
|
||||
|
||||
return active, nil
|
||||
}
|
||||
|
||||
// resolveCleanupRepoRoot picks a controller worktree root to execute
|
||||
// `git worktree remove` against. It prefers a sibling worktree in the same
|
||||
// parent directory as targetWorktree, avoiding attempts to remove a worktree
|
||||
@@ -193,12 +313,6 @@ func findStaleRunsWithMinAge(root string, minAge time.Duration, now time.Time) [
|
||||
continue
|
||||
}
|
||||
|
||||
// Check liveness.
|
||||
isActive, _ := determineRunLiveness(root, state)
|
||||
if isActive {
|
||||
continue
|
||||
}
|
||||
|
||||
// Terminal runs (except completed) are cleanup candidates only when their
|
||||
// worktree still exists.
|
||||
if state.TerminalStatus != "" {
|
||||
@@ -237,6 +351,12 @@ func findStaleRunsWithMinAge(root string, minAge time.Duration, now time.Time) [
|
||||
continue
|
||||
}
|
||||
|
||||
// Check liveness.
|
||||
isActive, _ := determineRunLiveness(root, state)
|
||||
if isActive {
|
||||
continue
|
||||
}
|
||||
|
||||
// Non-terminal completed runs are not stale.
|
||||
if state.Phase >= completedPhaseNumber(*state) {
|
||||
continue
|
||||
@@ -317,7 +437,7 @@ func markRunStale(sr staleRunEntry) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// removeOrphanedWorktree removes a worktree directory and its branch.
|
||||
// removeOrphanedWorktree removes a worktree directory and any legacy branch marker.
|
||||
func removeOrphanedWorktree(repoRoot, worktreePath, runID string) error {
|
||||
// Safety: validate that worktreePath is a sibling of the repo root (same parent dir).
|
||||
// Worktrees are created as ../repo-rpi-<id>/ — siblings of the repo, not children.
|
||||
@@ -343,11 +463,13 @@ func removeOrphanedWorktree(repoRoot, worktreePath, runID string) error {
|
||||
}
|
||||
}
|
||||
|
||||
// Delete the branch.
|
||||
branchName := "rpi/" + runID
|
||||
branchCmd := exec.Command("git", "branch", "-D", branchName)
|
||||
branchCmd.Dir = repoRoot
|
||||
_ = branchCmd.Run() // Best-effort; branch may not exist.
|
||||
// Delete legacy branch marker if present.
|
||||
if strings.TrimSpace(runID) != "" {
|
||||
branchName := "rpi/" + runID
|
||||
branchCmd := exec.Command("git", "branch", "-D", branchName)
|
||||
branchCmd.Dir = repoRoot
|
||||
_ = branchCmd.Run() // Best-effort; branch may not exist.
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -3,6 +3,7 @@ package main
|
||||
import (
|
||||
"encoding/json"
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
@@ -336,7 +337,7 @@ func TestExecuteRPICleanup_StaleAfterOnlyMarksOldRuns(t *testing.T) {
|
||||
makeRun("old-run", now.Add(-2*time.Hour))
|
||||
makeRun("new-run", now.Add(-10*time.Minute))
|
||||
|
||||
if err := executeRPICleanup(tmpDir, "", true, false, false, 1*time.Hour); err != nil {
|
||||
if err := executeRPICleanup(tmpDir, "", true, false, false, false, 1*time.Hour); err != nil {
|
||||
t.Fatalf("executeRPICleanup: %v", err)
|
||||
}
|
||||
|
||||
@@ -465,3 +466,87 @@ func TestRemoveOrphanedWorktree_RepoRootProtection(t *testing.T) {
|
||||
t.Fatalf("sentinel file was deleted — repo root was removed!")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCollectLegacyRPIBranches_RunIDScope(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
repoPath := tmpDir
|
||||
|
||||
runGit := func(args ...string) {
|
||||
t.Helper()
|
||||
cmd := exec.Command("git", args...)
|
||||
cmd.Dir = repoPath
|
||||
if output, err := cmd.CombinedOutput(); err != nil {
|
||||
t.Fatalf("git %v failed: %v (%s)", strings.Join(args, " "), err, string(output))
|
||||
}
|
||||
}
|
||||
|
||||
runGit("init", "-q")
|
||||
runGit("config", "user.email", "noreply@example.com")
|
||||
runGit("config", "user.name", "Test User")
|
||||
runGit("checkout", "-q", "-b", "main")
|
||||
runGit("commit", "-q", "--allow-empty", "-m", "init")
|
||||
runGit("branch", "-q", "rpi/target")
|
||||
runGit("branch", "-q", "rpi/other")
|
||||
runGit("branch", "-q", "codex/auto-rpi-old")
|
||||
|
||||
branches, err := collectLegacyRPIBranches(repoPath, "target", false)
|
||||
if err != nil {
|
||||
t.Fatalf("collect branches: %v", err)
|
||||
}
|
||||
if len(branches) != 1 || branches[0] != "rpi/target" {
|
||||
t.Fatalf("expected only runID branch, got %v", branches)
|
||||
}
|
||||
|
||||
if err := cleanupLegacyRPIBranches(repoPath, "target", false, true); err != nil {
|
||||
t.Fatalf("cleanup dry run: %v", err)
|
||||
}
|
||||
if err := runGitCheckBranch(repoPath, "rpi/target"); err != nil {
|
||||
t.Fatalf("dry-run should preserve branch: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCleanupLegacyRPIBranches_AllAndActiveSafety(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
repoPath := tmpDir
|
||||
|
||||
runGit := func(args ...string) {
|
||||
t.Helper()
|
||||
cmd := exec.Command("git", args...)
|
||||
cmd.Dir = repoPath
|
||||
if output, err := cmd.CombinedOutput(); err != nil {
|
||||
t.Fatalf("git %v failed: %v (%s)", strings.Join(args, " "), err, string(output))
|
||||
}
|
||||
}
|
||||
|
||||
runGit("init", "-q")
|
||||
runGit("config", "user.email", "noreply@example.com")
|
||||
runGit("config", "user.name", "Test User")
|
||||
runGit("checkout", "-q", "-b", "main")
|
||||
runGit("commit", "-q", "--allow-empty", "-m", "init")
|
||||
runGit("checkout", "-q", "-b", "rpi/active")
|
||||
runGit("branch", "-q", "rpi/inactive")
|
||||
runGit("branch", "-q", "codex/auto-rpi-old")
|
||||
runGit("checkout", "-q", "main")
|
||||
worktreeActivePath := filepath.Join(repoPath, "active-worktree")
|
||||
runGit("worktree", "add", worktreeActivePath, "rpi/active")
|
||||
|
||||
if err := cleanupLegacyRPIBranches(repoPath, "", true, false); err != nil {
|
||||
t.Fatalf("cleanup all: %v", err)
|
||||
}
|
||||
|
||||
if err := runGitCheckBranch(repoPath, "rpi/active"); err != nil {
|
||||
t.Fatalf("active branch should be preserved: %v", err)
|
||||
}
|
||||
if err := runGitCheckBranch(repoPath, "codex/auto-rpi-old"); err == nil {
|
||||
t.Fatalf("codex/auto-rpi-old branch should be removed")
|
||||
}
|
||||
if err := runGitCheckBranch(repoPath, "rpi/inactive"); err == nil {
|
||||
t.Fatalf("rpi/inactive branch should be removed")
|
||||
}
|
||||
}
|
||||
|
||||
func runGitCheckBranch(repoPath, name string) error {
|
||||
cmd := exec.Command("git", "show-ref", "--verify", "--quiet", "refs/heads/"+name)
|
||||
cmd.Dir = repoPath
|
||||
return cmd.Run()
|
||||
}
|
||||
|
||||
@@ -115,7 +115,7 @@ func resolveLoopSupervisorConfig(cmd *cobra.Command, cwd string) (rpiLoopSupervi
|
||||
cfg.LeaseEnabled = true
|
||||
}
|
||||
if !cmd.Flags().Changed("detached-heal") {
|
||||
cfg.DetachedHeal = true
|
||||
cfg.DetachedHeal = false
|
||||
}
|
||||
if !cmd.Flags().Changed("auto-clean") {
|
||||
cfg.AutoClean = true
|
||||
@@ -149,6 +149,9 @@ func resolveLoopSupervisorConfig(cmd *cobra.Command, cwd string) (rpiLoopSupervi
|
||||
if cfg.AutoCleanStaleAfter <= 0 {
|
||||
cfg.AutoCleanStaleAfter = 24 * time.Hour
|
||||
}
|
||||
if rpiSupervisor && !cmd.Flags().Changed("auto-clean-stale-after") {
|
||||
cfg.AutoCleanStaleAfter = 0
|
||||
}
|
||||
if cfg.LeasePath == "" {
|
||||
cfg.LeasePath = filepath.Join(".agents", "rpi", "supervisor.lock")
|
||||
}
|
||||
@@ -315,7 +318,7 @@ func ensureLoopAttachedBranch(cwd, branchPrefix string) (string, bool, error) {
|
||||
}
|
||||
|
||||
func runSupervisorCleanup(cwd string, staleAfter time.Duration, prune bool) error {
|
||||
return executeRPICleanup(cwd, "", true, prune, GetDryRun(), staleAfter)
|
||||
return executeRPICleanup(cwd, "", true, prune, false, GetDryRun(), staleAfter)
|
||||
}
|
||||
|
||||
func runSupervisorGates(cwd string, cfg rpiLoopSupervisorConfig) error {
|
||||
|
||||
@@ -56,8 +56,8 @@ func TestResolveLoopSupervisorConfig_AppliesSupervisorDefaults(t *testing.T) {
|
||||
if !cfg.LeaseEnabled {
|
||||
t.Fatal("expected lease to be enabled in supervisor defaults")
|
||||
}
|
||||
if !cfg.DetachedHeal {
|
||||
t.Fatal("expected detached heal to be enabled in supervisor defaults")
|
||||
if cfg.DetachedHeal {
|
||||
t.Fatal("expected detached heal to be disabled in supervisor defaults")
|
||||
}
|
||||
if !cfg.AutoClean {
|
||||
t.Fatal("expected auto-clean to be enabled in supervisor defaults")
|
||||
|
||||
@@ -607,7 +607,7 @@ func runRPIPhasedWithOpts(opts phasedEngineOptions, args []string) (retErr error
|
||||
minAge = 24 * time.Hour
|
||||
}
|
||||
fmt.Printf("Auto-cleaning stale runs older than %s before starting\n", minAge)
|
||||
if err := executeRPICleanup(cwd, "", true, false, GetDryRun(), minAge); err != nil {
|
||||
if err := executeRPICleanup(cwd, "", true, false, false, GetDryRun(), minAge); err != nil {
|
||||
VerbosePrintf("Warning: auto-clean stale runs failed: %v\n", err)
|
||||
}
|
||||
}
|
||||
@@ -657,6 +657,12 @@ func runRPIPhasedWithOpts(opts phasedEngineOptions, args []string) (retErr error
|
||||
updateRunHeartbeat(spawnCwd, state.RunID)
|
||||
|
||||
if err := runPhaseLoop(cwd, spawnCwd, state, startPhase, opts, statusPath, allPhases, logPath, executor); err != nil {
|
||||
state.TerminalStatus = "failed"
|
||||
state.TerminalReason = err.Error()
|
||||
state.TerminatedAt = time.Now().Format(time.RFC3339)
|
||||
if saveErr := savePhasedState(spawnCwd, state); saveErr != nil {
|
||||
VerbosePrintf("Warning: could not persist failed terminal state: %v\n", saveErr)
|
||||
}
|
||||
return err
|
||||
}
|
||||
|
||||
@@ -1776,7 +1782,6 @@ func getCurrentBranch(repoRoot string) (string, error) {
|
||||
|
||||
// createWorktree creates a sibling git worktree for isolated RPI execution.
|
||||
// Path: ../<repo-basename>-rpi-<runID>/
|
||||
// Branch: rpi/<runID>
|
||||
func createWorktree(cwd string) (worktreePath, runID string, err error) {
|
||||
return cliRPI.CreateWorktree(cwd, worktreeTimeout, VerbosePrintf)
|
||||
}
|
||||
@@ -1784,11 +1789,11 @@ func createWorktree(cwd string) (worktreePath, runID string, err error) {
|
||||
// mergeWorktree merges the RPI worktree branch back into the original branch.
|
||||
// Retries the pre-merge dirty check with backoff to handle the race where
|
||||
// another parallel run is mid-merge (repo momentarily dirty).
|
||||
func mergeWorktree(repoRoot, runID string) error {
|
||||
return cliRPI.MergeWorktree(repoRoot, runID, worktreeTimeout, VerbosePrintf)
|
||||
func mergeWorktree(repoRoot, worktreePath, runID string) error {
|
||||
return cliRPI.MergeWorktree(repoRoot, worktreePath, runID, worktreeTimeout, VerbosePrintf)
|
||||
}
|
||||
|
||||
// removeWorktree removes a worktree directory and its branch.
|
||||
// removeWorktree removes a worktree directory and any legacy branch marker.
|
||||
// Modeled on Olympus internal/git/worktree.go Remove().
|
||||
func removeWorktree(repoRoot, worktreePath, runID string) error {
|
||||
return cliRPI.RemoveWorktree(repoRoot, worktreePath, runID, worktreeTimeout)
|
||||
|
||||
@@ -45,7 +45,7 @@ func TestMergeFailurePropagation(t *testing.T) {
|
||||
}
|
||||
|
||||
// mergeWorktree must return a non-nil error — callers must propagate it.
|
||||
mergeErr := mergeWorktree(repo, runID)
|
||||
mergeErr := mergeWorktree(repo, worktreePath, runID)
|
||||
if mergeErr == nil {
|
||||
t.Fatal("expected mergeWorktree to return error for dirty repo; got nil (silent-success violation)")
|
||||
}
|
||||
|
||||
@@ -39,8 +39,8 @@ func runSinglePhase(cwd, spawnCwd string, state *phasedState, startPhase int, p
|
||||
fmt.Printf("[dry-run] Would spawn: %s -p '%s'\n", effectiveRuntimeCommand(state.Opts.RuntimeCommand), prompt)
|
||||
if !opts.NoWorktree && p.Num == startPhase {
|
||||
runID := generateRunID()
|
||||
fmt.Printf("[dry-run] Would create worktree: ../%s-rpi-%s/ (branch: rpi/%s)\n",
|
||||
filepath.Base(cwd), runID, runID)
|
||||
fmt.Printf("[dry-run] Would create worktree: ../%s-rpi-%s/ (detached)\n",
|
||||
filepath.Base(cwd), runID)
|
||||
}
|
||||
logPhaseTransition(logPath, state.RunID, p.Name, "dry-run")
|
||||
return nil
|
||||
|
||||
@@ -122,7 +122,7 @@ func setupWorktreeLifecycle(cwd, originalCwd string, opts phasedEngineOptions, s
|
||||
spawnCwd = worktreePath
|
||||
state.WorktreePath = worktreePath
|
||||
state.RunID = runID
|
||||
fmt.Printf("Worktree created: %s (branch: rpi/%s)\n", worktreePath, runID)
|
||||
fmt.Printf("Worktree created: %s (detached)\n", worktreePath)
|
||||
|
||||
sigCh := make(chan os.Signal, 1)
|
||||
signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM)
|
||||
@@ -147,7 +147,7 @@ func setupWorktreeLifecycle(cwd, originalCwd string, opts phasedEngineOptions, s
|
||||
return nil
|
||||
}
|
||||
|
||||
if mergeErr := mergeWorktree(originalCwd, runID); mergeErr != nil {
|
||||
if mergeErr := mergeWorktree(originalCwd, worktreePath, runID); mergeErr != nil {
|
||||
fmt.Fprintf(os.Stderr, "Merge failed: %v\nWorktree preserved at: %s\n", mergeErr, worktreePath)
|
||||
return fmt.Errorf("worktree merge failed: %w", mergeErr)
|
||||
}
|
||||
|
||||
@@ -122,13 +122,10 @@ func TestCreateWorktree(t *testing.T) {
|
||||
t.Fatalf("createWorktree: %v", err)
|
||||
}
|
||||
defer func() {
|
||||
// Cleanup: remove worktree and branch.
|
||||
// Cleanup: remove worktree.
|
||||
cmd := exec.Command("git", "worktree", "remove", worktreePath, "--force")
|
||||
cmd.Dir = repo
|
||||
_ = cmd.Run()
|
||||
cmd = exec.Command("git", "branch", "-D", "rpi/"+runID)
|
||||
cmd.Dir = repo
|
||||
_ = cmd.Run()
|
||||
}()
|
||||
|
||||
// Verify worktree directory exists.
|
||||
@@ -149,15 +146,15 @@ func TestCreateWorktree(t *testing.T) {
|
||||
t.Fatalf("unexpected basename: %q, expected %q", filepath.Base(worktreePath), expected)
|
||||
}
|
||||
|
||||
// Verify branch exists.
|
||||
// Verify branch was not created for detached worktree mode.
|
||||
cmd := exec.Command("git", "branch", "--list", "rpi/"+runID)
|
||||
cmd.Dir = repo
|
||||
out, err := cmd.Output()
|
||||
if err != nil {
|
||||
t.Fatalf("list branches: %v", err)
|
||||
}
|
||||
if !strings.Contains(string(out), "rpi/"+runID) {
|
||||
t.Fatalf("branch rpi/%s not found", runID)
|
||||
if strings.Contains(string(out), "rpi/"+runID) {
|
||||
t.Fatalf("branch rpi/%s should not exist", runID)
|
||||
}
|
||||
|
||||
// Verify .agents/rpi/ exists in worktree.
|
||||
@@ -176,29 +173,20 @@ func TestCreateWorktree_RetryOnCollision(t *testing.T) {
|
||||
}
|
||||
defer os.Chdir(origDir) //nolint:errcheck
|
||||
|
||||
// Pre-create a branch to simulate collision (unlikely in practice).
|
||||
cmd := exec.Command("git", "branch", "rpi/collision-test")
|
||||
cmd.Dir = repo
|
||||
if out, err := cmd.CombinedOutput(); err != nil {
|
||||
t.Fatalf("pre-create branch: %v\n%s", err, out)
|
||||
}
|
||||
|
||||
// createWorktree should still succeed with a different ID.
|
||||
// createWorktree should still succeed and follow detached naming.
|
||||
worktreePath, runID, err := createWorktree(repo)
|
||||
if err != nil {
|
||||
t.Fatalf("createWorktree should retry on collision: %v", err)
|
||||
t.Fatalf("createWorktree should succeed: %v", err)
|
||||
}
|
||||
defer func() {
|
||||
cmd := exec.Command("git", "worktree", "remove", worktreePath, "--force")
|
||||
cmd.Dir = repo
|
||||
_ = cmd.Run()
|
||||
cmd = exec.Command("git", "branch", "-D", "rpi/"+runID)
|
||||
cmd.Dir = repo
|
||||
_ = cmd.Run()
|
||||
}()
|
||||
|
||||
if runID == "collision-test" {
|
||||
t.Fatal("should have generated a different runID than the pre-existing branch")
|
||||
expected := filepath.Base(repo) + "-rpi-" + runID
|
||||
if filepath.Base(worktreePath) != expected {
|
||||
t.Fatalf("unexpected basename: %q, expected %q", filepath.Base(worktreePath), expected)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -220,9 +208,6 @@ func TestMergeWorktree(t *testing.T) {
|
||||
cmd := exec.Command("git", "worktree", "remove", worktreePath, "--force")
|
||||
cmd.Dir = repo
|
||||
_ = cmd.Run()
|
||||
cmd = exec.Command("git", "branch", "-D", "rpi/"+runID)
|
||||
cmd.Dir = repo
|
||||
_ = cmd.Run()
|
||||
}()
|
||||
|
||||
// Make a commit in the worktree.
|
||||
@@ -242,7 +227,7 @@ func TestMergeWorktree(t *testing.T) {
|
||||
}
|
||||
|
||||
// Merge back.
|
||||
if err := mergeWorktree(repo, runID); err != nil {
|
||||
if err := mergeWorktree(repo, worktreePath, runID); err != nil {
|
||||
t.Fatalf("mergeWorktree: %v", err)
|
||||
}
|
||||
|
||||
@@ -270,9 +255,6 @@ func TestMergeWorktree_Conflict(t *testing.T) {
|
||||
cmd := exec.Command("git", "worktree", "remove", worktreePath, "--force")
|
||||
cmd.Dir = repo
|
||||
_ = cmd.Run()
|
||||
cmd = exec.Command("git", "branch", "-D", "rpi/"+runID)
|
||||
cmd.Dir = repo
|
||||
_ = cmd.Run()
|
||||
}()
|
||||
|
||||
// Create conflicting changes in both repos.
|
||||
@@ -309,7 +291,7 @@ func TestMergeWorktree_Conflict(t *testing.T) {
|
||||
}
|
||||
|
||||
// Merge should fail with conflict info.
|
||||
err = mergeWorktree(repo, runID)
|
||||
err = mergeWorktree(repo, worktreePath, runID)
|
||||
if err == nil {
|
||||
t.Fatal("expected merge conflict error")
|
||||
}
|
||||
@@ -355,9 +337,6 @@ func TestMergeWorktree_DirtyRepo(t *testing.T) {
|
||||
cmd := exec.Command("git", "worktree", "remove", worktreePath, "--force")
|
||||
cmd.Dir = repo
|
||||
_ = cmd.Run()
|
||||
cmd = exec.Command("git", "branch", "-D", "rpi/"+runID)
|
||||
cmd.Dir = repo
|
||||
_ = cmd.Run()
|
||||
}()
|
||||
|
||||
// Make uncommitted changes in original repo.
|
||||
@@ -370,7 +349,7 @@ func TestMergeWorktree_DirtyRepo(t *testing.T) {
|
||||
t.Fatalf("git add: %v\n%s", err, out)
|
||||
}
|
||||
|
||||
err = mergeWorktree(repo, runID)
|
||||
err = mergeWorktree(repo, worktreePath, runID)
|
||||
if err == nil {
|
||||
t.Fatal("expected error for dirty repo")
|
||||
}
|
||||
@@ -408,13 +387,7 @@ func TestRemoveWorktree(t *testing.T) {
|
||||
t.Fatalf("worktree dir should be removed, got: %v", err)
|
||||
}
|
||||
|
||||
// Verify branch gone.
|
||||
cmd := exec.Command("git", "branch", "--list", "rpi/"+runID)
|
||||
cmd.Dir = repo
|
||||
out, _ := cmd.Output()
|
||||
if strings.Contains(string(out), "rpi/"+runID) {
|
||||
t.Fatalf("branch rpi/%s should be deleted", runID)
|
||||
}
|
||||
// No branch assertion in detached mode; branch cleanup is best-effort.
|
||||
}
|
||||
|
||||
func TestRemoveWorktree_PathValidation(t *testing.T) {
|
||||
|
||||
@@ -1247,6 +1247,7 @@ ao rpi cleanup [flags]
|
||||
--all Clean up all stale runs
|
||||
--dry-run Show what would be done without making changes
|
||||
-h, --help help for cleanup
|
||||
--prune-branches Delete legacy RPI branches (rpi/*, codex/auto-rpi-*)
|
||||
--prune-worktrees Run 'git worktree prune' after cleanup
|
||||
--run-id string Clean up a specific run by ID
|
||||
--stale-after duration Only clean runs older than this age (0 disables age filtering)
|
||||
|
||||
@@ -130,25 +130,39 @@ func GetRepoRoot(dir string, timeout time.Duration) (string, error) {
|
||||
}
|
||||
|
||||
// CreateWorktree creates a sibling git worktree for isolated RPI execution.
|
||||
// Worktree checkouts are detached (no new branch created).
|
||||
func CreateWorktree(cwd string, timeout time.Duration, verbosef func(string, ...interface{})) (worktreePath, runID string, err error) {
|
||||
repoRoot, err := GetRepoRoot(cwd, timeout)
|
||||
if err != nil {
|
||||
return "", "", err
|
||||
}
|
||||
|
||||
currentBranch, err := GetCurrentBranch(repoRoot, timeout)
|
||||
if err != nil {
|
||||
return "", "", err
|
||||
if branch, err := GetCurrentBranch(repoRoot, timeout); err == nil {
|
||||
if verbosef != nil {
|
||||
verbosef("Creating detached worktree from current branch=%s\n", branch)
|
||||
}
|
||||
}
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), timeout)
|
||||
cmdHead := exec.CommandContext(ctx, "git", "rev-parse", "HEAD")
|
||||
cmdHead.Dir = repoRoot
|
||||
headOut, headErr := cmdHead.CombinedOutput()
|
||||
cancel()
|
||||
if headErr != nil {
|
||||
return "", "", fmt.Errorf("git rev-parse HEAD: %w (output: %s)", headErr, strings.TrimSpace(string(headOut)))
|
||||
}
|
||||
currentCommit := strings.TrimSpace(string(headOut))
|
||||
if currentCommit == "" {
|
||||
return "", "", fmt.Errorf("unable to resolve HEAD commit for detached worktree creation")
|
||||
}
|
||||
|
||||
for attempt := 0; attempt < 3; attempt++ {
|
||||
runID = GenerateRunID()
|
||||
repoBasename := filepath.Base(repoRoot)
|
||||
worktreePath = filepath.Join(filepath.Dir(repoRoot), repoBasename+"-rpi-"+runID)
|
||||
branchName := "rpi/" + runID
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), timeout)
|
||||
cmd := exec.CommandContext(ctx, "git", "worktree", "add", "-b", branchName, worktreePath, currentBranch)
|
||||
ctx, cancel = context.WithTimeout(context.Background(), timeout)
|
||||
cmd := exec.CommandContext(ctx, "git", "worktree", "add", worktreePath, currentCommit)
|
||||
cmd.Dir = repoRoot
|
||||
output, cmdErr := cmd.CombinedOutput()
|
||||
cancel()
|
||||
@@ -164,7 +178,7 @@ func CreateWorktree(cwd string, timeout time.Duration, verbosef func(string, ...
|
||||
|
||||
if strings.Contains(string(output), "already exists") {
|
||||
if verbosef != nil {
|
||||
verbosef("Worktree branch collision on %s, retrying (%d/3)\n", branchName, attempt+1)
|
||||
verbosef("Worktree path collision on %s, retrying (%d/3)\n", worktreePath, attempt+1)
|
||||
}
|
||||
continue
|
||||
}
|
||||
@@ -174,11 +188,11 @@ func CreateWorktree(cwd string, timeout time.Duration, verbosef func(string, ...
|
||||
}
|
||||
return "", "", fmt.Errorf("git worktree add failed: %w (output: %s)", cmdErr, string(output))
|
||||
}
|
||||
return "", "", fmt.Errorf("failed to create unique worktree branch after 3 attempts")
|
||||
return "", "", fmt.Errorf("failed to create unique worktree path after 3 attempts")
|
||||
}
|
||||
|
||||
// MergeWorktree merges the RPI worktree branch back into the original branch.
|
||||
func MergeWorktree(repoRoot, runID string, timeout time.Duration, verbosef func(string, ...interface{})) error {
|
||||
// MergeWorktree merges the RPI worktree commit back into the original branch.
|
||||
func MergeWorktree(repoRoot, worktreePath, runID string, timeout time.Duration, verbosef func(string, ...interface{})) error {
|
||||
var dirtyErr error
|
||||
for attempt := 0; attempt < 5; attempt++ {
|
||||
ctx, cancel := context.WithTimeout(context.Background(), timeout)
|
||||
@@ -201,12 +215,38 @@ func MergeWorktree(repoRoot, runID string, timeout time.Duration, verbosef func(
|
||||
return fmt.Errorf("original repo has uncommitted changes after 5 retries: commit or stash before merge")
|
||||
}
|
||||
|
||||
if strings.TrimSpace(worktreePath) == "" {
|
||||
if strings.TrimSpace(runID) == "" {
|
||||
return fmt.Errorf("merge source unavailable: missing worktree path and run ID")
|
||||
}
|
||||
worktreePath = filepath.Join(filepath.Dir(repoRoot), filepath.Base(repoRoot)+"-rpi-"+runID)
|
||||
}
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), timeout)
|
||||
revCmd := exec.CommandContext(ctx, "git", "rev-parse", "HEAD")
|
||||
revCmd.Dir = worktreePath
|
||||
revOut, revErr := revCmd.CombinedOutput()
|
||||
cancel()
|
||||
if revErr != nil {
|
||||
return fmt.Errorf("resolve worktree merge source: %w (output: %s)", revErr, strings.TrimSpace(string(revOut)))
|
||||
}
|
||||
mergeSource := strings.TrimSpace(string(revOut))
|
||||
if mergeSource == "" {
|
||||
return fmt.Errorf("worktree merge source commit is empty")
|
||||
}
|
||||
shortMergeSource := mergeSource
|
||||
if len(shortMergeSource) > 12 {
|
||||
shortMergeSource = shortMergeSource[:12]
|
||||
}
|
||||
|
||||
ctx, cancel = context.WithTimeout(context.Background(), timeout)
|
||||
defer cancel()
|
||||
|
||||
branchName := "rpi/" + runID
|
||||
mergeMsg := fmt.Sprintf("Merge %s (ao rpi phased worktree)", branchName)
|
||||
mergeCmd := exec.CommandContext(ctx, "git", "merge", "--no-ff", "-m", mergeMsg, branchName)
|
||||
mergeMsg := "Merge ao rpi worktree (detached checkout)"
|
||||
if strings.TrimSpace(runID) != "" {
|
||||
mergeMsg = fmt.Sprintf("Merge %s (ao rpi worktree)", runID)
|
||||
}
|
||||
mergeCmd := exec.CommandContext(ctx, "git", "merge", "--no-ff", "-m", mergeMsg, mergeSource)
|
||||
mergeCmd.Dir = repoRoot
|
||||
if err := mergeCmd.Run(); err != nil {
|
||||
if ctx.Err() == context.DeadlineExceeded {
|
||||
@@ -221,14 +261,23 @@ func MergeWorktree(repoRoot, runID string, timeout time.Duration, verbosef func(
|
||||
files := strings.TrimSpace(string(conflictOut))
|
||||
if files != "" {
|
||||
return fmt.Errorf("merge conflict in %s.\nConflicting files:\n%s\nResolve manually: cd %s && git merge %s",
|
||||
branchName, files, repoRoot, branchName)
|
||||
shortMergeSource, files, repoRoot, mergeSource)
|
||||
}
|
||||
return fmt.Errorf("git merge failed: %w", err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// RemoveWorktree removes a worktree directory and its branch.
|
||||
func rpiRunIDFromWorktree(repoRoot, worktreePath string) string {
|
||||
base := filepath.Base(worktreePath)
|
||||
prefix := filepath.Base(repoRoot) + "-rpi-"
|
||||
if !strings.HasPrefix(base, prefix) {
|
||||
return ""
|
||||
}
|
||||
return strings.TrimPrefix(base, prefix)
|
||||
}
|
||||
|
||||
// RemoveWorktree removes a worktree directory and optionally a legacy branch reference.
|
||||
func RemoveWorktree(repoRoot, worktreePath, runID string, timeout time.Duration) error {
|
||||
absPath, err := filepath.EvalSymlinks(worktreePath)
|
||||
if err != nil {
|
||||
@@ -241,6 +290,12 @@ func RemoveWorktree(repoRoot, worktreePath, runID string, timeout time.Duration)
|
||||
if err != nil {
|
||||
resolvedRoot = repoRoot
|
||||
}
|
||||
if strings.TrimSpace(runID) == "" {
|
||||
runID = rpiRunIDFromWorktree(resolvedRoot, absPath)
|
||||
if strings.TrimSpace(runID) == "" {
|
||||
return fmt.Errorf("invalid run id for removeWorktree path %s", absPath)
|
||||
}
|
||||
}
|
||||
expectedBasename := filepath.Base(resolvedRoot) + "-rpi-" + runID
|
||||
expectedPath := filepath.Join(filepath.Dir(resolvedRoot), expectedBasename)
|
||||
if absPath != expectedPath {
|
||||
@@ -256,10 +311,13 @@ func RemoveWorktree(repoRoot, worktreePath, runID string, timeout time.Duration)
|
||||
_ = os.RemoveAll(absPath) //nolint:errcheck
|
||||
}
|
||||
|
||||
branchName := "rpi/" + runID
|
||||
branchCmd := exec.CommandContext(ctx, "git", "branch", "-D", branchName)
|
||||
branchCmd.Dir = repoRoot
|
||||
_ = branchCmd.Run() //nolint:errcheck
|
||||
// Best-effort cleanup for legacy branch-based runs.
|
||||
if strings.TrimSpace(runID) != "" {
|
||||
branchName := "rpi/" + runID
|
||||
branchCmd := exec.CommandContext(ctx, "git", "branch", "-D", branchName)
|
||||
branchCmd.Dir = repoRoot
|
||||
_ = branchCmd.Run() //nolint:errcheck
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -81,14 +81,7 @@ fi
|
||||
|
||||
current_branch="$(git rev-parse --abbrev-ref HEAD)"
|
||||
if [[ "$current_branch" == "HEAD" ]]; then
|
||||
ts="$(date -u +%Y%m%d%H%M%S)"
|
||||
new_branch="codex/auto-rpi-$ts"
|
||||
if git switch -c "$new_branch" >/dev/null 2>&1; then
|
||||
:
|
||||
else
|
||||
git checkout -b "$new_branch" >/dev/null
|
||||
fi
|
||||
echo "Detached HEAD detected. Created branch: $new_branch"
|
||||
echo "Detached HEAD detected. Running detached-safe, no branch created." >&2
|
||||
fi
|
||||
|
||||
stale_after="${AO_RPI_AUTO_CLEAN_AFTER:-24h}"
|
||||
|
||||
@@ -249,12 +249,18 @@ Read `references/error-handling.md` for failure semantics and retries.
|
||||
|
||||
| Problem | Cause | Solution |
|
||||
|---------|-------|----------|
|
||||
| Supervisor spiraled branch count | Detached HEAD healing or legacy `codex/auto-rpi-*` naming created detached branches | Keep `--detached-heal` off for supervisor mode (default), prefer detached worktree execution, then run cleanup: `ao rpi cleanup --all --prune-worktrees --prune-branches --dry-run` to preview, then rerun without `--dry-run`. |
|
||||
| Discovery retries hit max attempts | Plan has unresolved risks | Review pre-mortem findings, re-run `/rpi --from=discovery` |
|
||||
| Implementation retries hit max attempts | Epic has blockers or unresolved dependencies | Inspect `bd show <epic-id>`, fix blockers, re-run `/rpi --from=implementation` |
|
||||
| Validation retries hit max attempts | Vibe found critical defects repeatedly | Apply findings, re-run `/rpi --from=validation` |
|
||||
| Missing epic ID at implementation start | Discovery did not produce a parseable epic | Verify latest open epic with `bd list --type epic --status open` |
|
||||
| Large-repo context pressure | Too much context in one window | Use `references/context-windowing.md` and summarize phase outputs aggressively |
|
||||
|
||||
### Emergency control
|
||||
|
||||
- Cancel in-flight RPI work immediately: `ao rpi cancel --all` (or `--run-id <id>` for one run).
|
||||
- Remove stale worktrees and legacy branches: `ao rpi cleanup --all --prune-worktrees --prune-branches`.
|
||||
|
||||
## See Also
|
||||
|
||||
- `skills/research/SKILL.md` — discovery exploration
|
||||
|
||||
Reference in New Issue
Block a user