diff --git a/docs/changes/unreleased/2037-cellstore-absent-tree-alias.md b/docs/changes/unreleased/2037-cellstore-absent-tree-alias.md new file mode 100644 index 0000000000..fec24159df --- /dev/null +++ b/docs/changes/unreleased/2037-cellstore-absent-tree-alias.md @@ -0,0 +1,14 @@ +--- +kind: fixed +title: A chat folder that does not exist yet keeps one spelling through linked folders +pr: 2037 +surface: [engine] +invalidates: + - "A saved chat's folder that did not exist yet was named by its spelled path, and the same folder was named by its resolved path once created. Its longest existing ancestor is now resolved first, so the folder keeps one name before and after it is created." +--- + +macOS reaches `/tmp` and `/var` through links, and a home or project folder can +sit under a linked folder. Taking a chat into a folder that had to be created +there recorded one spelling and then read another, which cost an extra rebind +on the next save. A chat reached through a link that saves only its own folder +no longer calls its own not-yet-created `work/` folder foreign. diff --git a/internal/cellstore/canonical_tree_test.go b/internal/cellstore/canonical_tree_test.go new file mode 100644 index 0000000000..6c97f5ad11 --- /dev/null +++ b/internal/cellstore/canonical_tree_test.go @@ -0,0 +1,115 @@ +package cellstore + +import ( + "os" + "path/filepath" + "testing" +) + +// linkedRoot is a real folder and a symbolic link to it, both made by the test, +// so the alias is there on every platform and not only where the temporary +// folder happens to be reached through one (macOS /var). +func linkedRoot(t *testing.T) (real, link string) { + t.Helper() + base := t.TempDir() + real = filepath.Join(base, "real") + if err := os.Mkdir(real, 0o700); err != nil { + t.Fatal(err) + } + link = filepath.Join(base, "link") + if err := os.Symlink(real, link); err != nil { + t.Skipf("this filesystem cannot make a symbolic link: %v", err) + } + resolved, err := filepath.EvalSymlinks(real) + if err != nil { + t.Fatal(err) + } + return resolved, link +} + +// A folder that does not exist yet resolves the ancestors it has, so the same +// folder keeps one spelling before and after it is created: a binding recorded +// before creation must not read as another folder after it. +func TestCanonicalTreeResolvesAncestorOfMissingWorkspace(t *testing.T) { + real, link := linkedRoot(t) + missing := filepath.Join(link, "work", "nested") + want := filepath.Join(real, "work", "nested") + if got := canonicalTree(missing); got != want { + t.Fatalf("absent folder under a link: got %q, want %q", got, want) + } + if err := os.MkdirAll(missing, 0o700); err != nil { + t.Fatal(err) + } + if got := canonicalTree(missing); got != want { + t.Fatalf("the same folder once created: got %q, want %q", got, want) + } + if got := canonicalTree(filepath.Join(real, "other")); got != filepath.Join(real, "other") { + t.Fatalf("an absent folder under a real path changed spelling: %q", got) + } +} + +// A path through a file is not a missing folder: it keeps its absolute +// spelling rather than being resolved past the file. +func TestCanonicalTreeKeepsAPathThroughAFile(t *testing.T) { + real, link := linkedRoot(t) + if err := os.WriteFile(filepath.Join(real, "file"), []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + through := filepath.Join(link, "file", "child") + if got := canonicalTree(through); got != through { + t.Fatalf("a path through a file: got %q, want it unchanged %q", got, through) + } +} + +// A chat reached through a link that saves only its own folder still owns its +// work/ before that folder exists; a folder outside the chat stays foreign. +func TestForeignFolderKeepsMissingWorkUnderSymlinkedChat(t *testing.T) { + real, link := linkedRoot(t) + useHome(t, t.TempDir()) + root := filepath.Join(link, "chat") + if err := os.MkdirAll(filepath.Join(root, filepath.FromSlash(filepath.Dir(OwnFolderPath))), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, filepath.FromSlash(OwnFolderPath)), []byte("own\n"), 0o600); err != nil { + t.Fatal(err) + } + if ForeignFolder(root, filepath.Join(root, "work")) { + t.Fatal("the chat's own absent work/ is called foreign") + } + if ForeignFolder(filepath.Join(real, "chat"), filepath.Join(root, "work")) { + t.Fatal("the chat's own absent work/, named through the link, is called foreign from the real spelling") + } + outside := filepath.Join(real, "elsewhere") + if err := os.Mkdir(outside, 0o700); err != nil { + t.Fatal(err) + } + if !ForeignFolder(root, outside) { + t.Fatal("a folder outside the chat is not called foreign") + } +} + +// A link whose target is missing is not a missing folder: a path through it +// keeps its spelled, absolute form, so a chat's work/ that is a dangling link +// to somewhere else stays foreign when the chat is named through an alias. +func TestCanonicalTreeDoesNotResolvePastADanglingLink(t *testing.T) { + real, link := linkedRoot(t) + chat := filepath.Join(real, "chat") + if err := os.MkdirAll(filepath.Join(chat, filepath.FromSlash(filepath.Dir(OwnFolderPath))), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(chat, filepath.FromSlash(OwnFolderPath)), []byte("own\n"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.Symlink(filepath.Join(t.TempDir(), "absent"), filepath.Join(chat, "work")); err != nil { + t.Skipf("this filesystem cannot make a symbolic link: %v", err) + } + for _, spelled := range []string{filepath.Join(link, "chat", "work"), filepath.Join(link, "chat", "work", "deeper")} { + if got := canonicalTree(spelled); got != spelled { + t.Errorf("a path through a dangling link: got %q, want it unchanged %q", got, spelled) + } + } + useHome(t, t.TempDir()) + if !ForeignFolder(filepath.Join(link, "chat"), filepath.Join(link, "chat", "work")) { + t.Error("a dangling work/ link named through the alias is not called foreign") + } +} diff --git a/internal/cellstore/engine.go b/internal/cellstore/engine.go index 53ae7e3211..9d4562b541 100644 --- a/internal/cellstore/engine.go +++ b/internal/cellstore/engine.go @@ -4,11 +4,13 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "os" "os/user" "path/filepath" "runtime" + "slices" "strings" "time" @@ -409,13 +411,50 @@ func (e Engine) target(c cell.Cell) Target { return Target{Tree: canonicalTree(e.tree(c)), DataDir: dir, CellDir: e.cellDir(c), Timeline: timeline} } +// canonicalTree is the physical spelling of tree, so one folder is one tree +// whichever spelling named it: macOS reaches /tmp and /var through links, and a +// home or project folder can sit under a linked folder too. +// +// A tree that does not exist yet still resolves its ancestors. Its longest +// existing ancestor is resolved and the missing rest is kept as written. Were +// the spelled path returned instead, the same folder would change spelling the +// moment it was created: a binding recorded before creation would read as +// another folder after it, and a chat's own absent work/ would compare as +// outside the chat's resolved folder. Only components that are truly absent +// are kept as written; a link whose target is missing is not a missing folder, +// so a path through one keeps its absolute spelling, as does any other failure. func canonicalTree(tree string) string { absolute, err := filepath.Abs(tree) if err != nil { return filepath.Clean(tree) } - if resolved, err := filepath.EvalSymlinks(absolute); err == nil { + resolved, err := filepath.EvalSymlinks(absolute) + if err == nil { return resolved } - return absolute + if !errors.Is(err, os.ErrNotExist) { + return absolute + } + existing, missing := absolute, []string(nil) + for { + _, err := os.Lstat(existing) + if err == nil { + break + } + parent := filepath.Dir(existing) + if !errors.Is(err, os.ErrNotExist) || parent == existing { + return absolute + } + missing = append(missing, filepath.Base(existing)) + existing = parent + } + if len(missing) == 0 { + return absolute + } + resolved, err = filepath.EvalSymlinks(existing) + if err != nil { + return absolute + } + slices.Reverse(missing) + return filepath.Join(append([]string{resolved}, missing...)...) }