From 8aedbb80a1cbb8c2afac59ab467b6570cad10515 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Thu, 29 Oct 2026 14:16:13 -0400 Subject: [PATCH 1/6] tests: fixtures get the physical temporary folder on macOS macOS sets TMPDIR to /var/folders/.../T/, reached through the /var -> /private/var link. 101 tests in nine packages compared a raw t.TempDir() with a path the product resolved, or met the product's deliberate refusal of a binding that traverses a link, and failed on every Mac run; all of them pass when the test binary starts with a link-free TMPDIR. internal/testutil.CanonicalizeTempRoots resolves a set TMPDIR and GOTMPDIR and rewrites them only when the physical spelling differs, so it writes nothing on Linux or wherever the folder is already physical. TestMain calls it first in workspaceidentity, taskcopy, syncsetup, cellstore and handoff (new), and in session, tui3, cmd/codeaf and run (after any child-process dispatch). No product code or Makefile changes. Co-Authored-By: Claude Opus 5.5 --- cmd/codeaf/logs_test.go | 5 + .../unreleased/0-mac-test-temp-roots.md | 7 + internal/cellstore/testmain_test.go | 18 +++ internal/handoff/testmain_test.go | 18 +++ internal/run/testmain_test.go | 7 + internal/session/hermetic_test.go | 5 + internal/syncsetup/testmain_test.go | 18 +++ internal/taskcopy/testmain_test.go | 18 +++ internal/testutil/temp.go | 34 ++++ internal/testutil/temp_test.go | 146 ++++++++++++++++++ internal/tui3/tui3_test.go | 5 + internal/workspaceidentity/testmain_test.go | 18 +++ 12 files changed, 299 insertions(+) create mode 100644 docs/changes/unreleased/0-mac-test-temp-roots.md create mode 100644 internal/cellstore/testmain_test.go create mode 100644 internal/handoff/testmain_test.go create mode 100644 internal/syncsetup/testmain_test.go create mode 100644 internal/taskcopy/testmain_test.go create mode 100644 internal/testutil/temp.go create mode 100644 internal/testutil/temp_test.go create mode 100644 internal/workspaceidentity/testmain_test.go diff --git a/cmd/codeaf/logs_test.go b/cmd/codeaf/logs_test.go index 121c8f7a7d..c2ddc55605 100644 --- a/cmd/codeaf/logs_test.go +++ b/cmd/codeaf/logs_test.go @@ -17,6 +17,7 @@ import ( "github.com/Agent-Field/codeaf/internal/calllog" "github.com/Agent-Field/codeaf/internal/catalog" "github.com/Agent-Field/codeaf/internal/config" + "github.com/Agent-Field/codeaf/internal/testutil" ) // TestMain switches the model-call log OFF for this package and gives the @@ -41,6 +42,10 @@ func TestMain(m *testing.M) { if code, child := runAsCarriedChild(); child { os.Exit(code) } + if err := testutil.CanonicalizeTempRoots(); err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } // C27.7 keeps unrelated CLI fixtures on host execution. Cell wiring tests // declare availability, and engine-backed tests require the real engine; // the stamped binary smoke independently proves the production default. diff --git a/docs/changes/unreleased/0-mac-test-temp-roots.md b/docs/changes/unreleased/0-mac-test-temp-roots.md new file mode 100644 index 0000000000..023e936c42 --- /dev/null +++ b/docs/changes/unreleased/0-mac-test-temp-roots.md @@ -0,0 +1,7 @@ +--- +kind: internal +title: Test fixtures use physical temporary folders on macOS +pr: 0 +surface: [build] +invalidates: [] +--- diff --git a/internal/cellstore/testmain_test.go b/internal/cellstore/testmain_test.go new file mode 100644 index 0000000000..3d51c67e55 --- /dev/null +++ b/internal/cellstore/testmain_test.go @@ -0,0 +1,18 @@ +package cellstore + +import ( + "fmt" + "os" + "testing" + + "github.com/Agent-Field/codeaf/internal/testutil" +) + +// TestMain gives fixtures physical paths so workspace bindings do not traverse host aliases. +func TestMain(m *testing.M) { + if err := testutil.CanonicalizeTempRoots(); err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + os.Exit(m.Run()) +} diff --git a/internal/handoff/testmain_test.go b/internal/handoff/testmain_test.go new file mode 100644 index 0000000000..ac4b5e35ca --- /dev/null +++ b/internal/handoff/testmain_test.go @@ -0,0 +1,18 @@ +package handoff + +import ( + "fmt" + "os" + "testing" + + "github.com/Agent-Field/codeaf/internal/testutil" +) + +// TestMain gives fixtures physical paths so workspace bindings do not traverse host aliases. +func TestMain(m *testing.M) { + if err := testutil.CanonicalizeTempRoots(); err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + os.Exit(m.Run()) +} diff --git a/internal/run/testmain_test.go b/internal/run/testmain_test.go index 3fdefd6ba4..6e211c2ad3 100644 --- a/internal/run/testmain_test.go +++ b/internal/run/testmain_test.go @@ -1,8 +1,11 @@ package run_test import ( + "fmt" "os" "testing" + + "github.com/Agent-Field/codeaf/internal/testutil" ) // TestMain keeps subprocess workers made by this suite out of the plan that @@ -16,6 +19,10 @@ func TestMain(m *testing.M) { if code, child := runAsDelegateChild(); child { os.Exit(code) } + if err := testutil.CanonicalizeTempRoots(); err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } _ = os.Unsetenv("PLANDB_DB") _ = os.Unsetenv("PLANDB_RUN") os.Exit(m.Run()) diff --git a/internal/session/hermetic_test.go b/internal/session/hermetic_test.go index 8a0001db8d..1e5aeedbc6 100644 --- a/internal/session/hermetic_test.go +++ b/internal/session/hermetic_test.go @@ -43,6 +43,7 @@ import ( "testing" "github.com/Agent-Field/codeaf/internal/home" + "github.com/Agent-Field/codeaf/internal/testutil" ) func TestMain(m *testing.M) { os.Exit(runTests(m)) } @@ -52,6 +53,10 @@ func TestMain(m *testing.M) { os.Exit(runTests(m)) } // call, and a package that leaks a directory per run to stop leaking files per // run has not fixed anything. func runTests(m *testing.M) int { + if err := testutil.CanonicalizeTempRoots(); err != nil { + fmt.Fprintf(os.Stderr, "session tests: %v\n", err) + return 1 + } // The guard watches wherever this machine actually keeps its state: the real // home, or the CODEAF_HOME a developer had already pointed somewhere else. // It asks [home.InheritedDir] rather than [home.Dir] because Dir hands a diff --git a/internal/syncsetup/testmain_test.go b/internal/syncsetup/testmain_test.go new file mode 100644 index 0000000000..575743b702 --- /dev/null +++ b/internal/syncsetup/testmain_test.go @@ -0,0 +1,18 @@ +package syncsetup + +import ( + "fmt" + "os" + "testing" + + "github.com/Agent-Field/codeaf/internal/testutil" +) + +// TestMain gives fixtures physical paths so workspace bindings do not traverse host aliases. +func TestMain(m *testing.M) { + if err := testutil.CanonicalizeTempRoots(); err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + os.Exit(m.Run()) +} diff --git a/internal/taskcopy/testmain_test.go b/internal/taskcopy/testmain_test.go new file mode 100644 index 0000000000..a35dd0a77f --- /dev/null +++ b/internal/taskcopy/testmain_test.go @@ -0,0 +1,18 @@ +package taskcopy + +import ( + "fmt" + "os" + "testing" + + "github.com/Agent-Field/codeaf/internal/testutil" +) + +// TestMain gives fixtures physical paths so workspace bindings do not traverse host aliases. +func TestMain(m *testing.M) { + if err := testutil.CanonicalizeTempRoots(); err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + os.Exit(m.Run()) +} diff --git a/internal/testutil/temp.go b/internal/testutil/temp.go new file mode 100644 index 0000000000..2f48be7ec2 --- /dev/null +++ b/internal/testutil/temp.go @@ -0,0 +1,34 @@ +// Package testutil keeps fixtures independent of the host's path aliases. +package testutil + +import ( + "fmt" + "os" + "path/filepath" +) + +// CanonicalizeTempRoots makes temporary fixtures agree with canonical workspace paths. +// Empty roots retain Go's defaults, and physical roots retain their original spelling. +// This belongs before suite setup because testing caches temporary parents per test. +func CanonicalizeTempRoots() error { + for _, name := range []string{"TMPDIR", "GOTMPDIR"} { + raw := os.Getenv(name) + if raw == "" { + continue + } + absolute, err := filepath.Abs(raw) + if err != nil { + return fmt.Errorf("cannot resolve %s: %w", name, err) + } + resolved, err := filepath.EvalSymlinks(absolute) + if err != nil { + return fmt.Errorf("cannot resolve %s: %w", name, err) + } + if resolved != filepath.Clean(raw) { + if err := os.Setenv(name, resolved); err != nil { + return fmt.Errorf("cannot set %s: %w", name, err) + } + } + } + return nil +} diff --git a/internal/testutil/temp_test.go b/internal/testutil/temp_test.go new file mode 100644 index 0000000000..b787f6ec53 --- /dev/null +++ b/internal/testutil/temp_test.go @@ -0,0 +1,146 @@ +package testutil_test + +import ( + "errors" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/Agent-Field/codeaf/internal/testutil" +) + +func TestCanonicalizeTempRootsTMPDIRAlias(t *testing.T) { + root := physicalTempDir(t) + alias := filepath.Join(root, "alias") + if err := os.Symlink(root, alias); err != nil { + t.Fatalf("create TMPDIR alias: %v", err) + } + t.Setenv("TMPDIR", alias+string(os.PathSeparator)) + t.Setenv("GOTMPDIR", "") + + for call := 1; call <= 2; call++ { + if err := testutil.CanonicalizeTempRoots(); err != nil { + t.Fatalf("canonicalize temp roots on call %d: %v", call, err) + } + if got := os.Getenv("TMPDIR"); got != root { + t.Fatalf("TMPDIR on call %d = %q, want physical directory %q", call, got, root) + } + } + child, err := os.MkdirTemp("", "x") + if err != nil { + t.Fatalf("create directory under canonical TMPDIR: %v", err) + } + if got := filepath.Dir(child); got != root { + t.Fatalf("temporary directory parent = %q, want physical directory %q", got, root) + } +} + +func TestCanonicalizeTempRootsSeparateAliases(t *testing.T) { + root := physicalTempDir(t) + for _, name := range []string{"TMPDIR", "GOTMPDIR"} { + target := filepath.Join(root, name) + if err := os.Mkdir(target, 0o700); err != nil { + t.Fatalf("create %s target: %v", name, err) + } + alias := target + "-alias" + if err := os.Symlink(target, alias); err != nil { + t.Fatalf("create %s alias: %v", name, err) + } + t.Setenv(name, alias) + } + if err := testutil.CanonicalizeTempRoots(); err != nil { + t.Fatalf("canonicalize separate temp roots: %v", err) + } + for _, name := range []string{"TMPDIR", "GOTMPDIR"} { + want := filepath.Join(root, name) + if got := os.Getenv(name); got != want { + t.Fatalf("%s = %q, want its separate target %q", name, got, want) + } + } +} + +func TestCanonicalizeTempRootsEmptyAndUnset(t *testing.T) { + for _, unset := range []string{"TMPDIR", "GOTMPDIR"} { + t.Run("unset "+unset, func(t *testing.T) { + t.Setenv("TMPDIR", "") + t.Setenv("GOTMPDIR", "") + if err := os.Unsetenv(unset); err != nil { + t.Fatalf("unset %s: %v", unset, err) + } + if err := testutil.CanonicalizeTempRoots(); err != nil { + t.Fatalf("canonicalize empty and unset temp roots: %v", err) + } + for _, name := range []string{"TMPDIR", "GOTMPDIR"} { + value, set := os.LookupEnv(name) + wantSet := name != unset + if value != "" || set != wantSet { + t.Fatalf("LookupEnv(%q) = (%q, %t), want (%q, %t)", name, value, set, "", wantSet) + } + } + }) + } +} + +func TestCanonicalizeTempRootsPhysicalUnchanged(t *testing.T) { + root := physicalTempDir(t) + for _, suffix := range []string{"", string(os.PathSeparator), string(os.PathSeparator) + "."} { + t.Run("suffix="+suffix, func(t *testing.T) { + raw := root + suffix + t.Setenv("TMPDIR", raw) + t.Setenv("GOTMPDIR", raw) + if err := testutil.CanonicalizeTempRoots(); err != nil { + t.Fatalf("canonicalize physical temp roots: %v", err) + } + for _, name := range []string{"TMPDIR", "GOTMPDIR"} { + if got := os.Getenv(name); got != raw { + t.Fatalf("%s = %q, want byte-for-byte original %q", name, got, raw) + } + } + }) + } +} + +func TestCanonicalizeTempRootsMissingTarget(t *testing.T) { + root := physicalTempDir(t) + for _, name := range []string{"TMPDIR", "GOTMPDIR"} { + t.Run(name, func(t *testing.T) { + t.Setenv("TMPDIR", root) + t.Setenv("GOTMPDIR", "") + missing := filepath.Join(root, "missing") + t.Setenv(name, missing) + err := testutil.CanonicalizeTempRoots() + if err == nil { + t.Fatalf("missing %s target %q was accepted", name, missing) + } + if !strings.Contains(err.Error(), name) { + t.Fatalf("missing target error = %q, want variable name %q", err, name) + } + if !errors.Is(err, os.ErrNotExist) { + t.Fatalf("missing target error = %v, want wrapped os.ErrNotExist", err) + } + if got := os.Getenv(name); got != missing { + t.Fatalf("%s after error = %q, want original %q", name, got, missing) + } + }) + } +} + +// physicalTempDir avoids caching a test's temporary parent before its environment changes. +func physicalTempDir(t *testing.T) string { + t.Helper() + root, err := os.MkdirTemp("", "codeaf-testutil-") + if err != nil { + t.Fatalf("create fixture root: %v", err) + } + t.Cleanup(func() { + if err := os.RemoveAll(root); err != nil { + t.Errorf("remove fixture root %q: %v", root, err) + } + }) + physical, err := filepath.EvalSymlinks(root) + if err != nil { + t.Fatalf("resolve fixture root: %v", err) + } + return physical +} diff --git a/internal/tui3/tui3_test.go b/internal/tui3/tui3_test.go index 109c7ed262..b1aea4d2c2 100644 --- a/internal/tui3/tui3_test.go +++ b/internal/tui3/tui3_test.go @@ -28,6 +28,7 @@ import ( "github.com/Agent-Field/codeaf/internal/config" "github.com/Agent-Field/codeaf/internal/home" "github.com/Agent-Field/codeaf/internal/session" + "github.com/Agent-Field/codeaf/internal/testutil" "github.com/Agent-Field/codeaf/internal/tui2/tokens" ) @@ -105,6 +106,10 @@ func waitOut(cmd tea.Cmd) tea.Msg { // temporary root is still removed on the way out — os.Exit runs no deferred // call. func runTests(m *testing.M) int { + if err := testutil.CanonicalizeTempRoots(); err != nil { + fmt.Fprintf(os.Stderr, "tui3 tests: %v\n", err) + return 1 + } root, err := os.MkdirTemp("", "codeaf-tui3-test-home-") if err != nil { fmt.Fprintf(os.Stderr, "tui3 tests: no temporary state root: %v\n", err) diff --git a/internal/workspaceidentity/testmain_test.go b/internal/workspaceidentity/testmain_test.go new file mode 100644 index 0000000000..db39b99201 --- /dev/null +++ b/internal/workspaceidentity/testmain_test.go @@ -0,0 +1,18 @@ +package workspaceidentity_test + +import ( + "fmt" + "os" + "testing" + + "github.com/Agent-Field/codeaf/internal/testutil" +) + +// TestMain gives fixtures physical paths so workspace bindings do not traverse host aliases. +func TestMain(m *testing.M) { + if err := testutil.CanonicalizeTempRoots(); err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + os.Exit(m.Run()) +} From 8991d087b20187ec6fc1636a8e5463f9bbada49f Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Thu, 29 Oct 2026 14:26:46 -0400 Subject: [PATCH 2/6] tests: temporary roots change only on macOS, links first, and tui3's scratch roots follow Review found three gaps. The helper also rewrote a linked, relative or missing root on Linux, so it was not the no-op it claimed; it now returns at once on every system but macOS, and a Linux test pins that. filepath.Abs cleaned a ".." before links were followed, which could name another folder; links are now resolved first. tui3 reads its scratch roots at package initialisation, before TestMain, so they kept the alias while fixtures moved to the physical folder; its TestMain refreshes that root. Co-Authored-By: Claude Opus 5.5 --- internal/testutil/temp.go | 16 +++++++-- internal/testutil/temp_test.go | 66 ++++++++++++++++++++++++++++++++++ internal/tui3/tui3_test.go | 3 ++ 3 files changed, 82 insertions(+), 3 deletions(-) diff --git a/internal/testutil/temp.go b/internal/testutil/temp.go index 2f48be7ec2..baa8975599 100644 --- a/internal/testutil/temp.go +++ b/internal/testutil/temp.go @@ -5,22 +5,32 @@ import ( "fmt" "os" "path/filepath" + "runtime" ) // CanonicalizeTempRoots makes temporary fixtures agree with canonical workspace paths. -// Empty roots retain Go's defaults, and physical roots retain their original spelling. +// macOS hands every process a TMPDIR under /var/folders, which is a link to +// /private/var/folders, so a fixture's t.TempDir() spelling differs from the path +// the product records once it resolves links. Empty roots retain Go's defaults, +// and physical roots retain their original spelling. Other systems keep their +// environment untouched, because their temporary folder is not an alias. // This belongs before suite setup because testing caches temporary parents per test. func CanonicalizeTempRoots() error { + if runtime.GOOS != "darwin" { + return nil + } for _, name := range []string{"TMPDIR", "GOTMPDIR"} { raw := os.Getenv(name) if raw == "" { continue } - absolute, err := filepath.Abs(raw) + // Links are followed before any lexical cleaning, so a ".." after a link + // climbs out of the link's target exactly as the kernel would. + resolved, err := filepath.EvalSymlinks(raw) if err != nil { return fmt.Errorf("cannot resolve %s: %w", name, err) } - resolved, err := filepath.EvalSymlinks(absolute) + resolved, err = filepath.Abs(resolved) if err != nil { return fmt.Errorf("cannot resolve %s: %w", name, err) } diff --git a/internal/testutil/temp_test.go b/internal/testutil/temp_test.go index b787f6ec53..5f286a2eb9 100644 --- a/internal/testutil/temp_test.go +++ b/internal/testutil/temp_test.go @@ -4,13 +4,17 @@ import ( "errors" "os" "path/filepath" + "runtime" "strings" "testing" "github.com/Agent-Field/codeaf/internal/testutil" ) +// On macOS an aliased TMPDIR, even with a trailing separator, becomes the +// physical folder, and later temporary folders are made under it. func TestCanonicalizeTempRootsTMPDIRAlias(t *testing.T) { + requireDarwin(t) root := physicalTempDir(t) alias := filepath.Join(root, "alias") if err := os.Symlink(root, alias); err != nil { @@ -36,7 +40,9 @@ func TestCanonicalizeTempRootsTMPDIRAlias(t *testing.T) { } } +// TMPDIR and GOTMPDIR are resolved separately and keep their own targets. func TestCanonicalizeTempRootsSeparateAliases(t *testing.T) { + requireDarwin(t) root := physicalTempDir(t) for _, name := range []string{"TMPDIR", "GOTMPDIR"} { target := filepath.Join(root, name) @@ -60,6 +66,7 @@ func TestCanonicalizeTempRootsSeparateAliases(t *testing.T) { } } +// Empty and unset roots stay exactly as they were. func TestCanonicalizeTempRootsEmptyAndUnset(t *testing.T) { for _, unset := range []string{"TMPDIR", "GOTMPDIR"} { t.Run("unset "+unset, func(t *testing.T) { @@ -82,6 +89,7 @@ func TestCanonicalizeTempRootsEmptyAndUnset(t *testing.T) { } } +// An already physical root keeps its spelling byte for byte. func TestCanonicalizeTempRootsPhysicalUnchanged(t *testing.T) { root := physicalTempDir(t) for _, suffix := range []string{"", string(os.PathSeparator), string(os.PathSeparator) + "."} { @@ -101,7 +109,9 @@ func TestCanonicalizeTempRootsPhysicalUnchanged(t *testing.T) { } } +// A root that does not exist stops the suite with the variable's name. func TestCanonicalizeTempRootsMissingTarget(t *testing.T) { + requireDarwin(t) root := physicalTempDir(t) for _, name := range []string{"TMPDIR", "GOTMPDIR"} { t.Run(name, func(t *testing.T) { @@ -126,6 +136,62 @@ func TestCanonicalizeTempRootsMissingTarget(t *testing.T) { } } +// A ".." after a link climbs out of the link's target, as the kernel resolves +// it, and never out of the folder that spells the link. +func TestCanonicalizeTempRootsFollowsLinksBeforeDotDot(t *testing.T) { + requireDarwin(t) + root := physicalTempDir(t) + deep := filepath.Join(root, "other", "deep") + for _, dir := range []string{deep, filepath.Join(root, "other", "T"), filepath.Join(root, "case", "T")} { + if err := os.MkdirAll(dir, 0o700); err != nil { + t.Fatalf("create %s: %v", dir, err) + } + } + if err := os.Symlink(deep, filepath.Join(root, "case", "link")); err != nil { + t.Fatalf("create link: %v", err) + } + t.Setenv("TMPDIR", filepath.Join(root, "case", "link")+"/../T") + t.Setenv("GOTMPDIR", "") + if err := testutil.CanonicalizeTempRoots(); err != nil { + t.Fatalf("canonicalize a root with a dot-dot after a link: %v", err) + } + if got, want := os.Getenv("TMPDIR"), filepath.Join(root, "other", "T"); got != want { + t.Fatalf("TMPDIR = %q, want the folder the kernel reaches, %q", got, want) + } +} + +// Elsewhere the environment is never touched, whatever it holds: the +// temporary folder is not an alias there, and a suite must not fail on it. +func TestCanonicalizeTempRootsLeavesOtherSystemsAlone(t *testing.T) { + if runtime.GOOS == "darwin" { + t.Skip("macOS is the system whose temporary folder is an alias") + } + root := physicalTempDir(t) + alias := filepath.Join(root, "alias") + if err := os.Symlink(root, alias); err != nil { + t.Fatalf("create alias: %v", err) + } + for _, raw := range []string{alias + string(os.PathSeparator), "relative/tmp", filepath.Join(root, "missing")} { + t.Setenv("TMPDIR", raw) + t.Setenv("GOTMPDIR", raw) + if err := testutil.CanonicalizeTempRoots(); err != nil { + t.Fatalf("canonicalize %q on %s: %v", raw, runtime.GOOS, err) + } + for _, name := range []string{"TMPDIR", "GOTMPDIR"} { + if got := os.Getenv(name); got != raw { + t.Fatalf("%s on %s = %q, want it untouched %q", name, runtime.GOOS, got, raw) + } + } + } +} + +func requireDarwin(t *testing.T) { + t.Helper() + if runtime.GOOS != "darwin" { + t.Skip("only macOS rewrites its temporary roots") + } +} + // physicalTempDir avoids caching a test's temporary parent before its environment changes. func physicalTempDir(t *testing.T) string { t.Helper() diff --git a/internal/tui3/tui3_test.go b/internal/tui3/tui3_test.go index b1aea4d2c2..c83b0d4ac7 100644 --- a/internal/tui3/tui3_test.go +++ b/internal/tui3/tui3_test.go @@ -110,6 +110,9 @@ func runTests(m *testing.M) int { fmt.Fprintf(os.Stderr, "tui3 tests: %v\n", err) return 1 } + // The scratch roots were read at package initialisation, before the + // temporary folder took its physical spelling; fixtures now live under it. + homeScratchRoots[0] = filepath.Clean(os.TempDir()) root, err := os.MkdirTemp("", "codeaf-tui3-test-home-") if err != nil { fmt.Fprintf(os.Stderr, "tui3 tests: no temporary state root: %v\n", err) From 961651e03fe03c8edd4d360e90997da528229ca1 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Thu, 29 Oct 2026 14:30:46 -0400 Subject: [PATCH 3/6] tests: leave a relative temporary root alone, and spare Windows the alias case Review round 2: filepath.Abs anchored a relative root to the working directory's logical spelling, which can name another folder; a relative root is now left exactly as given (launchd always sets an absolute TMPDIR). The other-systems test no longer needs link privileges on Windows: its alias case runs only where a link can be made. Co-Authored-By: Claude Opus 5.5 --- internal/testutil/temp.go | 10 ++++------ internal/testutil/temp_test.go | 27 ++++++++++++++++++++++++--- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/internal/testutil/temp.go b/internal/testutil/temp.go index baa8975599..9bd5d542f9 100644 --- a/internal/testutil/temp.go +++ b/internal/testutil/temp.go @@ -12,7 +12,7 @@ import ( // macOS hands every process a TMPDIR under /var/folders, which is a link to // /private/var/folders, so a fixture's t.TempDir() spelling differs from the path // the product records once it resolves links. Empty roots retain Go's defaults, -// and physical roots retain their original spelling. Other systems keep their +// relative and physical roots retain their spelling. Other systems keep their // environment untouched, because their temporary folder is not an alias. // This belongs before suite setup because testing caches temporary parents per test. func CanonicalizeTempRoots() error { @@ -21,7 +21,9 @@ func CanonicalizeTempRoots() error { } for _, name := range []string{"TMPDIR", "GOTMPDIR"} { raw := os.Getenv(name) - if raw == "" { + // A relative root depends on the working directory's spelling, so it + // is left exactly as given rather than anchored to a guess. + if raw == "" || !filepath.IsAbs(raw) { continue } // Links are followed before any lexical cleaning, so a ".." after a link @@ -30,10 +32,6 @@ func CanonicalizeTempRoots() error { if err != nil { return fmt.Errorf("cannot resolve %s: %w", name, err) } - resolved, err = filepath.Abs(resolved) - if err != nil { - return fmt.Errorf("cannot resolve %s: %w", name, err) - } if resolved != filepath.Clean(raw) { if err := os.Setenv(name, resolved); err != nil { return fmt.Errorf("cannot set %s: %w", name, err) diff --git a/internal/testutil/temp_test.go b/internal/testutil/temp_test.go index 5f286a2eb9..729d84773d 100644 --- a/internal/testutil/temp_test.go +++ b/internal/testutil/temp_test.go @@ -167,11 +167,16 @@ func TestCanonicalizeTempRootsLeavesOtherSystemsAlone(t *testing.T) { t.Skip("macOS is the system whose temporary folder is an alias") } root := physicalTempDir(t) + roots := []string{"relative/tmp", filepath.Join(root, "missing")} + // Windows may refuse to make links without a privilege; the other roots + // still prove the environment is left alone there. alias := filepath.Join(root, "alias") - if err := os.Symlink(root, alias); err != nil { - t.Fatalf("create alias: %v", err) + if err := os.Symlink(root, alias); err == nil { + roots = append(roots, alias+string(os.PathSeparator)) + } else { + t.Logf("no alias case on %s: %v", runtime.GOOS, err) } - for _, raw := range []string{alias + string(os.PathSeparator), "relative/tmp", filepath.Join(root, "missing")} { + for _, raw := range roots { t.Setenv("TMPDIR", raw) t.Setenv("GOTMPDIR", raw) if err := testutil.CanonicalizeTempRoots(); err != nil { @@ -192,6 +197,22 @@ func requireDarwin(t *testing.T) { } } +// A relative root is left as given on macOS too, because anchoring it would +// depend on how the working directory happens to be spelled. +func TestCanonicalizeTempRootsLeavesRelativeRootsAlone(t *testing.T) { + requireDarwin(t) + t.Setenv("TMPDIR", "../T") + t.Setenv("GOTMPDIR", "relative/go") + if err := testutil.CanonicalizeTempRoots(); err != nil { + t.Fatalf("canonicalize relative roots: %v", err) + } + for name, want := range map[string]string{"TMPDIR": "../T", "GOTMPDIR": "relative/go"} { + if got := os.Getenv(name); got != want { + t.Fatalf("%s = %q, want the relative root untouched %q", name, got, want) + } + } +} + // physicalTempDir avoids caching a test's temporary parent before its environment changes. func physicalTempDir(t *testing.T) string { t.Helper() From 681998eb30e17c0fb44b427ff1a64e11175afad2 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Thu, 29 Oct 2026 14:33:23 -0400 Subject: [PATCH 4/6] docs: number the temporary-root change entry for #2033 Co-Authored-By: Claude Opus 5.5 --- .../{0-mac-test-temp-roots.md => 2033-mac-test-temp-roots.md} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename docs/changes/unreleased/{0-mac-test-temp-roots.md => 2033-mac-test-temp-roots.md} (92%) diff --git a/docs/changes/unreleased/0-mac-test-temp-roots.md b/docs/changes/unreleased/2033-mac-test-temp-roots.md similarity index 92% rename from docs/changes/unreleased/0-mac-test-temp-roots.md rename to docs/changes/unreleased/2033-mac-test-temp-roots.md index 023e936c42..0f61b463e0 100644 --- a/docs/changes/unreleased/0-mac-test-temp-roots.md +++ b/docs/changes/unreleased/2033-mac-test-temp-roots.md @@ -1,7 +1,7 @@ --- kind: internal title: Test fixtures use physical temporary folders on macOS -pr: 0 +pr: 2033 surface: [build] invalidates: [] --- From 2dcf42689c5a23afdb510a1da63e66e182422937 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Thu, 29 Oct 2026 19:35:28 -0400 Subject: [PATCH 5/6] tests: the removed-computer label test draws a frame its project fits in TestRemovedComputerReopensWithTheSameProjectLabel/project checks the hint row for "project: " and the chat's t.TempDir(). With the physical /private/var/folders spelling this PR gives tui3's TestMain, that folder, named after the test, is longer than a 160-column row leaves for the project, so the label was elided and the check failed on macOS. The frame is now 260 columns, as the package's other project-label tests draw it. The label itself is unchanged. Co-Authored-By: Claude Opus 5.5 --- internal/tui3/relay_surface_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/tui3/relay_surface_test.go b/internal/tui3/relay_surface_test.go index 103a8edf21..cc015f14fe 100644 --- a/internal/tui3/relay_surface_test.go +++ b/internal/tui3/relay_surface_test.go @@ -106,7 +106,10 @@ func TestRemovedComputerReopensWithTheSameProjectLabel(t *testing.T) { spend(t, a, a.takeBeside(Conversation{Agent: &fakeAgent{model: "m"}, SessionFile: "/tmp/next/transcript.jsonl"})) a.title = "Next chat" a.Update(homeMachinesMsg{err: directory.ErrRevoked}) - a.width = 160 + // The frame is wide enough for the whole label. A temporary folder + // under macOS's physical /private/var/folders spelling, named after + // this test, is longer than a 160-column row leaves for the project. + a.width = 260 a.touch() a.tabsRow(a.width) span := tabSpanFor(t, a, "Copy chat") From 71dda06cea76222cd57ccf438396429f78651b50 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Thu, 29 Oct 2026 19:35:28 -0400 Subject: [PATCH 6/6] tests: cmd/codeaf's short state roots start at /private/tmp on macOS shortStateRoot and shortEngineHome exist so a workspace host's socket fits the 104-byte limit. Under the physical /private/var/folders spelling this PR gives cmd/codeaf's TestMain, host.sock lands a few bytes past it, and the host, environment, cells and stale-socket tests failed with "too long a path for a socket". On macOS both roots are now made under /tmp, spelled physically (/private/tmp) like the package's other fixture roots. Linux and Windows keep the temporary folder, so they run exactly as before. Co-Authored-By: Claude Opus 5.5 --- cmd/codeaf/chatv3_local_test.go | 20 +++++++++++++++++++- cmd/codeaf/engine_stale_test.go | 2 +- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/cmd/codeaf/chatv3_local_test.go b/cmd/codeaf/chatv3_local_test.go index b695984b70..49c0e4ff51 100644 --- a/cmd/codeaf/chatv3_local_test.go +++ b/cmd/codeaf/chatv3_local_test.go @@ -16,6 +16,7 @@ import ( "net/http/httptest" "os" "path/filepath" + "runtime" "strconv" "strings" "sync/atomic" @@ -155,7 +156,7 @@ func drainConsentRoad(t *testing.T, events <-chan session.Event, answer func(ses // rule. func shortStateRoot(t *testing.T) string { t.Helper() - root, err := os.MkdirTemp("", "afl") + root, err := os.MkdirTemp(shortSocketParent(t), "afl") if err != nil { t.Fatalf("make a state root: %v", err) } @@ -164,6 +165,23 @@ func shortStateRoot(t *testing.T) string { return root } +// shortSocketParent is where a short state root is made. On macOS it is /tmp, +// spelled physically (/private/tmp) like every other fixture root here: +// TestMain gives this package the physical /private/var/folders spelling of the +// temporary folder, and a workspace host's socket under that spelling is a few +// bytes past the socket limit. Other systems keep the temporary folder. +func shortSocketParent(t *testing.T) string { + t.Helper() + if runtime.GOOS != "darwin" { + return "" + } + parent, err := filepath.EvalSymlinks("/tmp") + if err != nil { + t.Fatalf("resolve /tmp: %v", err) + } + return parent +} + // answeringHost is a socket where this workspace's host would be, and nothing // behind it. The rule asks one question — is anybody there — so a listener is // the whole of what it takes to be somebody. diff --git a/cmd/codeaf/engine_stale_test.go b/cmd/codeaf/engine_stale_test.go index e5d087e590..6a3eb34772 100644 --- a/cmd/codeaf/engine_stale_test.go +++ b/cmd/codeaf/engine_stale_test.go @@ -210,7 +210,7 @@ func mustStandInJSON(value any) json.RawMessage { func shortEngineHome(t *testing.T) { t.Helper() - root, err := os.MkdirTemp("", "eh") + root, err := os.MkdirTemp(shortSocketParent(t), "eh") if err != nil { t.Fatalf("make a state root: %v", err) }