e2e: own the shared fakeagent build directory and clean it up (plan 056)

internal/e2e/fakeagent_cleanup_test.go

@@ -0,0 +1,149 @@
+package e2e
+
+import (
+	"errors"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// TestFakeAgentBuildFailureRemovesOwnedDir: a failed build removes the
+// directory it created — including a partial binary — and nothing else.
+func TestFakeAgentBuildFailureRemovesOwnedDir(t *testing.T) {
+	dest := t.TempDir()
+	var ownedDir string
+	_, err := buildFakeAgentBinary(dest, func(outPath string) error {
+		ownedDir = filepath.Dir(outPath)
+		// Simulate a partial build artifact before failing.
+		if werr := os.WriteFile(outPath, []byte("partial"), 0o755); werr != nil {
+			t.Fatalf("write partial: %v", werr)
+		}
+		return errors.New("build exploded")
+	})
+	if err == nil {
+		t.Fatal("expected build error")
+	}
+	if ownedDir == "" || !strings.HasPrefix(filepath.Base(ownedDir), fakeAgentBuildDirPrefix) {
+		t.Fatalf("helper did not allocate an owned dir under %q: %q", dest, ownedDir)
+	}
+	if _, serr := os.Stat(ownedDir); !os.IsNotExist(serr) {
+		t.Fatalf("failed build left owned dir behind: %s", ownedDir)
+	}
+	// The destination root itself must survive.
+	if entries, rerr := os.ReadDir(dest); rerr != nil || len(entries) != 0 {
+		t.Fatalf("destination root disturbed: entries=%v err=%v", entries, rerr)
+	}
+}
+
+// TestFakeAgentBuildSuccessKeepsOwnedDir: a successful build keeps its
+// directory for the process-level cleanup to remove later.
+func TestFakeAgentBuildSuccessKeepsOwnedDir(t *testing.T) {
+	dest := t.TempDir()
+	out, err := buildFakeAgentBinary(dest, func(outPath string) error {
+		return os.WriteFile(outPath, []byte("bin"), 0o755)
+	})
+	if err != nil {
+		t.Fatalf("build: %v", err)
+	}
+	dir := filepath.Dir(out)
+	if _, serr := os.Stat(out); serr != nil {
+		t.Fatalf("successful build lost its binary: %v", serr)
+	}
+	// Ownership transferred: cleanup removes it, and a second call is a no-op.
+	if err := cleanupOwnedFakeAgentDir(out); err != nil {
+		t.Fatalf("cleanup: %v", err)
+	}
+	if _, serr := os.Stat(dir); !os.IsNotExist(serr) {
+		t.Fatalf("cleanup left the owned dir: %s", dir)
+	}
+	if err := cleanupOwnedFakeAgentDir(out); err != nil {
+		t.Fatalf("cleanup not idempotent: %v", err)
+	}
+}
+
+// TestCleanupFakeAgentEmptyAndRefusal: empty path is a no-op; a directory
+// outside the owned naming convention is refused and left intact.
+func TestCleanupFakeAgentEmptyAndRefusal(t *testing.T) {
+	if err := cleanupOwnedFakeAgentDir(""); err != nil {
+		t.Fatalf("empty path should be a no-op, got %v", err)
+	}
+
+	unrelated := t.TempDir() // not an amux-fakeagent-* name
+	sentinel := filepath.Join(unrelated, "sentinel.txt")
+	if err := os.WriteFile(sentinel, []byte("keep"), 0o644); err != nil {
+		t.Fatalf("write sentinel: %v", err)
+	}
+	err := cleanupOwnedFakeAgentDir(filepath.Join(unrelated, "fakeagent"))
+	if err == nil {
+		t.Fatal("expected refusal for unowned directory")
+	}
+	if _, serr := os.Stat(sentinel); serr != nil {
+		t.Fatalf("refusal deleted the unrelated dir anyway: %v", serr)
+	}
+}
+
+// TestFakeAgentCleanupHelper runs only inside the subprocess spawned by
+// TestFakeAgentCleanupAfterTestMain. It builds a fake binary through the real
+// owned helper, records the directory for the parent, and relies on the
+// process's TestMain to remove it after m.Run — proving process-lifetime
+// cleanup, not per-test cleanup.
+func TestFakeAgentCleanupHelper(t *testing.T) {
+	if os.Getenv("AMUX_E2E_FAKEAGENT_CLEANUP_HELPER") != "1" {
+		t.Skip("helper process only")
+	}
+	outFile := os.Getenv("AMUX_E2E_FAKEAGENT_DIR_OUT")
+	if outFile == "" {
+		t.Fatal("AMUX_E2E_FAKEAGENT_DIR_OUT unset")
+	}
+	out, err := buildFakeAgentBinary(os.TempDir(), func(outPath string) error {
+		return os.WriteFile(outPath, []byte("bin"), 0o755)
+	})
+	if err != nil {
+		t.Fatalf("build: %v", err)
+	}
+	fakeAgentPath = out // this process's owned binary, as the real once-build sets it
+	dir := filepath.Dir(out)
+	if _, err := os.Stat(dir); err != nil {
+		t.Fatalf("owned dir missing before exit: %v", err)
+	}
+	if err := os.WriteFile(outFile, []byte(dir), 0o644); err != nil {
+		t.Fatalf("record dir: %v", err)
+	}
+}
+
+// TestFakeAgentCleanupAfterTestMain runs the test binary as a subprocess
+// executing only the helper test, with a private TMPDIR, and asserts the
+// owned fakeagent directory is gone after normal TestMain exit.
+func TestFakeAgentCleanupAfterTestMain(t *testing.T) {
+	helperTmp := t.TempDir()
+	outFile := filepath.Join(t.TempDir(), "dir.txt")
+	self, err := os.Executable()
+	if err != nil {
+		t.Fatalf("Executable: %v", err)
+	}
+	cmd := exec.Command(self, "-test.run=^TestFakeAgentCleanupHelper$", "-test.count=1")
+	cmd.Env = append(os.Environ(),
+		"AMUX_E2E_FAKEAGENT_CLEANUP_HELPER=1",
+		"AMUX_E2E_FAKEAGENT_DIR_OUT="+outFile,
+		"TMPDIR="+helperTmp,
+	)
+	if combined, err := cmd.CombinedOutput(); err != nil {
+		t.Fatalf("helper subprocess failed: %v\n%s", err, combined)
+	}
+	dirBytes, err := os.ReadFile(outFile)
+	if err != nil {
+		t.Fatalf("helper did not record its dir: %v", err)
+	}
+	dir := strings.TrimSpace(string(dirBytes))
+	if !strings.HasPrefix(filepath.Base(dir), fakeAgentBuildDirPrefix) {
+		t.Fatalf("helper recorded unexpected dir %q", dir)
+	}
+	if filepath.Dir(dir) != helperTmp {
+		t.Fatalf("helper built outside its private TMPDIR: %q", dir)
+	}
+	if _, serr := os.Stat(dir); !os.IsNotExist(serr) {
+		t.Fatalf("owned fakeagent dir survived TestMain cleanup: %s", dir)
+	}
+}

internal/e2e/fakeagent_test.go

@@ -6,6 +6,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"strings"
 	"sync"
 	"testing"
 	"time"
@@ -19,6 +20,50 @@ var (
 	fakeAgentErr  error
 )
 
+// fakeAgentBuildDirPrefix marks the directories this test process creates and
+// owns for the shared fakeagent binary; the cleanup guard keys on it.
+const fakeAgentBuildDirPrefix = "amux-fakeagent-"
+
+// buildFakeAgentBinary owns the fakeagent build directory lifecycle: it creates
+// an amux-fakeagent-* directory under destRoot, runs the injected build step,
+// and removes the directory immediately if the build fails. On success the
+// directory is kept — the binary is process-shared, so process-level cleanup
+// (cleanupBuiltFakeAgent from TestMain) removes it after every test finishes.
+// It never creates or deletes caller-owned directories.
+func buildFakeAgentBinary(destRoot string, runBuild func(outPath string) error) (string, error) {
+	dir, err := os.MkdirTemp(destRoot, fakeAgentBuildDirPrefix+"*")
+	if err != nil {
+		return "", err
+	}
+	out := filepath.Join(dir, "fakeagent")
+	if err := runBuild(out); err != nil {
+		_ = os.RemoveAll(dir)
+		return "", err
+	}
+	return out, nil
+}
+
+// cleanupBuiltFakeAgent removes the shared fakeagent build directory after all
+// tests in the process finish. It is a no-op when no binary was built and
+// refuses paths outside the owned naming convention.
+func cleanupBuiltFakeAgent() error {
+	return cleanupOwnedFakeAgentDir(fakeAgentPath)
+}
+
+// cleanupOwnedFakeAgentDir removes the directory containing binPath after
+// checking it carries this process's owned naming convention. An empty path is
+// a no-op; an unrelated directory is refused, never removed.
+func cleanupOwnedFakeAgentDir(binPath string) error {
+	if binPath == "" {
+		return nil
+	}
+	dir := filepath.Dir(binPath)
+	if !strings.HasPrefix(filepath.Base(dir), fakeAgentBuildDirPrefix) {
+		return fmt.Errorf("refusing to remove unexpected fakeagent directory %q", dir)
+	}
+	return os.RemoveAll(dir)
+}
+
 // buildFakeAgent compiles internal/e2e/fakeagent once per test binary and returns
 // the resulting executable path. Reused by the full close-the-loop E2E test.
 func buildFakeAgent(t *testing.T) string {
@@ -29,18 +74,18 @@ func buildFakeAgent(t *testing.T) string {
 			fakeAgentErr = err
 			return
 		}
-		dir, err := os.MkdirTemp("", "amux-fakeagent-*")
+		out, err := buildFakeAgentBinary(os.TempDir(), func(outPath string) error {
+			cmd := exec.Command("go", "build", "-o", outPath, "./internal/e2e/fakeagent")
+			cmd.Dir = root
+			if combined, err := cmd.CombinedOutput(); err != nil {
+				return fmt.Errorf("build fakeagent: %w\n%s", err, combined)
+			}
+			return nil
+		})
 		if err != nil {
 			fakeAgentErr = err
 			return
 		}
-		out := filepath.Join(dir, "fakeagent")
-		cmd := exec.Command("go", "build", "-o", out, "./internal/e2e/fakeagent")
-		cmd.Dir = root
-		if combined, err := cmd.CombinedOutput(); err != nil {
-			fakeAgentErr = fmt.Errorf("build fakeagent: %w\n%s", err, combined)
-			return
-		}
 		fakeAgentPath = out
 	})
 	if fakeAgentErr != nil {

internal/e2e/main_test.go

@@ -20,5 +20,8 @@ func TestMain(m *testing.M) {
 	if err := cleanupBuiltAmuxBinary(); err != nil {
 		fmt.Fprintf(os.Stderr, "e2e binary cleanup: %v\n", err)
 	}
+	if err := cleanupBuiltFakeAgent(); err != nil {
+		fmt.Fprintf(os.Stderr, "e2e fakeagent cleanup: %v\n", err)
+	}
 	os.Exit(code)
 }

plans/README.md

@@ -40,7 +40,7 @@ clean baseline. All rows below mean plans ready for an executor, not fixes shipp
 | [053](053-replace-executable-without-path-gap.md) | Replace executable without a pathname gap | P2 | M | — | DONE |
 | [054](054-scroll-wrapped-diff-rows.md) | Scroll wrapped diff display rows | P2 | M | — | DONE |
 | [055](055-load-project-tree-asynchronously.md) | Load the project tree asynchronously | P2 | M | — | DONE |
-| [056](056-clean-up-fakeagent-build-directory.md) | Clean up the shared fakeagent fixture | P3 | S | — | TODO |
+| [056](056-clean-up-fakeagent-build-directory.md) | Clean up the shared fakeagent fixture | P3 | S | — | DONE |
 | [057](057-spike-script-output-search.md) | Spike: search retained script output | P3 | S | 038 | TODO |
 | [058](058-spike-workspace-recovery-diagnostics.md) | Spike: workspace recovery diagnostics | P3 | S | 039, 041, 048 | TODO |
 | [060](060-prefix-palette-after-restore-flake.md) | Prefix palette intermittently fails after restore | P1 | M | — | DONE — identity-drift mutation guard + stale-duplicate restore skip |