Let idle desktop chat views beyond the three most recent go

cmd/codeaf/app.go

@@ -25,7 +25,8 @@ package main
 // idle conversations stay as indexed records, crew members included. Viewing,
 // sending or waking opens one through the same per-agent gate. Listing agents,
 // tasks and crews reads the disk. A conversation left alone for [appAgentIdle]
-// without an open view is let go and rebuilt transparently on demand.
+// without an open view is let go and rebuilt transparently on demand. Open
+// views keep theirs only for the [appViewHolds] most recently used.
 
 import (
 	"cmp"
@@ -75,12 +76,21 @@ import (
 // appAgentIdle is how long a conversation the app holds may sit with no turn
 // running, no question waiting and no open view before it is let go. Its
 // journal stays on disk, its state still reads idle, and the next send opens
-// it again. An explicitly open view keeps the conversation and its hold.
+// it again. An explicitly open view keeps the conversation and its hold while
+// it is among the [appViewHolds] most recently used.
 const appAgentIdle = 10 * time.Minute
 
 // appSweepEvery is how often the idle rule above is asked.
 const appSweepEvery = 30 * time.Second
 
+// appViewHolds is how many open views keep their idle conversation held: the
+// most recently used ones. A window opens every chat it shows and never says
+// when it stops showing one, so without a bound each chat shown in a session
+// kept its journal, presence watch and timers until the app quit (#2471). An
+// older idle view is let go like a background chat; its next read or send
+// opens it again.
+const appViewHolds = 3
+
 // appCrewPoll is how often the teams file is looked at for crew.changed, once
 // the app has asked for the crews at all. It is one stat when nothing moved.
 const appCrewPoll = 2 * time.Second
@@ -779,7 +789,8 @@ type appAgent struct {
 	localNotices map[string]bool
 	windowMoved  bool
 	// viewOpen preserves the person's open view through idle sweeps. Retiring
-	// it would let its next read immediately take back the released hold.
+	// it would let its next read immediately take back the released hold, so
+	// only views beyond the [appViewHolds] most recently used are let go.
 	viewOpen bool
 	// crewHeld keeps an established crew hold due after its engine closes.
 	crewHeld bool
@@ -4743,30 +4754,48 @@ func (b *appBackend) sweep() {
 }
 
 // sweepOnce is one look at the idle rule. A conversation is kept while a turn
-// runs, a question waits for the person or a view stays open, whatever the clock says.
+// runs or a question waits for the person, whatever the clock says, and so is
+// an open view among the [appViewHolds] most recently used.
 func (b *appBackend) sweepOnce() {
 	var syncing bool
 	b.locked(func() { syncing = b.drives != nil })
 	if syncing {
 		_, _ = b.SyncStatus()
 	}
 	now := b.now()
-	var idle []*appAgent
+	type candidate struct {
+		agent *appAgent
+		view  bool
+	}
+	var idle []candidate
 	b.locked(func() {
+		var views []*appAgent
 		for _, a := range b.agents {
-			if live := a.live; live != nil && !a.viewOpen && !live.running && now.Sub(live.lastUse) >= appAgentIdle {
-				idle = append(idle, a)
+			live := a.live
+			switch {
+			case live == nil:
+			case a.viewOpen:
+				views = append(views, a)
+			case !live.running && now.Sub(live.lastUse) >= appAgentIdle:
+				idle = append(idle, candidate{agent: a})
+			}
+		}
+		sort.Slice(views, func(i, j int) bool { return views[i].live.lastUse.After(views[j].live.lastUse) })
+		for _, a := range views[min(len(views), appViewHolds):] {
+			if !a.live.running && now.Sub(a.live.lastUse) >= appAgentIdle {
+				idle = append(idle, candidate{agent: a, view: true})
 			}
 		}
 	})
-	for _, a := range idle {
-		b.sweepAgent(a, now)
+	for _, c := range idle {
+		b.sweepAgent(c.agent, now, c.view)
 	}
 }
 
 // sweepAgent lets one idle conversation go, unless something took its gate or
-// woke it since the look.
-func (b *appBackend) sweepAgent(a *appAgent, now time.Time) {
+// woke it since the look. A view is let go only when view says it fell outside
+// the most recently used holds.
+func (b *appBackend) sweepAgent(a *appAgent, now time.Time, view bool) {
 	if !a.gate.TryLock() {
 		return
 	}
@@ -4775,7 +4804,7 @@ func (b *appBackend) sweepAgent(a *appAgent, now time.Time) {
 	keep := true
 	b.locked(func() {
 		live = a.live
-		keep = live == nil || a.viewOpen || live.running || now.Sub(live.lastUse) < appAgentIdle
+		keep = live == nil || (a.viewOpen && !view) || live.running || now.Sub(live.lastUse) < appAgentIdle
 	})
 	// A member of an open crew stays held while the crew is open.
 	if !keep && !b.liveNeedsYou(live) && !b.crewKeeps(a.view.ID) && !b.serviceKeeps(a.view.ID) {

cmd/codeaf/app_view_holds_test.go

@@ -0,0 +1,102 @@
+package main
+
+import (
+	"fmt"
+	"path/filepath"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/Agent-Field/codeaf/internal/appwire"
+)
+
+// AN OPEN VIEW KEEPS ITS CONVERSATION ONLY AMONG THE MOST RECENTLY USED. A
+// window opens every chat it shows and never says when it stops showing one,
+// so each chat shown in a session used to stay held, with its journal and
+// presence watch, until the app quit (#2471). Past the idle rule, only the
+// [appViewHolds] most recently used views keep their hold, however many chats
+// were shown.
+func TestAppIdleViewsBeyondTheRecentHoldsAreLetGo(t *testing.T) {
+	var mu sync.Mutex
+	var opens, releases int
+	open := func(ask appOpenAsk) (*appConn, error) {
+		mu.Lock()
+		opens++
+		mu.Unlock()
+		return &appConn{
+			Conv: &fakeConversation{}, Workspace: ask.Workspace, Transcript: ask.Transcript,
+			Release: func() { mu.Lock(); releases++; mu.Unlock() },
+		}, nil
+	}
+	backend := newFakeBackend(newAppRecorder(), open)
+	defer backend.Close()
+	dir := t.TempDir()
+	minted := 0
+	backend.mint = func(string) (string, error) {
+		minted++
+		return filepath.Join(dir, fmt.Sprintf("chat%02d", minted), "transcript.jsonl"), nil
+	}
+	clock := time.Now()
+	backend.now = func() time.Time { return clock }
+
+	var ids []string
+	view := func() {
+		t.Helper()
+		clock = clock.Add(time.Minute)
+		agent, err := mustCreate(t, backend, appwire.CreateAsk{Workspace: t.TempDir()})
+		if err != nil {
+			t.Fatal(err)
+		}
+		if _, err := backend.OpenAgent(agent.ID); err != nil {
+			t.Fatal(err)
+		}
+		ids = append(ids, agent.ID)
+	}
+	held := func() map[string]bool {
+		out := map[string]bool{}
+		backend.locked(func() {
+			for id, a := range backend.agents {
+				if a.live != nil {
+					out[id] = true
+				}
+			}
+		})
+		return out
+	}
+
+	const chats = appViewHolds + 4
+	for range chats {
+		view()
+	}
+	backend.sweepOnce()
+	if got := len(held()); got != chats {
+		t.Fatalf("within the idle rule %d of %d views are held", got, chats)
+	}
+
+	clock = clock.Add(appAgentIdle + time.Second)
+	backend.sweepOnce()
+	kept := held()
+	if len(kept) != appViewHolds {
+		t.Fatalf("after the idle rule %d views are held, want %d", len(kept), appViewHolds)
+	}
+	for i, id := range ids {
+		if want := i >= len(ids)-appViewHolds; kept[id] != want {
+			t.Fatalf("view %d held = %v, want only the %d most recently used", i, kept[id], appViewHolds)
+		}
+	}
+
+	// Opening more chats and leaving them idle never grows the holds.
+	for round := 0; round < 3; round++ {
+		view()
+		clock = clock.Add(appAgentIdle + time.Second)
+		backend.sweepOnce()
+		if got := len(held()); got != appViewHolds {
+			t.Fatalf("round %d: %d views held, want %d", round, got, appViewHolds)
+		}
+	}
+	mu.Lock()
+	defer mu.Unlock()
+	if releases != opens-appViewHolds {
+		t.Fatalf("opened %d, released %d; want all but %d released", opens, releases, appViewHolds)
+	}
+}

docs/changes/unreleased/2472-idle-view-holds-bounded.md

@@ -0,0 +1,14 @@
+---
+kind: fixed
+title: Desktop chats shown in a session no longer stay open in the engine until the app quits
+pr: 2472
+surface: [app, engine, docs]
+invalidates:
+  - "A chat opened for viewing in the desktop app kept its conversation and relay hold until it was explicitly closed. Only the three most recently used open chats keep theirs now; an older open chat idle for ten minutes is let go like a background chat and opens again on its next read or send."
+---
+
+The window opens every chat it shows and does not say when it stops showing
+one, so each chat shown in a session stayed held with its journal, its presence
+watch (about six file handles on macOS) and its timers. The idle sweep now keeps
+at most three open views held; running turns, waiting questions, crews, kept
+services and explicit closes are unchanged.

internal/manual/chat/desktop-sync.md

@@ -194,10 +194,13 @@ once if that computer saved again first.
 
 ## Does an idle desktop chat keep its hold on the relay
 
-A chat opened for viewing keeps its conversation and relay hold until it is
-explicitly closed, including while its tab is hidden. Heartbeats renew the same
-hold without releasing it or moving its fence. Reading the chat does not cause
-ownership changes or repeat unchanged chat rows.
+A chat opened for viewing keeps its conversation and relay hold, including while
+its tab is hidden, as long as it is one of the three most recently used open
+chats. Heartbeats renew the same hold without releasing it or moving its fence.
+Reading the chat does not cause ownership changes or repeat unchanged chat rows.
+An older open chat left idle for ten minutes is let go like a background chat,
+which frees its files and timers; using it again opens it through the normal
+ownership check. Closing a chat lets it go at once.
 
 A conversation opened only for background work is let go after ten idle minutes;
 the sweep checks every 30 seconds. It stays released until work needs it again.