diff --git a/docs/changes/unreleased/904-dev-red-anchored-workspace.md b/docs/changes/unreleased/904-dev-red-anchored-workspace.md new file mode 100644 index 0000000000..08fb1b71a1 --- /dev/null +++ b/docs/changes/unreleased/904-dev-red-anchored-workspace.md @@ -0,0 +1,19 @@ +--- +kind: fixed +title: A conversation's anchor writes a whole identity, so an older snapshot cannot undo it +pr: 904 +surface: [chat, engine] +invalidates: + - "`/workspace` and the model's `workspace` tool wrote meta.json by reading it, replacing two fields and saving what came back. Since #876 deferred the opening message's stamp, that read can be of a file that does not exist yet — and the anchor then saved an identity-less meta.json, which [LoadMeta] answers with a blank Meta exactly as it answers a missing file. The anchoring was on disk in a record nothing could read. The anchor now fills the identity whole when its read came back blank." + - "A metadata transaction carrying a snapshot taken BEFORE an anchor then met that identity-less file, took [Agent.updateMeta]'s seeding branch and wrote the snapshot's ownership and workspace back over the anchoring: a conversation the person had pointed at a project came back project-less. #695's contract is unchanged and holds; what was wrong was the door that left a file the transaction had to rebuild." + - "An anchor over a conversation that ALREADY has an identity still writes exactly the two fields it owns — the workspace and the ownership — and nothing else. The totals, the name, the model and the folders another window wrote are left where they are, which is what the folder's lock is for (#695)." +--- + +dev went red on the push of #876 with one test in `internal/session`: +`TestOldNamingAndSpendSnapshotsPreserveAnAnchoredWorkspace`, which is #695's law +that an old snapshot never undoes a workspace anchored after it was taken. The +failure is scheduling-sensitive rather than environment-sensitive — the deferred +stamp loses on a loaded runner a race it wins on an idle laptop, and `-cpu=1` +reproduces it every time — so what changed was not the contract but which writer +reaches meta.json first. #876 moved WHEN the stamp lands on the disk; it did not +change what a metadata transaction owes. diff --git a/internal/session/anchor_workspace_test.go b/internal/session/anchor_workspace_test.go index ad67968a22..a5176ce8f4 100644 --- a/internal/session/anchor_workspace_test.go +++ b/internal/session/anchor_workspace_test.go @@ -11,6 +11,90 @@ import ( "github.com/Agent-Field/aforge-v2/internal/exec/bare" ) +// An anchor can be the first writer of a conversation's identity now that the +// opening-message stamp lands behind the person's path. It must write a whole +// identity, and an older metadata transaction must preserve that anchoring. +func TestAnchoringBeforeTheFirstStampWritesAWholeIdentity(t *testing.T) { + dir := filepath.Join(t.TempDir(), "0123456789abcdef") + place := Place{Dir: dir, Owned: true} + place.Workspace = place.Work() + if err := os.MkdirAll(place.Work(), 0o700); err != nil { + t.Fatal(err) + } + agent, err := newAgent(Config{ + Workspace: place.Work(), Place: place, SessionFile: place.Transcript(), Model: "test/model", + }, &scriptedCompleter{}) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = agent.Close() }) + + if _, err := os.Stat(place.MetaPath()); !os.IsNotExist(err) { + t.Fatalf("new conversation already has meta.json: %v", err) + } + stale := agent.metaSnapshot() + anchored, err := agent.AnchorWorkspace(t.TempDir()) + if err != nil { + t.Fatal(err) + } + meta, err := LoadMeta(dir) + if err != nil || meta.ID == "" || meta.Workspace != anchored || meta.Owned { + raw, _ := os.ReadFile(place.MetaPath()) + t.Fatalf("first anchor wrote an unreadable identity: %+v %v\n%s", meta, err, raw) + } + + agent.updateMeta(dir, stale, func(meta *Meta) { meta.Title = "parser migration failures" }) + agent.writeSpendSnapshot(dir, stale, .5, 270) + meta, err = LoadMeta(dir) + if err != nil || meta.Owned || meta.Workspace != anchored || meta.Title != "parser migration failures" || meta.Tokens != 270 { + t.Fatalf("old snapshot undid first-write anchoring: %+v %v", meta, err) + } +} + +// And an anchor over a conversation that already has one owns its two fields and +// no others: the totals and the model another writer put there are still there +// afterwards, which is the whole point of the folder's lock (#695). +func TestAnchoringOverAWrittenIdentityKeepsTheFieldsItDoesNotOwn(t *testing.T) { + dir := filepath.Join(t.TempDir(), "0123456789abcdef") + place := Place{Dir: dir, Owned: true} + place.Workspace = place.Work() + if err := os.MkdirAll(place.Work(), 0o700); err != nil { + t.Fatal(err) + } + agent, err := newAgent(Config{ + Workspace: place.Work(), Place: place, SessionFile: place.Transcript(), Model: "test/model", + }, &scriptedCompleter{}) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = agent.Close() }) + agent.mu.Lock() + agent.stampUserLocked("diagnose parser migration failures") + agent.mu.Unlock() + agent.SettleWrites() + + written, err := LoadMeta(dir) + if err != nil || written.ID == "" { + t.Fatalf("the stamp left no identity to anchor over: %+v %v", written, err) + } + written.Model, written.SpentUSD, written.Tokens = "other/model", .5, 270 + if err := SaveMeta(dir, written); err != nil { + t.Fatal(err) + } + + anchored, err := agent.AnchorWorkspace(t.TempDir()) + if err != nil { + t.Fatal(err) + } + meta, err := LoadMeta(dir) + if err != nil || meta.Workspace != anchored || meta.Owned { + t.Fatalf("the anchor did not land: %+v %v", meta, err) + } + if meta.Model != "other/model" || meta.Tokens != 270 || meta.SpentUSD != .5 { + t.Fatalf("the anchor wrote over a field it does not own: %+v", meta) + } +} + func TestAnchoringAnOwnedSessionPersistsAndReloadsProjectInstructions(t *testing.T) { repo := newTestRepo(t) if err := os.Mkdir(filepath.Join(repo, "subdir"), 0o755); err != nil { diff --git a/internal/session/tools_anchor_workspace.go b/internal/session/tools_anchor_workspace.go index 446c8b3202..be9778b8a2 100644 --- a/internal/session/tools_anchor_workspace.go +++ b/internal/session/tools_anchor_workspace.go @@ -111,6 +111,19 @@ func (a *Agent) anchorMetaLocked(dir, workspace string) error { if err != nil { return fmt.Errorf("read conversation place: %w", err) } + // AN ANCHOR CAN BE THE FIRST WRITER OF THIS FILE, now that the opening + // message's stamp is owed behind the person's path rather than written on + // it (placemeta.go's [Agent.metaStamp]). [LoadMeta] answers an identity-less + // meta.json with a blank Meta exactly as it answers a missing one, so an + // anchor that saved back what it had just read would leave the anchoring in + // a file nothing can read — and the next transaction carrying a snapshot + // taken before the anchor would seed ownership and workspace out of that + // snapshot and undo it. A blank read is therefore filled whole here. A file + // that already holds an identity is left alone: this transaction owns two + // fields, and every other one belongs to whoever wrote it. + if strings.TrimSpace(meta.ID) == "" { + meta = a.fillMetaLocked(meta) + } meta.Workspace, meta.Owned = workspace, false if err := SaveMeta(dir, meta); err != nil { return fmt.Errorf("save conversation place: %w", err)