Skip to content

Commit 67f2cfd

Browse files
AbirAbbasclaude
andcommitted
Only a conversation member uses the saved-folder fallback
A blank found on disk shared the listing cache that task and process members use to find their owner, so a collection read could open a crew member's conversation to look for a child that does not exist. The fallback now serves conversation members only, from its own cache. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 48c7ac3 commit 67f2cfd

2 files changed

Lines changed: 204 additions & 62 deletions

File tree

‎cmd/codeaf/appcollectionkeep_test.go‎

Lines changed: 173 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"os"
77
"path/filepath"
88
"reflect"
9+
"sync/atomic"
910
"testing"
1011
"time"
1112

@@ -43,8 +44,6 @@ func TestACollectionHeldBlankIsNeverTheConversationALaunchResumes(t *testing.T)
4344
{name: "only a held blank", held: true},
4445
{name: "an unreadable store beside an older spoken conversation", spoken: true, unreadable: true},
4546
{name: "only a blank with an unreadable store", unreadable: true},
46-
{name: "an unreferenced blank beside an older spoken conversation", spoken: true},
47-
{name: "only an unreferenced blank"},
4847
{name: "a blank held by both a team and a collection", held: true, team: true},
4948
} {
5049
t.Run(test.name, func(t *testing.T) {
@@ -98,13 +97,8 @@ func TestACollectionHeldBlankIsNeverTheConversationALaunchResumes(t *testing.T)
9897
} else if len(spoken) != 0 {
9998
t.Fatalf("a blank became a resume candidate: %+v", spoken)
10099
}
101-
kept := test.held || test.unreadable || test.team
102-
if kept {
103-
if len(empty) != 0 {
104-
t.Fatalf("a kept blank was offered for reuse: %+v", empty)
105-
}
106-
} else if len(empty) != 1 || empty[0].dir != blank {
107-
t.Fatalf("unreferenced blank candidates: %+v", empty)
100+
if len(empty) != 0 {
101+
t.Fatalf("a kept blank was offered for reuse: %+v", empty)
108102
}
109103
folder, resumed, found := v3PickFolder(spoken, empty, true)
110104
if test.spoken {
@@ -117,59 +111,87 @@ func TestACollectionHeldBlankIsNeverTheConversationALaunchResumes(t *testing.T)
117111
want := ""
118112
if test.spoken {
119113
want = session.Place{Dir: conversation}.Transcript()
120-
} else if !kept {
121-
want = session.Place{Dir: blank}.Transcript()
122114
}
123115
if got := v3LatestTranscript(root); got != want {
124116
t.Fatalf("host discovery found %q, want %q", got, want)
125117
}
126118
// Recheck a stale cleanup candidate as well as the scan's own pile.
127119
v3ReapEmpty([]v3Folder{{dir: blank}}, "")
128-
_, err := os.Stat(blank)
129-
if kept && err != nil {
120+
if _, err := os.Stat(blank); err != nil {
130121
t.Fatalf("cleanup removed a kept blank: %v", err)
131122
}
132-
if !kept && !os.IsNotExist(err) {
133-
t.Fatalf("cleanup kept an unreferenced blank: %v", err)
134-
}
135123
})
136124
}
137125
}
138126

139127
func TestABlankConversationCanBeReapedAfterItsLastCollectionReferenceIsRemoved(t *testing.T) {
140-
_, bucket := collectionKeepEnvironment(t)
141-
dir := writeV3Session(t, bucket, "reserved-agent", "", time.Time{})
142-
store, err := workspace.Open(home.Join("v3", "collections.db"))
143-
if err != nil {
144-
t.Fatal(err)
145-
}
146-
t.Cleanup(func() { _ = store.Close() })
147-
collection, err := store.Create(context.Background(), "Reserved")
148-
if err != nil {
149-
t.Fatal(err)
150-
}
151-
ref := workspace.Ref{Kind: workspace.ConversationKind, ID: filepath.Base(dir)}
152-
if err := store.Add(context.Background(), collection.ID, ref); err != nil {
153-
t.Fatal(err)
154-
}
155-
spoken, empty := v3ScanBucket(bucket)
156-
if len(spoken) != 0 || len(empty) != 0 || v3EmptySession(dir) {
157-
t.Fatal("a held blank was offered for resume or cleanup")
158-
}
159-
if err := store.Remove(context.Background(), collection.ID, ref); err != nil {
160-
t.Fatal(err)
161-
}
162-
spoken, empty = v3ScanBucket(bucket)
163-
if len(spoken) != 0 || len(empty) != 1 || empty[0].dir != dir || !v3EmptySession(dir) {
164-
t.Fatalf("a detached blank was still kept: spoken=%+v empty=%+v", spoken, empty)
165-
}
166-
folder, resumed, found := v3PickFolder(spoken, empty, false)
167-
if !found || resumed || folder.dir != dir {
168-
t.Fatalf("host discovery did not reuse the detached blank: %+v, resumed=%t found=%t", folder, resumed, found)
169-
}
170-
v3ReapEmpty(empty, "")
171-
if _, err := os.Stat(dir); !os.IsNotExist(err) {
172-
t.Fatalf("cleanup kept a detached blank: %v", err)
128+
for _, withSpoken := range []bool{false, true} {
129+
name := "only a detached blank"
130+
if withSpoken {
131+
name = "a detached blank beside an older spoken conversation"
132+
}
133+
t.Run(name, func(t *testing.T) {
134+
root, bucket := collectionKeepEnvironment(t)
135+
dir := writeV3Session(t, bucket, "reserved-agent", "", time.Time{})
136+
older := time.Date(2026, 10, 28, 9, 0, 0, 0, time.UTC)
137+
newer := older.Add(24 * time.Hour)
138+
if err := os.Chtimes(dir, newer, newer); err != nil {
139+
t.Fatal(err)
140+
}
141+
var conversation string
142+
wantSpoken := 0
143+
if withSpoken {
144+
conversation = writeV3Session(t, bucket, "spoken-agent", "keep these notes", older)
145+
wantSpoken = 1
146+
}
147+
store, err := workspace.Open(home.Join("v3", "collections.db"))
148+
if err != nil {
149+
t.Fatal(err)
150+
}
151+
t.Cleanup(func() { _ = store.Close() })
152+
collection, err := store.Create(context.Background(), "Reserved")
153+
if err != nil {
154+
t.Fatal(err)
155+
}
156+
ref := workspace.Ref{Kind: workspace.ConversationKind, ID: filepath.Base(dir)}
157+
if err := store.Add(context.Background(), collection.ID, ref); err != nil {
158+
t.Fatal(err)
159+
}
160+
spoken, empty := v3ScanBucket(bucket)
161+
if len(spoken) != wantSpoken || withSpoken && spoken[0].dir != conversation || len(empty) != 0 || v3EmptySession(dir) {
162+
t.Fatalf("a held blank was offered for resume or cleanup: spoken=%+v empty=%+v", spoken, empty)
163+
}
164+
if err := store.Remove(context.Background(), collection.ID, ref); err != nil {
165+
t.Fatal(err)
166+
}
167+
spoken, empty = v3ScanBucket(bucket)
168+
if len(spoken) != wantSpoken || withSpoken && spoken[0].dir != conversation || len(empty) != 1 || empty[0].dir != dir || !v3EmptySession(dir) {
169+
t.Fatalf("a detached blank was still kept: spoken=%+v empty=%+v", spoken, empty)
170+
}
171+
want := dir
172+
if withSpoken {
173+
want = conversation
174+
}
175+
folder, resumed, found := v3PickFolder(spoken, empty, false)
176+
if !found || resumed != withSpoken || folder.dir != want {
177+
t.Fatalf("host discovery picked %+v, resumed=%t found=%t", folder, resumed, found)
178+
}
179+
if got := v3LatestTranscript(root); got != (session.Place{Dir: want}).Transcript() {
180+
t.Fatalf("host discovery found %q, want the transcript in %s", got, want)
181+
}
182+
folder, resumed, found = v3PickFolder(spoken, empty, true)
183+
if withSpoken {
184+
if !found || !resumed || folder.dir != conversation {
185+
t.Fatalf("launch picked %+v, resumed=%t found=%t", folder, resumed, found)
186+
}
187+
} else if found || resumed {
188+
t.Fatalf("launch did not ask for a fresh identity: %+v, resumed=%t found=%t", folder, resumed, found)
189+
}
190+
v3ReapEmpty(empty, "")
191+
if _, err := os.Stat(dir); !os.IsNotExist(err) {
192+
t.Fatalf("cleanup kept a detached blank: %v", err)
193+
}
194+
})
173195
}
174196
}
175197

@@ -465,6 +487,108 @@ func TestACollectionHeldBlankAgentIsLiveAfterRestart(t *testing.T) {
465487
}
466488
}
467489

490+
// STAB2-fix R4
491+
func TestACollectionReadNeverOpensAHeldBlankToResolveItsChildren(t *testing.T) {
492+
for _, method := range []string{"collections.open", "collections.expand"} {
493+
t.Run(method, func(t *testing.T) {
494+
root, bucket := collectionKeepEnvironment(t)
495+
const id = "reserved-agent"
496+
dir := writeV3Session(t, bucket, id, "", time.Time{})
497+
if err := session.SaveMeta(dir, session.Meta{ID: id, Workspace: root, Title: "Reserved notes",
498+
Created: time.Date(2026, 10, 29, 9, 0, 0, 0, time.UTC),
499+
}); err != nil {
500+
t.Fatal(err)
501+
}
502+
transcript := session.Place{Dir: dir}.Transcript()
503+
if err := teams.Update(config.ProfileDir(), func(f *teams.File) error {
504+
crew := teams.NewID()
505+
f.Teams = append(f.Teams, teams.Team{ID: crew, Name: "Notes", Made: time.Now()})
506+
if err := f.AddMember(crew, teams.Member{Key: transcript, File: transcript, Where: root, Word: "notes", Handle: "notes"}); err != nil {
507+
return err
508+
}
509+
return f.SetManager(crew, transcript)
510+
}); err != nil {
511+
t.Fatal(err)
512+
}
513+
store, err := workspace.Open(home.Join("v3", "collections.db"))
514+
if err != nil {
515+
t.Fatal(err)
516+
}
517+
collection, err := store.Create(context.Background(), "Reserved")
518+
if err != nil {
519+
t.Fatal(err)
520+
}
521+
refs := []workspace.Ref{
522+
{Kind: workspace.ConversationKind, ID: id},
523+
{Kind: workspace.TaskKind, ID: "1", SessionID: id},
524+
{Kind: workspace.ProcessKind, ID: id + ":p1", SessionID: id},
525+
}
526+
// Reads visit newest members first, so resolve the conversation's
527+
// fallback before either child asks for its owner.
528+
if _, _, err := store.AddRefs(context.Background(), collection.ID, []workspace.Addition{
529+
{Ref: refs[0], AddedAt: 3},
530+
{Ref: refs[1], AddedAt: 2},
531+
{Ref: refs[2], AddedAt: 1},
532+
}); err != nil {
533+
t.Fatal(err)
534+
}
535+
if err := store.Close(); err != nil {
536+
t.Fatal(err)
537+
}
538+
539+
var opened atomic.Int32
540+
b := newFakeBackend(newAppRecorder(), func(ask appOpenAsk) (*appConn, error) {
541+
opened.Add(1)
542+
return refuseAppOpen(ask)
543+
})
544+
t.Cleanup(func() {
545+
_ = b.Close()
546+
if got := opened.Load(); got != 0 {
547+
t.Errorf("resolving collection members opened %d conversations", got)
548+
}
549+
})
550+
b.profileDir = config.ProfileDir()
551+
list, err := b.ListAgents("")
552+
if err != nil {
553+
t.Fatal(err)
554+
}
555+
if _, found := listsAgent(list, id); found {
556+
t.Fatal("agents.list included a blank conversation")
557+
}
558+
if method == "collections.open" {
559+
open, err := b.OpenCollection(collection.ID, 1, 500, "")
560+
if err != nil || len(open.Members) != len(refs) {
561+
t.Fatalf("collections.open: %+v, %v", open, err)
562+
}
563+
for i, member := range open.Members {
564+
if member.Kind != string(refs[i].Kind) || member.ID != refs[i].ID || member.SessionID != refs[i].SessionID {
565+
t.Fatalf("collections.open member %d: %+v", i, member)
566+
}
567+
state := appwire.TargetDeleted
568+
if i == 0 {
569+
state = appwire.TargetLive
570+
if member.Target.Title != "Reserved notes" || !reflect.DeepEqual(member.Target.In, []string{"Notes"}) {
571+
t.Fatalf("held blank conversation: %+v", member)
572+
}
573+
}
574+
if member.Target.State != state {
575+
t.Fatalf("collections.open member: %+v, want %s", member, state)
576+
}
577+
}
578+
} else {
579+
expanded, err := b.ExpandCollection(collection.ID, 3, 200)
580+
if err != nil || expanded.Missing != 2 || expanded.Unavailable != 0 || len(expanded.Refs) != 1 {
581+
t.Fatalf("collections.expand: %+v, %v", expanded, err)
582+
}
583+
live := expanded.Refs[0]
584+
if live.Kind != appwire.RefConversation || live.ID != id || live.Target.State != appwire.TargetLive || live.Target.Title != "Reserved notes" || !reflect.DeepEqual(live.Target.In, []string{"Notes"}) {
585+
t.Fatalf("collections.expand member: %+v", live)
586+
}
587+
}
588+
})
589+
}
590+
}
591+
468592
// STAB2-fix R1
469593
func TestAPermanentlyDeletedConversationStaysDeletedInItsCollection(t *testing.T) {
470594
for _, pending := range []bool{false, true} {

‎cmd/codeaf/appcollections.go‎

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,11 @@ package main
1010
//
1111
// RESOLUTION ASKS EACH OWNER ONCE PER CALL, NEVER ONCE PER MEMBER. A call
1212
// takes one agents listing (every state, closed and archived included), one
13-
// folder lookup per unlisted agent id, one crews listing, one task list and
14-
// one process list per distinct agent, and one look for a git work tree per
15-
// directory; a collection of a thousand files in one repository costs one walk
16-
// up to its .git. No transcript is read to resolve anything. A member whose
17-
// owner cannot be asked is "unknown", which says nothing about the member
13+
// folder lookup per unlisted conversation id, one crews listing, one task list
14+
// and one process list per distinct listed agent, and one look for a git work
15+
// tree per directory; a collection of a thousand files in one repository costs
16+
// one walk up to its .git. No transcript is read to resolve anything. A member
17+
// whose owner cannot be asked is "unknown", which says nothing about the member
1818
// itself, with a reason naming the owner and why it could not be asked.
1919

2020
import (
@@ -493,9 +493,10 @@ func appUnaskedReason(owner string, err error) string {
493493
type collectionResolver struct {
494494
b *appBackend
495495

496-
agentsRead bool
497-
agents map[string]appwire.Agent
498-
agentsErr error
496+
agentsRead bool
497+
agents map[string]appwire.Agent
498+
agentsErr error
499+
savedAgents map[string]appwire.Agent
499500

500501
crewsRead bool
501502
crews []appwire.Crew
@@ -527,7 +528,7 @@ func (r *collectionResolver) target(ref workspace.Ref) appwire.Target {
527528
}
528529
return r.factoryTargets[ref]
529530
case workspace.ConversationKind:
530-
a, not := r.agent(ref.ID)
531+
a, not := r.conversation(ref.ID)
531532
if not != nil {
532533
return *not
533534
}
@@ -582,7 +583,7 @@ func (r *collectionResolver) target(ref workspace.Ref) appwire.Target {
582583
return appwire.Target{State: appwire.TargetDeleted}
583584
}
584585

585-
// agent is one agent off the listing or its saved folder, or the target a member
586+
// agent is one agent off the listing, or the target a member
586587
// of it is when there is no such agent (deleted) or no listing (unknown, and why).
587588
func (r *collectionResolver) agent(id string) (appwire.Agent, *appwire.Target) {
588589
if !r.agentsRead {
@@ -598,7 +599,24 @@ func (r *collectionResolver) agent(id string) (appwire.Agent, *appwire.Target) {
598599
t := appUnknown("the conversation list", r.agentsErr)
599600
return appwire.Agent{}, &t
600601
}
601-
a, ok := r.agents[id]
602+
a := r.agents[id]
603+
if a.ID == "" {
604+
return appwire.Agent{}, &appwire.Target{State: appwire.TargetDeleted}
605+
}
606+
return a, nil
607+
}
608+
609+
// conversation also checks saved folders for explicitly referenced agents.
610+
// Its cache stays separate so child references still require a listed owner.
611+
func (r *collectionResolver) conversation(id string) (appwire.Agent, *appwire.Target) {
612+
a, not := r.agent(id)
613+
if not == nil || not.State != appwire.TargetDeleted {
614+
return a, not
615+
}
616+
if r.savedAgents == nil {
617+
r.savedAgents = map[string]appwire.Agent{}
618+
}
619+
a, ok := r.savedAgents[id]
602620
if !ok {
603621
// Named blanks stay out of agents.list but can still be held by a
604622
// collection. Cache misses too, since several members can name one agent.
@@ -609,10 +627,10 @@ func (r *collectionResolver) agent(id string) (appwire.Agent, *appwire.Target) {
609627
a = appRowView(row)
610628
}
611629
}
612-
r.agents[id] = a
630+
r.savedAgents[id] = a
613631
}
614632
if a.ID == "" {
615-
return appwire.Agent{}, &appwire.Target{State: appwire.TargetDeleted}
633+
return appwire.Agent{}, not
616634
}
617635
return a, nil
618636
}

0 commit comments

Comments
 (0)