fix(root): Decouple project and setup selection - #94
Conversation
JordanCoin
commented
Aug 2, 2026
Reviewed. Separating "repo being analyzed" from "repo whose Codemap setup/state is reused" is a genuinely useful distinction for sandboxed worktrees, and Main design question before this lands:
Two smaller notes:
|
Keep analyzed-project and setup-storage roots as immutable invocation values, including linked-worktree validation. Resolve Git ownership through physical ancestry while preserving caller spelling when it maps to the same repository, and accept missing descendants without process-global state. This prepares explicit routing as a staged follow-up for agentic coding sandboxes. Signed-off-by: GPT-5.6 Sol <codex@openai.com> Co-Authored-By: GPT-5.6 Sol <codex@openai.com>
45303f3 to
173f5d5CompareApplied feedback:
Deep Codemap before and after found the final delta limited to cmd/root.go and cmd/root_test.go, with no cross-package importer or remaining singleton reference. The diff is huge though unfortunately, you have to review completely. |
reneleonhardt
commented
Aug 2, 2026
diff --git a/cmd/root.go b/cmd/root.go
index ff29b3e..d6c4dc4 100644
--- a/cmd/root.go+++ b/cmd/root.go@@ -129,42 +129,75 @@ func validateCodemapStorageRoot(root string) error {
}
// ResolveNearestGitRoot returns the nearest ancestor directory that contains a
-// .git entry. It accepts both .git directories and .git files used by linked-// worktrees. When no repository root exists, it returns the absolute input-// path and found=false.+// .git entry. It accepts missing descendants and both .git directories and+// .git files used by linked worktrees. When no repository root exists, it+// returns the absolute input path and found=false without resolving symlinks.
func ResolveNearestGitRoot(path string) (resolved string, found bool, err error) {
absPath, err := filepath.Abs(path)
if err != nil {
return "", false, err
}
- canonicalPath, err := filepath.EvalSymlinks(absPath)- if err != nil {- return "", false, err+ absPath = filepath.Clean(absPath)+ var existingPath string+ for current := absPath; ; current = filepath.Dir(current) {+ info, statErr := os.Stat(current)+ if statErr == nil {+ if !info.IsDir() {+ return "", false, fmt.Errorf("%q is not a directory", path)+ }+ existingPath = current+ break+ }+ if !os.IsNotExist(statErr) {+ return "", false, statErr+ }+ if _, lstatErr := os.Lstat(current); lstatErr == nil {+ return "", false, fmt.Errorf("%q is not a directory", path)+ } else if !os.IsNotExist(lstatErr) {+ return "", false, lstatErr+ }+ if filepath.Dir(current) == current {+ break+ }
}
- info, err := os.Stat(canonicalPath)++ physicalPath, err := filepath.EvalSymlinks(existingPath)
if err != nil {
return "", false, err
}
- if !info.IsDir() {- return "", false, fmt.Errorf("%q is not a directory", path)- }-- for current := canonicalPath; ; current = filepath.Dir(current) {+ physicalPath = filepath.Clean(physicalPath)+ for current := physicalPath; ; current = filepath.Dir(current) {
valid, err := validGitMarker(current)
if err != nil {
return "", false, err
}
if valid {
- return current, true, nil+ return logicalRootForPhysical(existingPath, physicalPath, current), true, nil
}
parent := filepath.Dir(current)
if parent == current {
- return canonicalPath, false, nil+ return absPath, false, nil
}
}
}
+func logicalRootForPhysical(logicalPath, physicalPath, physicalRoot string) string {+ rel, err := filepath.Rel(physicalRoot, physicalPath)+ if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) {+ return physicalRoot+ }+ logicalRoot := logicalPath+ for remaining := rel; remaining != "."; remaining = filepath.Dir(remaining) {+ logicalRoot = filepath.Dir(logicalRoot)+ }+ resolved, err := filepath.EvalSymlinks(logicalRoot)+ if err == nil && filepath.Clean(resolved) == physicalRoot {+ return logicalRoot+ }+ return physicalRoot+}+
func validGitMarker(root string) (bool, error) {
marker := filepath.Join(root, ".git")
info, err := os.Lstat(marker)
diff --git a/cmd/root_test.go b/cmd/root_test.go
index 1dd6054..741e6c5 100644
--- a/cmd/root_test.go+++ b/cmd/root_test.go@@ -27,12 +27,40 @@ func TestResolveNearestGitRoot(t *testing.T) {
if !found {
t.Fatal("expected repository root to be found")
}
- want := canonicalTestPath(t, root)+ want := root
if got != want {
t.Fatalf("ResolveNearestGitRoot() = %q, want %q", got, want)
}
})
+ t.Run("missing descendant resolves repository root", func(t *testing.T) {+ root := t.TempDir()+ if err := os.MkdirAll(filepath.Join(root, ".git"), 0o755); err != nil {+ t.Fatal(err)+ }+ missing := filepath.Join(root, "future", "feature")++ got, found, err := ResolveNearestGitRoot(missing)+ if err != nil {+ t.Fatalf("ResolveNearestGitRoot() error: %v", err)+ }+ if !found || got != root {+ t.Fatalf("ResolveNearestGitRoot() = %q, %t; want %q, true", got, found, root)+ }+ })++ t.Run("missing non repository preserves absolute input", func(t *testing.T) {+ missing := filepath.Join(t.TempDir(), "future", "feature")++ got, found, err := ResolveNearestGitRoot(missing)+ if err != nil {+ t.Fatalf("ResolveNearestGitRoot() error: %v", err)+ }+ if found || got != missing {+ t.Fatalf("ResolveNearestGitRoot() = %q, %t; want %q, false", got, found, missing)+ }+ })+
t.Run("git worktree file resolves repository root", func(t *testing.T) {
root := t.TempDir()
gitDir := filepath.Join(t.TempDir(), "worktrees", "example")
@@ -54,13 +82,13 @@ func TestResolveNearestGitRoot(t *testing.T) {
if !found {
t.Fatal("expected worktree root to be found")
}
- want := canonicalTestPath(t, root)+ want := root
if got != want {
t.Fatalf("ResolveNearestGitRoot() = %q, want %q", got, want)
}
})
- t.Run("symlinked nested directory resolves canonical repository root", func(t *testing.T) {+ t.Run("symlinked nested directory preserves repository alias", func(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("directory symlinks may require elevated privileges")
}
@@ -81,12 +109,95 @@ func TestResolveNearestGitRoot(t *testing.T) {
if err != nil {
t.Fatalf("ResolveNearestGitRoot() error: %v", err)
}
- want := canonicalTestPath(t, root)+ want := link
if !found || got != want {
t.Fatalf("ResolveNearestGitRoot() = %q, %t; want %q, true", got, found, want)
}
})
+ t.Run("repository-local symlink to external directory does not inherit repository", func(t *testing.T) {+ if runtime.GOOS == "windows" {+ t.Skip("directory symlinks may require elevated privileges")+ }+ root := t.TempDir()+ if err := os.Mkdir(filepath.Join(root, ".git"), 0o755); err != nil {+ t.Fatal(err)+ }+ external := t.TempDir()+ link := filepath.Join(root, "external")+ if err := os.Symlink(external, link); err != nil {+ t.Fatal(err)+ }++ for _, input := range []string{link, filepath.Join(link, "future", "feature")} {+ got, found, err := ResolveNearestGitRoot(input)+ if err != nil {+ t.Fatalf("ResolveNearestGitRoot(%q) error: %v", input, err)+ }+ if found || got != input {+ t.Fatalf("ResolveNearestGitRoot(%q) = %q, %t; want %q, false", input, got, found, input)+ }+ }+ })++ t.Run("repository-local symlink into another repository uses physical repository", func(t *testing.T) {+ if runtime.GOOS == "windows" {+ t.Skip("directory symlinks may require elevated privileges")+ }+ outer := t.TempDir()+ if err := os.Mkdir(filepath.Join(outer, ".git"), 0o755); err != nil {+ t.Fatal(err)+ }+ other := t.TempDir()+ if err := os.Mkdir(filepath.Join(other, ".git"), 0o755); err != nil {+ t.Fatal(err)+ }+ nested := filepath.Join(other, "pkg")+ if err := os.Mkdir(nested, 0o755); err != nil {+ t.Fatal(err)+ }+ link := filepath.Join(outer, "other-pkg")+ if err := os.Symlink(nested, link); err != nil {+ t.Fatal(err)+ }++ for _, input := range []string{link, filepath.Join(link, "future", "feature")} {+ got, found, err := ResolveNearestGitRoot(input)+ if err != nil {+ t.Fatalf("ResolveNearestGitRoot(%q) error: %v", input, err)+ }+ physicalOther, err := filepath.EvalSymlinks(other)+ if err != nil {+ t.Fatal(err)+ }+ if !found || got != physicalOther {+ t.Fatalf("ResolveNearestGitRoot(%q) = %q, %t; want physical root %q, true", input, got, found, physicalOther)+ }+ }+ })++ t.Run("macOS tmp spelling is preserved", func(t *testing.T) {+ if runtime.GOOS != "darwin" {+ t.Skip("macOS exposes /tmp through /private/tmp")+ }+ root, err := os.MkdirTemp("/tmp", "codemap-root-")+ if err != nil {+ t.Fatal(err)+ }+ t.Cleanup(func() { _ = os.RemoveAll(root) })+ if err := os.Mkdir(filepath.Join(root, ".git"), 0o755); err != nil {+ t.Fatal(err)+ }++ got, found, err := ResolveNearestGitRoot(root)+ if err != nil {+ t.Fatalf("ResolveNearestGitRoot() error: %v", err)+ }+ if !found || got != root {+ t.Fatalf("ResolveNearestGitRoot() = %q, %t; want %q, true", got, found, root)+ }+ })+
t.Run("regular file input is rejected", func(t *testing.T) {
root := t.TempDir()
file := filepath.Join(root, "input")
@@ -98,6 +209,55 @@ func TestResolveNearestGitRoot(t *testing.T) {
}
})
+ t.Run("symlink to file input is rejected", func(t *testing.T) {+ if runtime.GOOS == "windows" {+ t.Skip("symlinks may require elevated privileges")+ }+ root := t.TempDir()+ target := filepath.Join(root, "target")+ if err := os.WriteFile(target, []byte("not a directory"), 0o644); err != nil {+ t.Fatal(err)+ }+ link := filepath.Join(root, "link")+ if err := os.Symlink(target, link); err != nil {+ t.Fatal(err)+ }+ if _, _, err := ResolveNearestGitRoot(link); err == nil {+ t.Fatal("expected symlink to a file to be rejected")+ }+ })++ t.Run("dangling symlink input is rejected", func(t *testing.T) {+ if runtime.GOOS == "windows" {+ t.Skip("symlinks may require elevated privileges")+ }+ root := t.TempDir()+ link := filepath.Join(root, "link")+ if err := os.Symlink(filepath.Join(root, "missing"), link); err != nil {+ t.Fatal(err)+ }+ if _, _, err := ResolveNearestGitRoot(link); err == nil {+ t.Fatal("expected dangling symlink to be rejected")+ }+ })++ t.Run("missing descendant below dangling symlink is rejected", func(t *testing.T) {+ if runtime.GOOS == "windows" {+ t.Skip("symlinks may require elevated privileges")+ }+ root := t.TempDir()+ if err := os.Mkdir(filepath.Join(root, ".git"), 0o755); err != nil {+ t.Fatal(err)+ }+ link := filepath.Join(root, "link")+ if err := os.Symlink(filepath.Join(root, "missing"), link); err != nil {+ t.Fatal(err)+ }+ if _, _, err := ResolveNearestGitRoot(filepath.Join(link, "child")); err == nil {+ t.Fatal("expected a missing descendant below a dangling symlink to be rejected")+ }+ })+
t.Run("device file input is rejected", func(t *testing.T) {
if _, err := os.Stat(os.DevNull); err != nil {
t.Skipf("device file unavailable: %v", err)
@@ -130,7 +290,7 @@ func TestResolveNearestGitRoot(t *testing.T) {
if err != nil {
t.Fatalf("ResolveNearestGitRoot() error: %v", err)
}
- if want := canonicalTestPath(t, root); !found || got != want {+ if want := root; !found || got != want {
t.Fatalf("ResolveNearestGitRoot() = %q, %t; want %q, true", got, found, want)
}
})
@@ -175,7 +335,7 @@ func TestResolveNearestGitRoot(t *testing.T) {
if !found {
t.Fatal("expected repository root to be found")
}
- want := canonicalTestPath(t, root)+ want := root
if got != want {
t.Fatalf("ResolveNearestGitRoot() = %q, want %q", got, want)
}
@@ -194,7 +354,7 @@ func TestResolveNearestGitRoot(t *testing.T) {
if found {
t.Fatal("expected no repository root to be found")
}
- want := canonicalTestPath(t, root)+ want := root
if got != want {
t.Fatalf("ResolveNearestGitRoot() = %q, want %q", got, want)
}
@@ -273,7 +433,7 @@ func TestParseGlobalRootOptions(t *testing.T) {
}
func TestResolveGlobalRoots(t *testing.T) {
- launchDir := canonicalTestPath(t, t.TempDir())+ launchDir := t.TempDir()
projectRoot := filepath.Join(launchDir, "worktree")
setupRoot := filepath.Join(launchDir, "original")
for _, root := range []string{projectRoot, setupRoot} {
@@ -319,7 +479,7 @@ func TestResolveGlobalRoots(t *testing.T) {
})
t.Run("setup root alone preserves non repository project", func(t *testing.T) {
- project := canonicalTestPath(t, t.TempDir())+ project := t.TempDir()
roots, err := ResolveGlobalRoots(GlobalRootOptions{SetupRoot: setupNested}, project)
if err != nil {
t.Fatalf("ResolveGlobalRoots() error: %v", err)
@@ -424,12 +584,3 @@ func TestResolveGlobalRoots(t *testing.T) {
}
})
}
--func canonicalTestPath(t *testing.T, path string) string {- t.Helper()- resolved, err := filepath.EvalSymlinks(path)- if err != nil {- t.Fatal(err)- }- return resolved-}diff --git a/internal/projectpath/path.go b/internal/projectpath/path.go
deleted file mode 100644
index 8aeec0c..0000000
--- a/internal/projectpath/path.go+++ /dev/null@@ -1,60 +0,0 @@-// Package projectpath separates the analyzed project from its Codemap setup.-package projectpath--import (- "path/filepath"- "strings"- "sync"-)--var configuredSetupRoot struct {- sync.RWMutex- path string-}--// SetupRoot returns the configured setup root or the analyzed project root.-func SetupRoot(projectRoot string) string {- if root := ConfiguredSetupRoot(); root != "" {- return filepath.Clean(root)- }- return projectRoot-}--// ConfiguredSetupRoot returns the invocation-wide override, if any.-func ConfiguredSetupRoot() string {- configuredSetupRoot.RLock()- defer configuredSetupRoot.RUnlock()- return configuredSetupRoot.path-}--// SetSetupRoot configures the validated setup root for this process.-func SetSetupRoot(root string) {- configuredSetupRoot.Lock()- defer configuredSetupRoot.Unlock()- root = strings.TrimSpace(root)- if root == "" {- configuredSetupRoot.path = ""- return- }- configuredSetupRoot.path = filepath.Clean(root)-}--// ResetSetupRoot clears invocation-scoped setup state.-func ResetSetupRoot() {- SetSetupRoot("")-}--// PrependSetupRootArgs preserves setup selection across Codemap subprocesses.-func PrependSetupRootArgs(args ...string) []string {- root := ConfiguredSetupRoot()- result := make([]string, 0, len(args)+2)- if root != "" {- result = append(result, "--setup-root", root)- }- return append(result, args...)-}--// CodemapDir returns the .codemap directory associated with a project.-func CodemapDir(projectRoot string) string {- return filepath.Join(SetupRoot(projectRoot), ".codemap")-}diff --git a/internal/projectpath/path_test.go b/internal/projectpath/path_test.go
deleted file mode 100644
index 944193d..0000000
--- a/internal/projectpath/path_test.go+++ /dev/null@@ -1,61 +0,0 @@-package projectpath--import (- "path/filepath"- "testing"-)--func TestSetupRoot(t *testing.T) {- projectRoot := filepath.Join(t.TempDir(), "project")- setupRoot := filepath.Join(t.TempDir(), "setup")-- t.Run("defaults to project root", func(t *testing.T) {- ResetSetupRoot()- t.Cleanup(ResetSetupRoot)- t.Setenv("CODEMAP_SETUP_ROOT", setupRoot)- if got := SetupRoot(projectRoot); got != projectRoot {- t.Fatalf("SetupRoot() = %q, want %q", got, projectRoot)- }- if got := ConfiguredSetupRoot(); got != "" {- t.Fatalf("ConfiguredSetupRoot() = %q, want empty despite inherited environment", got)- }- })-- t.Run("uses configured setup root", func(t *testing.T) {- SetSetupRoot(setupRoot)- t.Cleanup(ResetSetupRoot)- if got := SetupRoot(projectRoot); got != setupRoot {- t.Fatalf("SetupRoot() = %q, want %q", got, setupRoot)- }- want := filepath.Join(setupRoot, ".codemap")- if got := CodemapDir(projectRoot); got != want {- t.Fatalf("CodemapDir() = %q, want %q", got, want)- }- })-- t.Run("clears configured setup root", func(t *testing.T) {- SetSetupRoot(setupRoot)- ResetSetupRoot()- if got := SetupRoot(projectRoot); got != projectRoot {- t.Fatalf("SetupRoot() = %q, want %q", got, projectRoot)- }- })-}--func TestPrependSetupRootArgs(t *testing.T) {- setupRoot := filepath.Join(t.TempDir(), "setup")- projectRoot := filepath.Join(t.TempDir(), "project")- SetSetupRoot(setupRoot)- t.Cleanup(ResetSetupRoot)-- got := PrependSetupRootArgs("watch", "start", projectRoot)- want := []string{"--setup-root", setupRoot, "watch", "start", projectRoot}- if len(got) != len(want) {- t.Fatalf("PrependSetupRootArgs() = %#v, want %#v", got, want)- }- for i := range want {- if got[i] != want[i] {- t.Fatalf("PrependSetupRootArgs() = %#v, want %#v", got, want)- }- }-} |
What does this PR do?
Adds the typed root-selection contract needed to fix root coupling for agentic coding in separate worktrees and sandbox checkouts, including linked-worktree validation.
Type of change
Checklist
go build && ./codemap .Additional notes
This is the independent root-selection contract; CLI routing follows in
feat/explicit-project-root-cli.After the full stack lands, an agent can analyze a sandbox worktree while reusing the canonical checkout's setup:
Developed with carefully directed, manually reviewed AI assistance.
Co-Authored-By: GPT-5.6 Sol codex@openai.com