From 9da91df8394302bafd6bf5ab93b25cf289b803c9 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 11 Sep 2026 12:00:28 -0400 Subject: [PATCH 1/2] sessions: an anchor writes a whole identity, so an older snapshot cannot undo it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit dev went red on the push of 1d48822b2 (#876): the light gate stayed green and `touched packages` failed on one test in internal/session, TestOldNamingAndSpendSnapshotsPreserveAnAnchoredWorkspace, which is #695's law that an old snapshot never undoes a workspace anchored after it was taken. #695 (4f8d20a5a) made a conversation's identity a transaction under the folder's own lock. #876 (1d48822b2) moved the opening message's stamp off the person's path, so it is owed to offpath.Write rather than written on the way through. Before that, the first writer of any conversation's meta.json was always that stamp; after it, the workspace anchor can be the first writer — and anchorMetaLocked saved what LoadMeta had just handed it, which for a missing file is a blank Meta. The file it left had no id, LoadMeta answers an id-less file with a blank Meta exactly as it answers a missing one, and the next transaction carrying a snapshot taken before the anchor took updateMeta's seeding branch and wrote the pre-anchor ownership and workspace back over it. #695's contract still holds — #876 changed when the stamp reaches the disk, not what a metadata transaction owes — so the test is untouched and the door is fixed: a blank read is filled whole before the anchor replaces its two fields, and a file that already holds an identity keeps every field this transaction does not own. Two tests pin both halves without depending on the scheduler. The failure is scheduling-sensitive, not environment-sensitive: -cpu=1 reproduces it on any box and TMPDIR makes no difference. Co-Authored-By: Claude Fable 5.1 --- .../903-dev-red-anchored-workspace.md | 19 +++++ internal/session/anchor_workspace_test.go | 84 +++++++++++++++++++ internal/session/tools_anchor_workspace.go | 13 +++ 3 files changed, 116 insertions(+) create mode 100644 docs/changes/unreleased/903-dev-red-anchored-workspace.md diff --git a/docs/changes/unreleased/903-dev-red-anchored-workspace.md b/docs/changes/unreleased/903-dev-red-anchored-workspace.md new file mode 100644 index 0000000000..001eb73946 --- /dev/null +++ b/docs/changes/unreleased/903-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: 903 +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) From 4aaa7aa521baaa2a37df82078c57223955151788 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 11 Sep 2026 12:10:39 -0400 Subject: [PATCH 2/2] changelog: number the entry for #904 Co-Authored-By: Claude Fable 5.1 --- ...-anchored-workspace.md => 904-dev-red-anchored-workspace.md} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename docs/changes/unreleased/{903-dev-red-anchored-workspace.md => 904-dev-red-anchored-workspace.md} (99%) diff --git a/docs/changes/unreleased/903-dev-red-anchored-workspace.md b/docs/changes/unreleased/904-dev-red-anchored-workspace.md similarity index 99% rename from docs/changes/unreleased/903-dev-red-anchored-workspace.md rename to docs/changes/unreleased/904-dev-red-anchored-workspace.md index 001eb73946..08fb1b71a1 100644 --- a/docs/changes/unreleased/903-dev-red-anchored-workspace.md +++ b/docs/changes/unreleased/904-dev-red-anchored-workspace.md @@ -1,7 +1,7 @@ --- kind: fixed title: A conversation's anchor writes a whole identity, so an older snapshot cannot undo it -pr: 903 +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."