Skip to content

Commit 42e85ec

Browse files
AbirAbbasclaude
andcommitted
cellstore: stop at a dangling link, walk the missing suffix once
Review fixes: only components that are truly absent (Lstat says so) are kept as written; a link whose target is missing keeps the absolute spelling, so a chat's dangling work/ link named through an alias stays foreign. The missing suffix is collected in one pass and the existing ancestor resolved once. The through-a-file test runs on Windows too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent a5f5064 commit 42e85ec

2 files changed

Lines changed: 53 additions & 12 deletions

File tree

‎internal/cellstore/canonical_tree_test.go‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package cellstore
33
import (
44
"os"
55
"path/filepath"
6-
"runtime"
76
"testing"
87
)
98

@@ -52,9 +51,6 @@ func TestCanonicalTreeResolvesAncestorOfMissingWorkspace(t *testing.T) {
5251
// A path through a file is not a missing folder: it keeps its absolute
5352
// spelling rather than being resolved past the file.
5453
func TestCanonicalTreeKeepsAPathThroughAFile(t *testing.T) {
55-
if runtime.GOOS == "windows" {
56-
t.Skip("Windows reports a path through a file as a missing path")
57-
}
5854
real, link := linkedRoot(t)
5955
if err := os.WriteFile(filepath.Join(real, "file"), []byte("x"), 0o600); err != nil {
6056
t.Fatal(err)
@@ -91,3 +87,29 @@ func TestForeignFolderKeepsMissingWorkUnderSymlinkedChat(t *testing.T) {
9187
t.Fatal("a folder outside the chat is not called foreign")
9288
}
9389
}
90+
91+
// A link whose target is missing is not a missing folder: a path through it
92+
// keeps its spelled, absolute form, so a chat's work/ that is a dangling link
93+
// to somewhere else stays foreign when the chat is named through an alias.
94+
func TestCanonicalTreeDoesNotResolvePastADanglingLink(t *testing.T) {
95+
real, link := linkedRoot(t)
96+
chat := filepath.Join(real, "chat")
97+
if err := os.MkdirAll(filepath.Join(chat, filepath.FromSlash(filepath.Dir(OwnFolderPath))), 0o700); err != nil {
98+
t.Fatal(err)
99+
}
100+
if err := os.WriteFile(filepath.Join(chat, filepath.FromSlash(OwnFolderPath)), []byte("own\n"), 0o600); err != nil {
101+
t.Fatal(err)
102+
}
103+
if err := os.Symlink(filepath.Join(t.TempDir(), "absent"), filepath.Join(chat, "work")); err != nil {
104+
t.Skipf("this filesystem cannot make a symbolic link: %v", err)
105+
}
106+
for _, spelled := range []string{filepath.Join(link, "chat", "work"), filepath.Join(link, "chat", "work", "deeper")} {
107+
if got := canonicalTree(spelled); got != spelled {
108+
t.Errorf("a path through a dangling link: got %q, want it unchanged %q", got, spelled)
109+
}
110+
}
111+
useHome(t, t.TempDir())
112+
if !ForeignFolder(filepath.Join(link, "chat"), filepath.Join(link, "chat", "work")) {
113+
t.Error("a dangling work/ link named through the alias is not called foreign")
114+
}
115+
}

‎internal/cellstore/engine.go‎

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"os/user"
1111
"path/filepath"
1212
"runtime"
13+
"slices"
1314
"strings"
1415
"time"
1516

@@ -414,13 +415,14 @@ func (e Engine) target(c cell.Cell) Target {
414415
// whichever spelling named it: macOS reaches /tmp and /var through links, and a
415416
// home or project folder can sit under a linked folder too.
416417
//
417-
// A TREE THAT DOES NOT EXIST YET STILL RESOLVES ITS ANCESTORS. Its longest
418+
// A tree that does not exist yet still resolves its ancestors. Its longest
418419
// existing ancestor is resolved and the missing rest is kept as written. Were
419-
// the lexical spelling returned instead, the same folder would change spelling
420-
// the moment it was created: a binding recorded before creation would read as
420+
// the spelled path returned instead, the same folder would change spelling the
421+
// moment it was created: a binding recorded before creation would read as
421422
// another folder after it, and a chat's own absent work/ would compare as
422-
// outside the chat's resolved folder. Any other failure keeps the absolute
423-
// spelling, as before.
423+
// outside the chat's resolved folder. Only components that are truly absent
424+
// are kept as written; a link whose target is missing is not a missing folder,
425+
// so a path through one keeps its absolute spelling, as does any other failure.
424426
func canonicalTree(tree string) string {
425427
absolute, err := filepath.Abs(tree)
426428
if err != nil {
@@ -433,9 +435,26 @@ func canonicalTree(tree string) string {
433435
if !errors.Is(err, os.ErrNotExist) {
434436
return absolute
435437
}
436-
parent := filepath.Dir(absolute)
437-
if parent == absolute {
438+
existing, missing := absolute, []string(nil)
439+
for {
440+
_, err := os.Lstat(existing)
441+
if err == nil {
442+
break
443+
}
444+
parent := filepath.Dir(existing)
445+
if !errors.Is(err, os.ErrNotExist) || parent == existing {
446+
return absolute
447+
}
448+
missing = append(missing, filepath.Base(existing))
449+
existing = parent
450+
}
451+
if len(missing) == 0 {
452+
return absolute
453+
}
454+
resolved, err = filepath.EvalSymlinks(existing)
455+
if err != nil {
438456
return absolute
439457
}
440-
return filepath.Join(canonicalTree(parent), filepath.Base(absolute))
458+
slices.Reverse(missing)
459+
return filepath.Join(append([]string{resolved}, missing...)...)
441460
}

0 commit comments

Comments
 (0)