From dddc7577804b8b8084a2abf3f95e5c6130667de6 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Thu, 29 Oct 2026 14:16:49 -0400 Subject: [PATCH 1/4] app: a file stat compares resolved paths when spellings differ statFile tested containment on the spelled paths only, so a file a picker or a drag names /private/tmp/demo/notes.md was outside a chat whose workspace was typed as /tmp/demo (the /folder command keeps the typed spelling), and its attachment chip said outside this workspace. Keep the spelled test first and, only when it fails, resolve both sides; a change recorded under the workspace's own spelling is found too. Co-Authored-By: Claude Opus 5.5 --- cmd/codeaf/appconvo.go | 23 +++- cmd/codeaf/appconvo_alias_test.go | 119 ++++++++++++++++++ .../unreleased/0-app-file-stat-alias.md | 14 +++ 3 files changed, 153 insertions(+), 3 deletions(-) create mode 100644 cmd/codeaf/appconvo_alias_test.go create mode 100644 docs/changes/unreleased/0-app-file-stat-alias.md diff --git a/cmd/codeaf/appconvo.go b/cmd/codeaf/appconvo.go index 0745d775bd..8428b98e45 100644 --- a/cmd/codeaf/appconvo.go +++ b/cmd/codeaf/appconvo.go @@ -973,9 +973,22 @@ func (b *appBackend) statFile(workspace string, changed map[string]appwire.FileC } full = filepath.Clean(full) stat := appwire.FileStat{Path: path, State: appwire.FileExists} - if rel, err := filepath.Rel(workspace, full); workspace == "" || err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { - stat.State = appwire.FileOutside - return stat + // ONE FOLDER CAN BE SPELLED TWO WAYS. macOS reaches /tmp and /var through + // links, and a project can sit under a linked folder, so a file a picker or + // a drag names /private/tmp/demo/notes.md is inside a workspace typed as + // /tmp/demo. The spelled test stays first, so nothing inside by spelling is + // newly called outside; only a path outside by spelling is resolved, and it + // is inside when its resolved place is. same is that file in the + // workspace's own spelling, the key a change recorded there is under. + same := full + if !appWithin(workspace, full) { + root, place := appCanonical(workspace), appCanonical(full) + rel, err := filepath.Rel(root, place) + if err != nil || !appWithin(root, place) { + stat.State = appwire.FileOutside + return stat + } + same = filepath.Join(workspace, rel) } info, err := os.Stat(full) if err != nil { @@ -1000,6 +1013,10 @@ func (b *appBackend) statFile(workspace string, changed map[string]appwire.FileC stat.State = appwire.FileChanged add, del := change.Add, change.Del stat.Add, stat.Del = &add, &del + } else if change, ok := changed[same]; ok { + stat.State = appwire.FileChanged + add, del := change.Add, change.Del + stat.Add, stat.Del = &add, &del } return stat } diff --git a/cmd/codeaf/appconvo_alias_test.go b/cmd/codeaf/appconvo_alias_test.go new file mode 100644 index 0000000000..cb9cd2df09 --- /dev/null +++ b/cmd/codeaf/appconvo_alias_test.go @@ -0,0 +1,119 @@ +package main + +import ( + "os" + "path/filepath" + "sync" + "testing" + + "github.com/Agent-Field/codeaf/internal/appwire" + "github.com/Agent-Field/codeaf/internal/session" +) + +// aliasBackend is a desktop backend with one chat whose workspace is spelled +// through a symbolic link the test made, so the alias exists on every platform +// and not only where the temporary folder is reached through one (macOS /var). +func aliasBackend(t *testing.T, spelledThroughLink bool) (backend *appBackend, agent appwire.Agent, real, link string) { + t.Helper() + base := t.TempDir() + real = filepath.Join(base, "real") + if err := os.Mkdir(real, 0o755); 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) + } + real = resolved + if err := os.WriteFile(filepath.Join(real, "notes.md"), []byte("# notes\n"), 0o644); err != nil { + t.Fatal(err) + } + var mu sync.Mutex + var opens, releases int + conv := &fakeConversation{script: []session.Event{{Kind: session.EventTurnDone}}} + backend = newFakeBackend(newAppRecorder(), fakeOpener(conv, &opens, &releases, &mu)) + t.Cleanup(func() { backend.Close() }) + backend.state = &appState{path: filepath.Join(t.TempDir(), appStateName)} + workspace := real + if spelledThroughLink { + workspace = link + } + agent, err = mustCreate(t, backend, appwire.CreateAsk{Workspace: workspace}) + if err != nil { + t.Fatal(err) + } + return backend, agent, real, link +} + +// A file the picker names in its resolved spelling is inside a workspace typed +// through a link, and the other way round; a missing file there is missing, +// not outside; a file that is really elsewhere stays outside. +func TestAppFileStatUsesResolvedWorkspaceContainment(t *testing.T) { + for _, throughLink := range []bool{true, false} { + backend, agent, real, link := aliasBackend(t, throughLink) + other := real + if !throughLink { + other = link + } + elsewhere := filepath.Join(t.TempDir(), "elsewhere.md") + if err := os.WriteFile(elsewhere, []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + stats, err := backend.StatFiles(agent.ID, []string{ + "notes.md", + filepath.Join(other, "notes.md"), + filepath.Join(other, "gone.md"), + elsewhere, + }) + if err != nil || len(stats) != 4 { + t.Fatalf("workspace %q: files.stat = %+v %v", agent.Workspace, stats, err) + } + want := []string{appwire.FileExists, appwire.FileExists, appwire.FileMissing, appwire.FileOutside} + for i, stat := range stats { + if stat.State != want[i] { + t.Errorf("workspace %q: %s is %q, want %q", agent.Workspace, stat.Path, stat.State, want[i]) + } + } + if stats[1].Path != filepath.Join(other, "notes.md") { + t.Errorf("the stat renamed the path it was asked about: %q", stats[1].Path) + } + chip, err := backend.AddAttachment(agent.ID, appwire.Attachment{Kind: appwire.AttachFile, Path: filepath.Join(other, "notes.md")}, "") + if err != nil || chip.State != appwire.FileExists { + t.Errorf("workspace %q: the attachment chip = %+v %v", agent.Workspace, chip, err) + } + } +} + +// A change recorded under the workspace's own spelling is found for the same +// file named in the other spelling. +func TestAppFileStatFindsAChangeRecordedUnderTheWorkspaceSpelling(t *testing.T) { + backend, agent, real, link := aliasBackend(t, true) + changed := map[string]appwire.FileChange{ + filepath.Join(link, "notes.md"): {Path: filepath.Join(link, "notes.md"), Add: 3, Del: 1}, + } + stat := backend.statFile(agent.Workspace, changed, filepath.Join(real, "notes.md")) + if stat.State != appwire.FileChanged || stat.Add == nil || *stat.Add != 3 || stat.Del == nil || *stat.Del != 1 { + t.Fatalf("the change was not found through the other spelling: %+v", stat) + } +} + +// A link inside the workspace that points elsewhere keeps the answer it had: +// it is inside by spelling, and this change only adds what is inside by place. +func TestAppFileStatKeepsALinkInsideTheWorkspaceInside(t *testing.T) { + backend, agent, real, _ := aliasBackend(t, false) + elsewhere := t.TempDir() + if err := os.WriteFile(filepath.Join(elsewhere, "far.md"), []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Symlink(elsewhere, filepath.Join(real, "far")); err != nil { + t.Skipf("this filesystem cannot make a symbolic link: %v", err) + } + stats, err := backend.StatFiles(agent.ID, []string{"far/far.md"}) + if err != nil || len(stats) != 1 || stats[0].State != appwire.FileExists { + t.Fatalf("a file through a link inside the workspace = %+v %v", stats, err) + } +} diff --git a/docs/changes/unreleased/0-app-file-stat-alias.md b/docs/changes/unreleased/0-app-file-stat-alias.md new file mode 100644 index 0000000000..e87bda85ed --- /dev/null +++ b/docs/changes/unreleased/0-app-file-stat-alias.md @@ -0,0 +1,14 @@ +--- +kind: fixed +title: A desktop file chip in the chat's folder is not called outside through a linked path +pr: 0 +surface: [engine] +invalidates: + - "A file inside the chat's workspace read as outside this workspace when the file and the workspace were spelled through different links, such as /private/tmp/demo and /tmp/demo on macOS. Containment now also compares resolved paths." +--- + +A workspace typed as `/tmp/demo` (or through a linked project folder) and a +file a picker or a drag names `/private/tmp/demo/notes.md` are one place. The +desktop's attachment chips and `files.stat` now answer `exists`, `changed` or +`missing` for it instead of `outside`. A path inside the workspace by spelling +keeps the answer it had, and a file that is really elsewhere is still outside. From f43f84ad93488cee1ab7a77f27210eb867da10f3 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Thu, 29 Oct 2026 14:29:52 -0400 Subject: [PATCH 2/4] app: a file stat opens only regular files Review fix: a FIFO newly reached through the resolved spelling (and one already inside the workspace by spelling) was opened to sniff its first bytes, which waits for a writer and holds files.stat or attachments.add. Only a regular file is opened now. Co-Authored-By: Claude Opus 5.5 --- cmd/codeaf/appconvo.go | 6 +++-- cmd/codeaf/appconvo_alias_unix_test.go | 31 ++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) create mode 100644 cmd/codeaf/appconvo_alias_unix_test.go diff --git a/cmd/codeaf/appconvo.go b/cmd/codeaf/appconvo.go index 8428b98e45..1e766f736b 100644 --- a/cmd/codeaf/appconvo.go +++ b/cmd/codeaf/appconvo.go @@ -973,7 +973,7 @@ func (b *appBackend) statFile(workspace string, changed map[string]appwire.FileC } full = filepath.Clean(full) stat := appwire.FileStat{Path: path, State: appwire.FileExists} - // ONE FOLDER CAN BE SPELLED TWO WAYS. macOS reaches /tmp and /var through + // One folder can be spelled two ways. macOS reaches /tmp and /var through // links, and a project can sit under a linked folder, so a file a picker or // a drag names /private/tmp/demo/notes.md is inside a workspace typed as // /tmp/demo. The spelled test stays first, so nothing inside by spelling is @@ -997,7 +997,9 @@ func (b *appBackend) statFile(workspace string, changed map[string]appwire.FileC } size := info.Size() stat.Size = &size - if !info.IsDir() { + // Only a regular file is opened for its first bytes: opening a FIFO or a + // device would wait for a writer and hold the request. + if info.Mode().IsRegular() { if file, err := os.Open(full); err == nil { head := make([]byte, 8000) n, _ := io.ReadFull(file, head) diff --git a/cmd/codeaf/appconvo_alias_unix_test.go b/cmd/codeaf/appconvo_alias_unix_test.go new file mode 100644 index 0000000000..997972c853 --- /dev/null +++ b/cmd/codeaf/appconvo_alias_unix_test.go @@ -0,0 +1,31 @@ +//go:build !windows + +package main + +import ( + "path/filepath" + "syscall" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/appwire" +) + +// A FIFO in the workspace is stat'ed, never opened: opening it would wait for +// a writer that never comes. The alias spelling reaches it the same way. +func TestAppFileStatNeverOpensAFIFO(t *testing.T) { + backend, agent, real, _ := aliasBackend(t, true) + if err := syscall.Mkfifo(filepath.Join(real, "pipe"), 0o600); err != nil { + t.Skipf("this filesystem cannot make a FIFO: %v", err) + } + done := make(chan appwire.FileStat, 1) + go func() { done <- backend.statFile(agent.Workspace, nil, filepath.Join(real, "pipe")) }() + select { + case stat := <-done: + if stat.State != appwire.FileExists || stat.Binary { + t.Fatalf("the FIFO's stat = %+v", stat) + } + case <-time.After(5 * time.Second): + t.Fatal("stat'ing a FIFO in the workspace waited for a writer") + } +} From fb89cdc6084ea1895d83af5d74c2b096e60fdd36 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Thu, 29 Oct 2026 14:45:04 -0400 Subject: [PATCH 3/4] app: sniff through the safe regular-file opener; find relative change keys Review fixes: the binary sniff reads through skills.ReadRegularHead, which opens nonblocking and checks the descriptor, so a FIFO or device put in a file's place between the stat and the open cannot hold the request. A change recorded under the path relative to the workspace (how a tool call usually names a file) is found for either spelling of the same file. Co-Authored-By: Claude Opus 5.5 --- cmd/codeaf/appconvo.go | 30 ++++++++++++++++++------------ cmd/codeaf/appconvo_alias_test.go | 8 ++++++++ 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/cmd/codeaf/appconvo.go b/cmd/codeaf/appconvo.go index 1e766f736b..d179f30e98 100644 --- a/cmd/codeaf/appconvo.go +++ b/cmd/codeaf/appconvo.go @@ -35,6 +35,7 @@ import ( "github.com/Agent-Field/codeaf/internal/redact" "github.com/Agent-Field/codeaf/internal/remote" "github.com/Agent-Field/codeaf/internal/session" + "github.com/Agent-Field/codeaf/internal/skills" "github.com/Agent-Field/codeaf/internal/tui3" ) @@ -979,16 +980,19 @@ func (b *appBackend) statFile(workspace string, changed map[string]appwire.FileC // /tmp/demo. The spelled test stays first, so nothing inside by spelling is // newly called outside; only a path outside by spelling is resolved, and it // is inside when its resolved place is. same is that file in the - // workspace's own spelling, the key a change recorded there is under. - same := full - if !appWithin(workspace, full) { + // workspace's own spelling and rel its path inside the workspace: the keys a + // change recorded under another spelling of it is found by. + same, rel := full, "" + if appWithin(workspace, full) { + rel, _ = filepath.Rel(workspace, full) + } else { root, place := appCanonical(workspace), appCanonical(full) - rel, err := filepath.Rel(root, place) + inside, err := filepath.Rel(root, place) if err != nil || !appWithin(root, place) { stat.State = appwire.FileOutside return stat } - same = filepath.Join(workspace, rel) + same, rel = filepath.Join(workspace, inside), inside } info, err := os.Stat(full) if err != nil { @@ -997,14 +1001,12 @@ func (b *appBackend) statFile(workspace string, changed map[string]appwire.FileC } size := info.Size() stat.Size = &size - // Only a regular file is opened for its first bytes: opening a FIFO or a - // device would wait for a writer and hold the request. + // The first bytes are read only from a regular file, through the opener + // that also refuses a FIFO or a device put in its place between the stat + // and the open: opening either would wait for a writer and hold the request. if info.Mode().IsRegular() { - if file, err := os.Open(full); err == nil { - head := make([]byte, 8000) - n, _ := io.ReadFull(file, head) - _ = file.Close() - stat.Binary = bytes.IndexByte(head[:n], 0) >= 0 + if head, err := skills.ReadRegularHead(full, 8000); err == nil { + stat.Binary = bytes.IndexByte(head, 0) >= 0 } } if change, ok := changed[full]; ok { @@ -1019,6 +1021,10 @@ func (b *appBackend) statFile(workspace string, changed map[string]appwire.FileC stat.State = appwire.FileChanged add, del := change.Add, change.Del stat.Add, stat.Del = &add, &del + } else if change, ok := changed[rel]; ok && rel != "" { + stat.State = appwire.FileChanged + add, del := change.Add, change.Del + stat.Add, stat.Del = &add, &del } return stat } diff --git a/cmd/codeaf/appconvo_alias_test.go b/cmd/codeaf/appconvo_alias_test.go index cb9cd2df09..bfea60aab5 100644 --- a/cmd/codeaf/appconvo_alias_test.go +++ b/cmd/codeaf/appconvo_alias_test.go @@ -99,6 +99,14 @@ func TestAppFileStatFindsAChangeRecordedUnderTheWorkspaceSpelling(t *testing.T) if stat.State != appwire.FileChanged || stat.Add == nil || *stat.Add != 3 || stat.Del == nil || *stat.Del != 1 { t.Fatalf("the change was not found through the other spelling: %+v", stat) } + // A tool call that named the file relative to the workspace records it so. + relative := map[string]appwire.FileChange{"notes.md": {Path: "notes.md", Add: 2, Del: 0}} + for _, spelled := range []string{filepath.Join(real, "notes.md"), filepath.Join(link, "notes.md")} { + stat := backend.statFile(agent.Workspace, relative, spelled) + if stat.State != appwire.FileChanged || stat.Add == nil || *stat.Add != 2 { + t.Fatalf("a change recorded as notes.md was not found for %s: %+v", spelled, stat) + } + } } // A link inside the workspace that points elsewhere keeps the answer it had: From a42d78934146b76f1991f11629dfaa9390041806 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Thu, 29 Oct 2026 19:16:25 -0400 Subject: [PATCH 4/4] docs: number the change entry for #2038 Co-Authored-By: Claude Opus 5.5 --- .../{0-app-file-stat-alias.md => 2038-file-stat-alias.md} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename docs/changes/unreleased/{0-app-file-stat-alias.md => 2038-file-stat-alias.md} (98%) diff --git a/docs/changes/unreleased/0-app-file-stat-alias.md b/docs/changes/unreleased/2038-file-stat-alias.md similarity index 98% rename from docs/changes/unreleased/0-app-file-stat-alias.md rename to docs/changes/unreleased/2038-file-stat-alias.md index e87bda85ed..f1a168d573 100644 --- a/docs/changes/unreleased/0-app-file-stat-alias.md +++ b/docs/changes/unreleased/2038-file-stat-alias.md @@ -1,7 +1,7 @@ --- kind: fixed title: A desktop file chip in the chat's folder is not called outside through a linked path -pr: 0 +pr: 2038 surface: [engine] invalidates: - "A file inside the chat's workspace read as outside this workspace when the file and the workspace were spelled through different links, such as /private/tmp/demo and /tmp/demo on macOS. Containment now also compares resolved paths."