diff --git a/cmd/codeaf/chatv3_layout.go b/cmd/codeaf/chatv3_layout.go index 11beaea45f..d12f64c081 100644 --- a/cmd/codeaf/chatv3_layout.go +++ b/cmd/codeaf/chatv3_layout.go @@ -82,14 +82,32 @@ func encodeWorkspace(workspace string) string { // explicit workspace — the engine's --workspace, a door that has already // resolved one — is taken as given and is always borrowed: it is a place // somebody named, and naming a place is asking to work in it. +// +// A PROJECT IS NAMED BY THE FOLDER, NOT A SPELLING OF IT. Every answer here is +// a key this surface reads and writes sessions, tasks and memory under, so the +// spelling a terminal carried into the directory (a path reached through a +// symlink) may not leak into it — the engine's door resolved (#1761), and a +// bucket minted from the alias here would still have been a second project, +// a second history, for one folder. A path that does not resolve keeps its +// spelling. func v3Workspace(cwd, explicit string) (string, bool) { if named := strings.TrimSpace(explicit); named != "" { - return named, false + return v3CanonicalPlace(named), false } if root, found := v3GitRoot(cwd); found { - return root, false + return v3CanonicalPlace(root), false + } + return v3CanonicalPlace(cwd), v3NoProjectPlace(cwd) +} + +// v3CanonicalPlace resolves the links in a path that is about to become a key, +// and hands back the spelling it was given when the path does not resolve — a +// workspace not on disk yet, a test's imagined directory. +func v3CanonicalPlace(dir string) string { + if resolved, err := filepath.EvalSymlinks(dir); err == nil { + return resolved } - return cwd, v3NoProjectPlace(cwd) + return dir } // v3NoProjectPlace reports whether a directory is somewhere the person stood by @@ -276,6 +294,19 @@ func v3ResolveSession(explicit, workspace, launchDir string, owned bool) (v3Sess // happened. return v3Session{Place: place, Transcript: place.Transcript(), Resumed: resumed, Bucket: bucket}, nil } + + // A CONVERSATION AN OLDER BUILD FILED UNDER ANOTHER SPELLING IS STILL THIS + // WORKSPACE'S CONVERSATION. Before the project key was resolved (#1761), + // buckets were minted from whatever spelling the terminal carried, and an + // upgrade that reads only the canonical bucket would open a fresh journal + // beside every conversation a person already had. The newest spoken folder + // whose recorded workspace resolves to this folder resumes where it lives — + // no merge, no move; the canonical bucket simply takes over from here. + if legacy, legacyBucket, found := v3LegacyFolder(workspace); found { + place := v3PlaceFor(legacy.dir, workspace, owned || legacy.meta.Owned) + return v3Session{Place: place, Transcript: place.Transcript(), Resumed: true, Bucket: legacyBucket}, nil + } + place, err := v3MintSession(bucket, workspace, launchDir, owned) if err != nil { return v3Session{}, err @@ -284,6 +315,58 @@ func v3ResolveSession(explicit, workspace, launchDir string, owned bool) (v3Sess return v3Session{Place: place, Transcript: place.Transcript(), Bucket: bucket}, nil } +// v3LegacyFolder searches the project buckets OTHER than this workspace's own +// for the newest conversation somebody has spoken in whose recorded workspace +// ([session.Meta.Workspace]) resolves to the same folder, and answers it with +// the bucket it lives in. It is the upgrade road for buckets minted before the +// project key was resolved (#1761): the bucket name is a spelling's encoding and +// cannot be reversed, but every folder cites its own workspace, and a citation +// that resolves to this folder is this folder's conversation. Empty folders and +// folders that cite nothing are not candidates — a resume must come back to +// words, and a citation that was never written cannot be trusted to spellings. +func v3LegacyFolder(workspace string) (v3Folder, string, bool) { + workspace = strings.TrimSpace(workspace) + if workspace == "" { + return v3Folder{}, "", false + } + resolved, err := filepath.EvalSymlinks(filepath.Clean(workspace)) + if err != nil { + return v3Folder{}, "", false + } + root := home.Join("v3", "projects") + entries, err := os.ReadDir(root) + if err != nil { + return v3Folder{}, "", false + } + canonical := filepath.Join(root, encodeWorkspace(workspace)) + var newest v3Folder + newestBucket := "" + for _, entry := range entries { + if !entry.IsDir() { + continue + } + bucket := filepath.Join(root, entry.Name()) + if bucket == canonical { + continue + } + spoken, _ := v3ScanBucket(bucket) + for _, folder := range spoken { + recorded := strings.TrimSpace(folder.meta.Workspace) + if recorded == "" { + continue + } + resolvedRecorded, err := filepath.EvalSymlinks(filepath.Clean(recorded)) + if err != nil || resolvedRecorded != resolved { + continue + } + if newestBucket == "" || folder.at.After(newest.at) { + newest, newestBucket = folder, bucket + } + } + } + return newest, newestBucket, newestBucket != "" +} + // v3PickFolder resumes the newest conversation somebody has spoken in. A // writer mints a fresh identity when there are only unused conversations: an // old empty identity could be assigned to a team between discovery and opening. @@ -326,6 +409,11 @@ func v3LatestTranscript(workspace string) string { spoken, empty := v3ScanBucket(bucket) folder, _, found := v3PickFolder(spoken, empty, false) if !found { + // A CONVERSATION AN OLDER BUILD FILED UNDER ANOTHER SPELLING IS STILL + // THIS WORKSPACE'S CONVERSATION ([v3LegacyFolder]). + if legacy, _, found := v3LegacyFolder(workspace); found { + return session.Place{Dir: legacy.dir}.Transcript() + } return "" } return session.Place{Dir: folder.dir}.Transcript() diff --git a/cmd/codeaf/chatv3_legacy_test.go b/cmd/codeaf/chatv3_legacy_test.go new file mode 100644 index 0000000000..03f7cc929b --- /dev/null +++ b/cmd/codeaf/chatv3_legacy_test.go @@ -0,0 +1,70 @@ +package main + +import ( + "os" + "path/filepath" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/home" + "github.com/Agent-Field/codeaf/internal/remote" + "github.com/Agent-Field/codeaf/internal/session" +) + +// THE CONVERSATIONS AN OLDER BUILD FILED UNDER ANOTHER SPELLING ARE STILL THIS +// FOLDER'S CONVERSATIONS. The upgrade regression for the resolved project key: +// a bucket minted before #1761 was named by the raw spelling the terminal +// carried, and a launch that reads only the canonical bucket would open a fresh +// journal beside every conversation a person already had. The newest spoken +// folder whose recorded workspace resolves to this folder resumes where it +// lives; a comprehensive merge of historical buckets is not required, and none +// is attempted here. +func TestAConversationInALegacyBucketStillResumes(t *testing.T) { + t.Setenv("CODEAF_HOME", filepath.Join(t.TempDir(), "state")) + target := t.TempDir() + link := filepath.Join(t.TempDir(), "linkedrepo") + if err := os.Symlink(target, link); err != nil { + t.Fatal(err) + } + resolved, err := filepath.EvalSymlinks(link) + if err != nil { + t.Fatal(err) + } + + // The old build wrote the bucket under the alias and cited the alias in the + // folder's own meta — the citation is the identity, the bucket name is not. + legacyBucket := filepath.Join(home.Join("v3", "projects"), encodeWorkspace(link)) + held := writeV3Session(t, legacyBucket, "00000000000000e1", "this morning", time.Now().Add(-time.Hour)) + meta, err := session.LoadMeta(held) + if err != nil { + t.Fatal(err) + } + meta.Workspace = link + if err := session.SaveMeta(held, meta); err != nil { + t.Fatal(err) + } + + // A launch that stands on the resolved folder resumes the held + // conversation, from the bucket it already lives in. + want := filepath.Join(held, "transcript.jsonl") + found, err := v3ResolveSession("", resolved, resolved, false) + if err != nil { + t.Fatal(err) + } + if found.Transcript != want { + t.Fatalf("the launch opened %q, want the conversation the old build filed (%s)", found.Transcript, want) + } + if !found.Resumed { + t.Fatal("a conversation somebody has spoken in resumed as a fresh one") + } + if found.Bucket != legacyBucket { + t.Fatalf("the resumed conversation was filed under %q, want the bucket it lives in %q", found.Bucket, legacyBucket) + } + + // And the engine's hello key — the question asked before anything opens — + // answers with the same conversation, so a plain hello joins it rather than + // booting a second agent onto the journal. + if key := engineHelloKey(remote.Hello{Version: remote.Version, Workspace: resolved}, resolved, ""); key != want { + t.Fatalf("a plain hello keyed to %q, want the held conversation (%s)", key, want) + } +} diff --git a/cmd/codeaf/chatv3_symlink_test.go b/cmd/codeaf/chatv3_symlink_test.go new file mode 100644 index 0000000000..9c61191eca --- /dev/null +++ b/cmd/codeaf/chatv3_symlink_test.go @@ -0,0 +1,55 @@ +package main + +import ( + "os" + "path/filepath" + "testing" +) + +// A PROJECT IS NAMED BY THE FOLDER, NOT A SPELLING OF IT. The surface-side +// regression for issue #1761: the bucket key a launch reads and writes sessions +// under used to carry whatever spelling the terminal walked in with, so one +// folder reached through a link was a second project — a second history — +// beside the same folder named by its resolved path. +func TestAProjectIsNamedByTheFolderNotTheSpelling(t *testing.T) { + target := t.TempDir() + link := filepath.Join(t.TempDir(), "linkedrepo") + if err := os.Symlink(target, link); err != nil { + t.Fatal(err) + } + resolved, err := filepath.EvalSymlinks(link) + if err != nil { + t.Fatalf("EvalSymlinks: %v", err) + } + + // A non-git directory reached through the link keys the same bucket as the + // folder it reaches. + aliasProject, _ := v3Workspace(link, "") + if aliasProject != resolved { + t.Fatalf("the alias spelling became the project: got %q, want %q", aliasProject, resolved) + } + bucketFromAlias, err := v3ProjectDir(aliasProject) + if err != nil { + t.Fatalf("v3ProjectDir(alias): %v", err) + } + bucketFromTarget, err := v3ProjectDir(resolved) + if err != nil { + t.Fatalf("v3ProjectDir(target): %v", err) + } + if bucketFromAlias != bucketFromTarget { + t.Fatalf("one folder was given two buckets: alias=%q target=%q", bucketFromAlias, bucketFromTarget) + } + + // Naming the folder explicitly keys one project either way. + namedAlias, borrowed := v3Workspace("", link) + if namedAlias != resolved || borrowed { + t.Fatalf("an explicit workspace keys %q (borrowed=%v), want %q borrowed", namedAlias, borrowed, resolved) + } + + // And a path that does not resolve keeps its spelling. + imagined := filepath.Join(t.TempDir(), "nowhere-at-all") + kept, _ := v3Workspace(imagined, "") + if kept != imagined { + t.Fatalf("a path that is not on disk was rewritten: got %q, want %q", kept, imagined) + } +} diff --git a/cmd/codeaf/engine.go b/cmd/codeaf/engine.go index 2577e8011c..ce2713b50b 100644 --- a/cmd/codeaf/engine.go +++ b/cmd/codeaf/engine.go @@ -1367,6 +1367,14 @@ func engineWorkspace(path string) (string, error) { } path = strings.TrimSpace(path) if path == "" { + // THE DEFAULT IS THE SAME DOOR AS THE NAMED ONE. Omitting --workspace + // means the home directory, and home reaches the resolution below only + // when somebody TYPES it — an omitted flag returned the raw spelling, so + // a home behind a symlink keyed two hosts depending on how it was said + // (#1761). + if resolved, err := filepath.EvalSymlinks(home); err == nil { + home = resolved + } return home, nil } expanded, err := expandHome(path) @@ -1377,6 +1385,14 @@ func engineWorkspace(path string) (string, error) { expanded = filepath.Join(home, expanded) } expanded = filepath.Clean(expanded) + // A WORKSPACE IS NAMED BY ITS RESOLVED PATH, so a folder reached through a + // symlink keys the same project, conversation history and engine host as + // the folder it points at. Clean alone leaves both spellings distinct and + // sha256(Clean(...)) in enginehost.where then builds one host directory per + // spelling — two engines, two histories, one folder. + if resolved, err := filepath.EvalSymlinks(expanded); err == nil { + expanded = resolved + } info, err := os.Stat(expanded) if err != nil { return "", fmt.Errorf("open %s: %w", expanded, err) diff --git a/cmd/codeaf/engine_stale_test.go b/cmd/codeaf/engine_stale_test.go index 73d4542e8a..22ff9f3a48 100644 --- a/cmd/codeaf/engine_stale_test.go +++ b/cmd/codeaf/engine_stale_test.go @@ -807,11 +807,17 @@ func TestStatusSaysNoneWhenNothingHoldsTheWorkspace(t *testing.T) { shortEngineHome(t) home := t.TempDir() t.Setenv("HOME", home) + // The status door names the workspace by its resolved path (#1761); the + // expectation follows the same spelling rather than the one typed in. + resolved, err := engineWorkspace(home) + if err != nil { + t.Fatal(err) + } var out strings.Builder if err := runEngineStatus(&out, home); err != nil { t.Fatal(err) } - if got := strings.TrimSpace(out.String()); got != "no engine is holding "+home+" on this machine" { + if got := strings.TrimSpace(out.String()); got != "no engine is holding "+resolved+" on this machine" { t.Fatalf("status of an empty workspace said %q", got) } } diff --git a/cmd/codeaf/engine_symlink_test.go b/cmd/codeaf/engine_symlink_test.go new file mode 100644 index 0000000000..17646fbf1a --- /dev/null +++ b/cmd/codeaf/engine_symlink_test.go @@ -0,0 +1,147 @@ +package main + +import ( + "os" + "path/filepath" + "testing" + + "github.com/Agent-Field/codeaf/internal/buildinfo" + "github.com/Agent-Field/codeaf/internal/remote" +) + +// TestEngineWorkspaceResolvesSymlink is the regression for issue #1761: a +// folder reached through a symlink must key to the same workspace as the +// folder it points at, so one folder cannot become two projects and two +// engines. Before the fix engineWorkspace stopped at filepath.Clean, so the +// alias and the target returned different paths and enginehost.where hashed +// each spelling to its own host directory. +func TestEngineWorkspaceResolvesSymlink(t *testing.T) { + tmp := t.TempDir() + real := filepath.Join(tmp, "realrepo") + if err := os.Mkdir(real, 0o755); err != nil { + t.Fatal(err) + } + link := filepath.Join(tmp, "linkedrepo") + if err := os.Symlink(real, link); err != nil { + t.Fatal(err) + } + + fromTarget, err := engineWorkspace(real) + if err != nil { + t.Fatalf("engineWorkspace(target): %v", err) + } + fromLink, err := engineWorkspace(link) + if err != nil { + t.Fatalf("engineWorkspace(link): %v", err) + } + + resolvedLink, err := filepath.EvalSymlinks(link) + if err != nil { + t.Fatalf("EvalSymlinks(link): %v", err) + } + + if fromLink != resolvedLink { + t.Fatalf("link spelling not resolved: got %q, want %q", fromLink, resolvedLink) + } + if fromLink != fromTarget { + t.Fatalf("two spellings keyed differently: link=%q target=%q", fromLink, fromTarget) + } +} + +// TestEngineWorkspaceKeepsNonSymlinkPathUnchanged guards that an ordinary +// folder still resolves to itself, so the fix only moves symlinked spellings. +func TestEngineWorkspaceKeepsNonSymlinkPathUnchanged(t *testing.T) { + tmp := t.TempDir() + real := filepath.Join(tmp, "plainrepo") + if err := os.Mkdir(real, 0o755); err != nil { + t.Fatal(err) + } + resolvedReal, err := filepath.EvalSymlinks(real) + if err != nil { + t.Fatalf("EvalSymlinks(plain): %v", err) + } + got, err := engineWorkspace(real) + if err != nil { + t.Fatalf("engineWorkspace(plain): %v", err) + } + if got != resolvedReal { + t.Fatalf("plain path changed: got %q, want %q", got, resolvedReal) + } +} + +// TestADialThroughASymlinkMeetsTheHostAtTheFolder is the launch-side regression +// for the review of #1798: the local surface used to hash the spelling the +// person's terminal carried (os.Getwd keeps a symlinked alias), while the +// daemon resolved its --workspace before it listened — so the surface dialed +// one host directory and the daemon was born in another, and the launch fell +// back to a conversation in the terminal. Host lookup, dial and spawn now all +// funnel through the one resolved identity (enginehost.where), so a host +// standing at the folder answers a dial that arrives through any spelling of +// it. +func TestADialThroughASymlinkMeetsTheHostAtTheFolder(t *testing.T) { + shortStateRoot(t) + target := t.TempDir() + link := filepath.Join(t.TempDir(), "linkedrepo") + if err := os.Symlink(target, link); err != nil { + t.Fatal(err) + } + + // A daemon was handed --workspace by a door that resolves (engineWorkspace), + // so a host stands where the FOLDER hashes. Stand one there, speaking this + // build's wire so the stale-host question before the dial is answered. + standIn(t, target, remote.HostSelf{Version: remote.Version, Build: buildinfo.Identity()}, false) + + // The surface reached the folder through the link and dials with the + // spelling it carries. The dial must still meet the host — and the spawn + // callback must never fire, because something is already answering. + linkDial := &localLink{workspace: link} + conn, err := linkDial.dial() + if err != nil { + t.Fatalf("a dial through the symlink missed the host at the folder: %v", err) + } + _ = conn.Close() + + // And the target's own spelling reaches the same place, since one folder is + // one host. + targetDial := &localLink{workspace: target} + conn, err = targetDial.dial() + if err != nil { + t.Fatalf("a dial from the target spelling missed its own host: %v", err) + } + _ = conn.Close() +} + +// TestTheDefaultWorkspaceIsTheSameFolderAsTheNamedHome is the default-home +// half of the #1798 review: engineWorkspace("") used to return home without +// the resolution a TYPED home goes through, so a home behind a symlink keyed +// two hosts depending on whether --workspace was omitted or spelled out. +func TestTheDefaultWorkspaceIsTheSameFolderAsTheNamedHome(t *testing.T) { + target := t.TempDir() + link := filepath.Join(t.TempDir(), "homelink") + if err := os.Symlink(target, link); err != nil { + t.Fatal(err) + } + // os.UserHomeDir reads HOME on unix and USERPROFILE on Windows; name both + // so the default reaches the link on every machine this runs on. + t.Setenv("HOME", link) + t.Setenv("USERPROFILE", link) + + omitted, err := engineWorkspace("") + if err != nil { + t.Fatalf("engineWorkspace(\"\"): %v", err) + } + named, err := engineWorkspace(link) + if err != nil { + t.Fatalf("engineWorkspace(home): %v", err) + } + resolved, err := filepath.EvalSymlinks(link) + if err != nil { + t.Fatalf("EvalSymlinks(home): %v", err) + } + if omitted != resolved { + t.Fatalf("the omitted flag kept the raw home spelling: got %q, want %q", omitted, resolved) + } + if omitted != named { + t.Fatalf("omitting --workspace and naming the same home keyed two workspaces: %q vs %q", omitted, named) + } +} diff --git a/cmd/codeaf/hostlatest_test.go b/cmd/codeaf/hostlatest_test.go index c4e4862efe..ab5e4c53d8 100644 --- a/cmd/codeaf/hostlatest_test.go +++ b/cmd/codeaf/hostlatest_test.go @@ -31,6 +31,13 @@ import ( func TestAPlainHelloKeysToTheConversationTheBootWouldOpen(t *testing.T) { t.Setenv("CODEAF_HOME", filepath.Join(t.TempDir(), "state")) workspace := t.TempDir() + // The fixture models a disk the CURRENT build wrote, and the current build + // names a workspace by its resolved path (#1761): on macOS the temporary + // root is reached through a link, so the fixture stands on the resolved + // spelling before it writes or asks anything. + if resolved, err := filepath.EvalSymlinks(workspace); err == nil { + workspace = resolved + } bucket, err := v3ProjectDir(workspace) if err != nil { t.Fatal(err) @@ -63,6 +70,11 @@ func TestAPlainHelloKeysToTheConversationTheBootWouldOpen(t *testing.T) { func TestAWorkspaceWithNoConversationKeysToNothingAndThenToWhatItOpened(t *testing.T) { t.Setenv("CODEAF_HOME", filepath.Join(t.TempDir(), "state")) workspace := t.TempDir() + // Same resolved fixture as above: the current build keys workspaces by + // their resolved path (#1761), so the fixture does too. + if resolved, err := filepath.EvalSymlinks(workspace); err == nil { + workspace = resolved + } if key := engineHelloKey(remote.Hello{Version: remote.Version, Workspace: workspace}, workspace, ""); key != "" { t.Fatalf("an empty workspace keyed to %s", key) diff --git a/docs/changes/unreleased/1798-symlink-workspace-keys.md b/docs/changes/unreleased/1798-symlink-workspace-keys.md new file mode 100644 index 0000000000..33fed8b76e --- /dev/null +++ b/docs/changes/unreleased/1798-symlink-workspace-keys.md @@ -0,0 +1,37 @@ +--- +kind: fixed +title: a folder reached through a symlink is one project and one engine, not two +pr: 1798 +surface: [engine, chat] +invalidates: + - "A folder reached through a symlink became two projects, two conversation histories and two engines. `codeaf engine --status --workspace ` missed the engine holding ``, and home's projects panel listed the folder once per spelling. The key-minting doors stopped at filepath.Clean, so each spelling hashed to its own key. Every door that turns a path into a key now resolves symlinks first (filepath.EvalSymlinks, with the original spelling kept when the path does not resolve): engineWorkspace (the engine commands and the host's workspace name, including the default home), enginehost.where (the host directory every lookup, dial, spawn, lock and status is keyed by), v3Workspace (the session bucket and tools root), gitidentity's path fallback (the memory project key), and the typed /project pin. Both spellings of one folder reach one project, one conversation history and one engine." +--- + +Found in the v0.7.1 happy-path pass on macOS, where `/tmp` is a link to +`/private/tmp`, and reproducible on any system with `mkdir d && ln -s d l`. +`engineWorkspace` — the door every engine command (`--daemon`, `--status`, +`--stop`) and the host's workspace name go through — expanded and cleaned the +path but never resolved its links, so two spellings of one folder keyed two +host directories in `internal/enginehost.where` (sha256 of the cleaned string), +and the person saw one folder while codeaf kept two of everything. + +The resolution was first added at `engineWorkspace` alone, and the review of +this pull request caught what that left split: the local surface still hashed +the alias its terminal carried while the daemon listened under the resolved +key, and the default home skipped the door entirely. The identity now resolves +at every layer that mints a key — the host directory inside `where` itself, so +lookup, dial, spawn, lock and status cannot disagree; the default home in +`engineWorkspace`; the session bucket and memory project key in `cmd/codeaf`; +the typed `/project` pin in `internal/tui3`. A regression test drives a symlink +through each layer and asserts both spellings land on one key. + +THE UPGRADE KEEPS WHAT THE OLD KEY WROTE. A host directory and a conversation +bucket an older build named by the raw spelling are still this folder's: a +`Dial` that misses the canonical key looks for host directories whose recorded +workspace (`internal/enginehost`'s place file) resolves to the same folder, so +`--status`, `--stop`, `--status-all`, `--stop-all` and the takeover rule reach +an engine the upgrade did not start; and a launch whose canonical bucket holds +no spoken conversation resumes the newest folder in another bucket whose +`meta.workspace` citation resolves to it. No migration, no merge — the old +records are read where they live, and the canonical key takes over from there. + diff --git a/internal/enginehost/enginehost.go b/internal/enginehost/enginehost.go index 0c34b3b7dc..313ccd18ec 100644 --- a/internal/enginehost/enginehost.go +++ b/internal/enginehost/enginehost.go @@ -125,12 +125,24 @@ func Dir(workspace string) (string, error) { // that a question was asked. The doors that are about to WRITE something — the // host's own listener, the spawn lock, the wait for a host to go — call [Dir] // and make it themselves. +// +// A HOST DIRECTORY IS NAMED BY THE FOLDER, NOT A SPELLING OF IT. Every identity +// question in this package — host lookup, dial, spawn, lock, status, stop — +// funnels through here, so this is the one place a symlinked spelling must be +// resolved: a surface that reached the folder through a link and a daemon that +// was handed the resolved path would otherwise hash to two host directories and +// spend the launch failing to meet (#1761). A path that does not resolve — a +// workspace not on disk yet, a test's imagined directory — keeps its spelling. func where(workspace string) (string, error) { workspace = strings.TrimSpace(workspace) if workspace == "" { return "", errors.New("engine host: no workspace to hold") } - sum := sha256.Sum256([]byte(filepath.Clean(workspace))) + workspace = filepath.Clean(workspace) + if resolved, err := filepath.EvalSymlinks(workspace); err == nil { + workspace = resolved + } + sum := sha256.Sum256([]byte(workspace)) return home.Join("v3", "hosts", hex.EncodeToString(sum[:8])), nil } @@ -151,12 +163,86 @@ func SocketPath(workspace string) (string, error) { // Dial connects to a host that is already running, and fails when none is. It // never starts one — see [Attach] for that. +// +// THE HOST AN OLDER BUILD WROTE FOR THIS FOLDER IS STILL THIS FOLDER'S HOST. +// The directory a host lives in is named by a hash of the workspace, and once +// that hash was taken from the resolved spelling (#1761), a directory an older +// build named by the raw spelling it was handed went dark for every door here: +// [Held] reads what was written inside, but Dial, Stop, the status door and the +// takeover rule ask by key. A directory whose recorded workspace ([placeName]) +// resolves to the same folder is this workspace's host whoever wrote it, so a +// canonical miss is answered by the legacy directories before this gives up — +// which is what keeps an upgrade from orphaning a running engine, its +// conversations and the commands that must still stop it. The canonical error +// is the one handed back when nothing anywhere answers, so the retry semantics +// above this door do not move. func Dial(workspace string) (net.Conn, error) { socket, err := SocketPath(workspace) if err != nil { return nil, err } - return net.DialTimeout("unix", socket, dialTimeout) + conn, dialErr := net.DialTimeout("unix", socket, dialTimeout) + if dialErr == nil { + return conn, nil + } + for _, legacy := range legacySockets(workspace) { + if conn, err := net.DialTimeout("unix", legacy, dialTimeout); err == nil { + return conn, nil + } + } + return nil, dialErr +} + +// legacySockets names the live sockets of host directories an older build wrote +// for this workspace — directories whose recorded workspace resolves to the +// same folder — and none when there are no such directories or the workspace +// itself does not resolve. It reads the state root and follows links, and makes +// nothing. +func legacySockets(workspace string) []string { + workspace = strings.TrimSpace(workspace) + if workspace == "" { + return nil + } + resolved, err := filepath.EvalSymlinks(filepath.Clean(workspace)) + if err != nil { + return nil + } + root := home.Join("v3", "hosts") + entries, err := os.ReadDir(root) + if err != nil { + return nil + } + canonical, err := where(workspace) + if err != nil { + return nil + } + var sockets []string + for _, entry := range entries { + if !entry.IsDir() { + continue + } + dir := filepath.Join(root, entry.Name()) + if dir == canonical { + continue + } + name, err := os.ReadFile(filepath.Join(dir, placeName)) + if err != nil { + continue + } + recorded := strings.TrimSpace(string(name)) + if recorded == "" { + continue + } + resolvedRecorded, err := filepath.EvalSymlinks(filepath.Clean(recorded)) + if err != nil || resolvedRecorded != resolved { + continue + } + socket := filepath.Join(dir, socketName) + if SocketPathFits(socket) { + sockets = append(sockets, socket) + } + } + return sockets } // dialTimeout is how long one connect attempt may take. A unix socket connects diff --git a/internal/enginehost/enginehost_legacy_test.go b/internal/enginehost/enginehost_legacy_test.go new file mode 100644 index 0000000000..e2a34adf04 --- /dev/null +++ b/internal/enginehost/enginehost_legacy_test.go @@ -0,0 +1,77 @@ +package enginehost + +import ( + "crypto/sha256" + "encoding/hex" + "net" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/Agent-Field/codeaf/internal/home" +) + +// legacyHostDir mints the host directory the way a build BEFORE the resolved +// workspace key (#1761) named it: sha256 of the spelling it was handed. An +// upgrade fixture needs the old name, because that is what the installed base +// left on disk. +func legacyHostDir(t *testing.T, workspace string) string { + t.Helper() + sum := sha256.Sum256([]byte(filepath.Clean(workspace))) + dir := home.Join("v3", "hosts", hex.EncodeToString(sum[:8])) + if err := os.MkdirAll(dir, 0o700); err != nil { + t.Fatalf("make a legacy host directory: %v", err) + } + return dir +} + +// THE HOST AN OLDER BUILD WROTE FOR THIS FOLDER IS STILL THIS FOLDER'S HOST. +// The upgrade regression for the resolved workspace key: a host directory named +// by the raw spelling an older build was handed — a link, the way a terminal +// carries it — stays reachable by lookup, so Dial meets it, and with it Stop, +// the status door and the takeover rule keep reaching an engine the upgrade did +// not start. Before the legacy lookup, Held listed the engine and Dial missed +// its live socket, which is an upgrade that strands a running engine. +func TestADialMeetsAHostAnOlderBuildWroteForTheSameFolder(t *testing.T) { + shortHome(t) + target := t.TempDir() + link := filepath.Join(t.TempDir(), "linkedrepo") + if err := os.Symlink(target, link); err != nil { + t.Fatal(err) + } + + // The old build was told the link's spelling and hashed that, so its host + // directory carries the old key and cites the spelling it was given. + dir := legacyHostDir(t, link) + if err := os.WriteFile(filepath.Join(dir, placeName), []byte(link+"\n"), 0o600); err != nil { + t.Fatal(err) + } + listener, err := net.Listen("unix", filepath.Join(dir, socketName)) + if err != nil { + t.Fatalf("stand a live socket in the legacy directory: %v", err) + } + t.Cleanup(func() { _ = listener.Close() }) + + // The new build asks by the FOLDER — the resolved spelling. Held already + // answers for the legacy directory; Dial must now reach it too, and the + // plain target spelling with it, since one folder is one host. + if held, err := Held(); err != nil || len(held) != 1 || !strings.HasSuffix(held[0], "linkedrepo") { + t.Fatalf("Held() = %v (%v), want the legacy directory's own citation", held, err) + } + conn, err := Dial(target) + if err != nil { + t.Fatalf("a dial by the resolved folder missed the host an older build wrote: %v", err) + } + _ = conn.Close() + conn, err = Dial(link) + if err != nil { + t.Fatalf("a dial by the legacy spelling missed its own host: %v", err) + } + _ = conn.Close() + + // And a folder that never had a host is still answered with nothing. + if _, err := Dial(filepath.Join(t.TempDir(), "nowhere")); err == nil { + t.Fatal("a workspace with no host anywhere dialed to something") + } +} diff --git a/internal/enginehost/enginehost_symlink_test.go b/internal/enginehost/enginehost_symlink_test.go new file mode 100644 index 0000000000..68c07ac89e --- /dev/null +++ b/internal/enginehost/enginehost_symlink_test.go @@ -0,0 +1,57 @@ +package enginehost + +import ( + "os" + "path/filepath" + "testing" +) + +// TestAHostDirectoryIsNamedByTheFolderNotASpelling is the identity-layer +// regression for issue #1761 and the review of #1798: every question this +// package answers — lookup, dial, lock, socket — funnels through [where], and +// where is the one place a symlinked spelling must resolve. Before the fix, a +// surface that reached the folder through a link and a daemon named by the +// resolved path hashed to two host directories, and the launch spent its wait +// failing to meet. +func TestAHostDirectoryIsNamedByTheFolderNotASpelling(t *testing.T) { + shortHome(t) + target := t.TempDir() + link := filepath.Join(t.TempDir(), "linkedrepo") + if err := os.Symlink(target, link); err != nil { + t.Fatal(err) + } + + socketFromTarget, err := SocketPath(target) + if err != nil { + t.Fatalf("SocketPath(target): %v", err) + } + socketFromLink, err := SocketPath(link) + if err != nil { + t.Fatalf("SocketPath(link): %v", err) + } + if socketFromLink != socketFromTarget { + t.Fatalf("two spellings were given two sockets: link=%q target=%q", socketFromLink, socketFromTarget) + } + + lockFromTarget, err := LockPath(target) + if err != nil { + t.Fatalf("LockPath(target): %v", err) + } + lockFromLink, err := LockPath(link) + if err != nil { + t.Fatalf("LockPath(link): %v", err) + } + if lockFromLink != lockFromTarget { + t.Fatalf("two spellings were given two locks: link=%q target=%q", lockFromLink, lockFromTarget) + } + + // And a plain directory still keys to itself: resolving only moves + // spellings that a link carries. + plainSocket, err := SocketPath(filepath.Join(t.TempDir(), "plainrepo")) + if err != nil { + t.Fatalf("SocketPath(plain): %v", err) + } + if plainSocket == socketFromTarget { + t.Fatal("a different workspace was given the same socket") + } +} diff --git a/internal/gitidentity/project.go b/internal/gitidentity/project.go index 3b9c8fff31..7ebce5202f 100644 --- a/internal/gitidentity/project.go +++ b/internal/gitidentity/project.go @@ -43,6 +43,13 @@ func ProjectKey(workspace string) (string, error) { if err != nil { return "", nil } + // THE CANONICAL PATH IS THE RESOLVED ONE. A folder reached through a + // symlink is one project: the alias and the target hash to the same key, or + // memory would hold two owners for one folder (#1761). A path that does not + // resolve keeps the absolute spelling. + if resolved, err := filepath.EvalSymlinks(absolute); err == nil { + return hashKey(resolved), nil + } return hashKey(absolute), nil } diff --git a/internal/gitidentity/project_symlink_test.go b/internal/gitidentity/project_symlink_test.go new file mode 100644 index 0000000000..cd283aca49 --- /dev/null +++ b/internal/gitidentity/project_symlink_test.go @@ -0,0 +1,37 @@ +package gitidentity + +import ( + "os" + "path/filepath" + "testing" +) + +// A folder reached through a symlink is one project: the alias and the target +// hash to the same key, or memory holds two owners for one folder (#1761). A +// workspace that is not on disk keeps the absolute spelling — the key is still +// minted, because a project with no key quarantines its writes. +func TestThePathFallbackKeysOneFolderAcrossItsSpellings(t *testing.T) { + target := t.TempDir() + link := filepath.Join(t.TempDir(), "linkedrepo") + if err := os.Symlink(target, link); err != nil { + t.Fatal(err) + } + + fromTarget, err := ProjectKey(target) + if err != nil { + t.Fatalf("ProjectKey(target): %v", err) + } + fromLink, err := ProjectKey(link) + if err != nil { + t.Fatalf("ProjectKey(link): %v", err) + } + if fromLink != fromTarget { + t.Fatalf("two spellings of one folder keyed two projects: link=%q target=%q", fromLink, fromTarget) + } + + imagined := filepath.Join(t.TempDir(), "nowhere-at-all") + key, err := ProjectKey(imagined) + if err != nil || key == "" { + t.Fatalf("a workspace that is not on disk minted no key: %q, %v", key, err) + } +} diff --git a/internal/tui3/homefate_test.go b/internal/tui3/homefate_test.go index d5fafdcb48..81ef9b7966 100644 --- a/internal/tui3/homefate_test.go +++ b/internal/tui3/homefate_test.go @@ -190,19 +190,25 @@ func TestTheStoreLandingKeepsHomesSheetAboutTheNextConversation(t *testing.T) { // /project WITH A PATH TAKES THE PATH AND OPENS NOTHING. A person who typed the // folder has already answered the question the browser exists to ask, and the // pin is a string on this window rather than a round trip — so the keys row -// says the new folder on the very next frame. +// says the new folder on the very next frame. The pin names the FOLDER the path +// reaches, not the spelling that was typed (#1761), so the expectation resolves +// the fixture path the same way the pin does. func TestProjectWithAPathAtHomePinsItWithoutTheBrowser(t *testing.T) { a, _, root := mixedLab(t) runCmd(a.openHome()) inner := filepath.Join(root, "here", "inner") runCmd(a.homeSlash("/project " + inner)) + pinned, err := filepath.EvalSymlinks(inner) + if err != nil { + t.Fatal(err) + } if a.folder.open { t.Fatal("/project opened the browser instead of taking the path") } - if a.target.where != inner { - t.Fatalf("/project pinned %q, want %q", a.target.where, inner) + if a.target.where != pinned { + t.Fatalf("/project pinned %q, want %q", a.target.where, pinned) } if !a.at(pageHome) { t.Fatal("/project left home") diff --git a/internal/tui3/projectcmd.go b/internal/tui3/projectcmd.go index bf6c882cc3..1bc3f8264a 100644 --- a/internal/tui3/projectcmd.go +++ b/internal/tui3/projectcmd.go @@ -22,6 +22,7 @@ package tui3 import ( "os" + "path/filepath" "strings" tea "charm.land/bubbletea/v2" @@ -75,6 +76,16 @@ func (a *app) runProjectCommand(rest string) tea.Cmd { return nil } path := a.resolvePath(rest) + // A PIN NAMES A FOLDER, NOT A SPELLING. A path taken through a symlink must + // pin the folder the link reaches, or home stores one project under the + // alias while a launch from the resolved path stores another — two + // projects, two histories, two engines for one folder (#1761). The stat + // below already pays the walk EvalSymlinks costs on a missing target, so a + // path that does not resolve simply falls through to the refusal word as + // it always did. + if resolved, err := filepath.EvalSymlinks(path); err == nil { + path = resolved + } info, err := os.Stat(path) if path == "" || err != nil || !info.IsDir() { a.home.say(projectNoFolderWord+rest, "") diff --git a/internal/tui3/projectcmd_symlink_test.go b/internal/tui3/projectcmd_symlink_test.go new file mode 100644 index 0000000000..6d9aa2bfc7 --- /dev/null +++ b/internal/tui3/projectcmd_symlink_test.go @@ -0,0 +1,46 @@ +package tui3 + +import ( + "os" + "path/filepath" + "testing" +) + +// A PIN NAMES THE FOLDER THE LINK REACHES, NOT THE SPELLING THAT WAS TYPED. +// The typed /project regression for issue #1761: a path taken through a symlink +// used to pin the alias, so home held one project under the link's name while a +// launch from the resolved path held another — two projects, two histories, two +// engines for one folder. The pin now resolves before it lands. +func TestProjectPinResolvesASymlinkedSpelling(t *testing.T) { + a, _, root := mixedLab(t) + runCmd(a.openHome()) + + real := filepath.Join(root, "realrepo") + if err := os.Mkdir(real, 0o755); err != nil { + t.Fatal(err) + } + link := filepath.Join(root, "linkedrepo") + if err := os.Symlink(real, link); err != nil { + t.Fatal(err) + } + + runCmd(a.homeSlash("/project " + link)) + + resolved, err := filepath.EvalSymlinks(link) + if err != nil { + t.Fatalf("EvalSymlinks: %v", err) + } + if a.target.where != resolved { + t.Fatalf("/project through a symlink pinned %q, want the folder it reaches %q", a.target.where, resolved) + } + + // And the folder's own spelling pins the same place, since one folder is + // one project. + runCmd(a.homeSlash("/project " + real)) + if a.target.where != resolved { + t.Fatalf("/project from the target pinned %q, want %q", a.target.where, resolved) + } + if a.home.msg != "" { + t.Fatalf("a taken path wrote %q over home's keys row", a.home.msg) + } +}