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 `