From 5257f939344d6ed3fd75006e95714fed1bab20c4 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Thu, 29 Oct 2026 14:16:41 -0400 Subject: [PATCH 1/3] cellstore: a missing tree resolves its existing ancestors canonicalTree returned the spelled path whenever the leaf did not exist, so a workspace under a linked ancestor (macOS /tmp and /var, a linked home or project folder) changed spelling the moment it was created. Follow then recorded one tree and the next save rebound to the other, and ForeignFolder called a linked chat's own absent work/ foreign. Resolve the longest existing ancestor and keep the missing rest as written; other errors keep the absolute spelling. Co-Authored-By: Claude Opus 5.5 --- .../0-app-cellstore-absent-tree-alias.md | 14 +++ internal/cellstore/canonical_tree_test.go | 93 +++++++++++++++++++ internal/cellstore/engine.go | 24 ++++- 3 files changed, 129 insertions(+), 2 deletions(-) create mode 100644 docs/changes/unreleased/0-app-cellstore-absent-tree-alias.md create mode 100644 internal/cellstore/canonical_tree_test.go diff --git a/docs/changes/unreleased/0-app-cellstore-absent-tree-alias.md b/docs/changes/unreleased/0-app-cellstore-absent-tree-alias.md new file mode 100644 index 0000000000..c3341cc3ee --- /dev/null +++ b/docs/changes/unreleased/0-app-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: 0 +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..86dfabb943 --- /dev/null +++ b/internal/cellstore/canonical_tree_test.go @@ -0,0 +1,93 @@ +package cellstore + +import ( + "os" + "path/filepath" + "runtime" + "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) { + if runtime.GOOS == "windows" { + t.Skip("Windows reports a path through a file as a missing path") + } + 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") + } +} diff --git a/internal/cellstore/engine.go b/internal/cellstore/engine.go index 53ae7e3211..76669a31e3 100644 --- a/internal/cellstore/engine.go +++ b/internal/cellstore/engine.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "os" "os/user" @@ -409,13 +410,32 @@ 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 lexical spelling 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. Any other failure keeps the absolute +// spelling, as before. 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 + } + parent := filepath.Dir(absolute) + if parent == absolute { + return absolute + } + return filepath.Join(canonicalTree(parent), filepath.Base(absolute)) } From 490c7a6d0dc8ccfb6e60a77b15ed26b658a072cd Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Thu, 29 Oct 2026 14:29:52 -0400 Subject: [PATCH 2/3] 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 --- internal/cellstore/canonical_tree_test.go | 30 ++++++++++++++++--- internal/cellstore/engine.go | 35 +++++++++++++++++------ 2 files changed, 53 insertions(+), 12 deletions(-) diff --git a/internal/cellstore/canonical_tree_test.go b/internal/cellstore/canonical_tree_test.go index 86dfabb943..6c97f5ad11 100644 --- a/internal/cellstore/canonical_tree_test.go +++ b/internal/cellstore/canonical_tree_test.go @@ -3,7 +3,6 @@ package cellstore import ( "os" "path/filepath" - "runtime" "testing" ) @@ -52,9 +51,6 @@ func TestCanonicalTreeResolvesAncestorOfMissingWorkspace(t *testing.T) { // 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) { - if runtime.GOOS == "windows" { - t.Skip("Windows reports a path through a file as a missing path") - } real, link := linkedRoot(t) if err := os.WriteFile(filepath.Join(real, "file"), []byte("x"), 0o600); err != nil { t.Fatal(err) @@ -91,3 +87,29 @@ func TestForeignFolderKeepsMissingWorkUnderSymlinkedChat(t *testing.T) { 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 76669a31e3..9d4562b541 100644 --- a/internal/cellstore/engine.go +++ b/internal/cellstore/engine.go @@ -10,6 +10,7 @@ import ( "os/user" "path/filepath" "runtime" + "slices" "strings" "time" @@ -414,13 +415,14 @@ func (e Engine) target(c cell.Cell) Target { // 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 +// 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 lexical spelling returned instead, the same folder would change spelling -// the moment it was created: a binding recorded before creation would read as +// 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. Any other failure keeps the absolute -// spelling, as before. +// 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 { @@ -433,9 +435,26 @@ func canonicalTree(tree string) string { if !errors.Is(err, os.ErrNotExist) { return absolute } - parent := filepath.Dir(absolute) - if parent == 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 } - return filepath.Join(canonicalTree(parent), filepath.Base(absolute)) + slices.Reverse(missing) + return filepath.Join(append([]string{resolved}, missing...)...) } From 3f3b18e89ea06ecf1d3bbc3787daa63fe5169571 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Thu, 29 Oct 2026 19:16:25 -0400 Subject: [PATCH 3/3] docs: number the change entry for #2037 Co-Authored-By: Claude Opus 5.5 --- ...absent-tree-alias.md => 2037-cellstore-absent-tree-alias.md} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename docs/changes/unreleased/{0-app-cellstore-absent-tree-alias.md => 2037-cellstore-absent-tree-alias.md} (98%) diff --git a/docs/changes/unreleased/0-app-cellstore-absent-tree-alias.md b/docs/changes/unreleased/2037-cellstore-absent-tree-alias.md similarity index 98% rename from docs/changes/unreleased/0-app-cellstore-absent-tree-alias.md rename to docs/changes/unreleased/2037-cellstore-absent-tree-alias.md index c3341cc3ee..fec24159df 100644 --- a/docs/changes/unreleased/0-app-cellstore-absent-tree-alias.md +++ b/docs/changes/unreleased/2037-cellstore-absent-tree-alias.md @@ -1,7 +1,7 @@ --- kind: fixed title: A chat folder that does not exist yet keeps one spelling through linked folders -pr: 0 +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."