perf: an idle client stops reopening the vault per key and retrying failed chat copies (#1980)

* test(vaultsync): a restore opens the vault a fixed number of times

Fails on dev: a restore of N shared keys opens the whole sealed vault
N+1 times, once per key the ledger knows.

* perf(vaultsync): read a scope's tombstones once per restore

A restore asked the vault whether each key the ledger knew had been
removed, and every question opened the whole sealed vault. A profile with
963 shared memories and a 1.5 MB vault decoded about 1.4 GB of JSON per
state round, every few seconds, in an idle terminal. Removed lists a
scope's tombstones in one read.

* refactor(olderchats): one visit per chat behind a copy seam

No behaviour change: the eligibility checks and the copy move into
olderChats.visit so a test can count copy attempts.

* test(olderchats): a failed copy waits for the chat to change

Fails on dev: the background copier tries a chat whose copy failed again
every round, and each attempt seals the chat's whole folder.

* perf(olderchats): a failed copy waits for its chat to change

The background copier visits every older chat each pass, and a copy seals
the chat's whole folder. A copy that failed was attempted again on every
pass for as long as the terminal stayed open. It is now remembered against
the journal's size and time, and tried again when the chat changes, on the
next launch, or when the person opens it.

* test(olderchats): the background copier leaves a folder that holds codeaf's home

Fails on dev: a chat started in a home directory is sealed whole in the
background, and that seal never settles while codeaf writes its own state
inside the folder.

* perf(olderchats): leave a folder that holds codeaf's home to be copied on open

A chat started in a home directory has codeaf's own state inside its
folder. Codeaf writes there while it runs, so the whole-folder seal a copy
begins with never settles: in a live terminal each attempt planned the
folder eight times and failed with "files kept changing". The background
copier now skips these chats; opening one still copies it.

* revert(olderchats): leave home-folder chats to the cellstore rule

#1976 makes a folder that holds the home folder seal only the chat's own
folder, which is the case this skip covered. One rule for it is enough;
the failed-copy memo stays.

* docs(changes): an idle terminal stops reopening the vault per key

Author: santoshkumarradha

cmd/codeaf/olderchats.go

@@ -55,31 +55,16 @@ func runOlderChats(ctx context.Context) {
 		return
 	case <-ticker.C:
 	}
+	chats := olderChats{copy: copyOlderChat}
 	for {
 		if syncsetup.MayTalk(home.Dir()) {
 			dirs, _ := filepath.Glob(home.Join("v3", "projects", "*", "*"))
 			for _, dir := range dirs {
 				if ctx.Err() != nil {
 					return
 				}
-				copied := cell.Copied(dir)
-				if copied != dir && session.InUse(filepath.Join(copied, cell.TranscriptPath)) {
-					continue
-				}
-				if copied != dir {
-					if _, err := os.Stat(filepath.Join(copied, cell.CopyOfferedPath)); err == nil {
-						continue
-					}
-				}
-				if _, err := os.Stat(filepath.Join(dir, "transcript.jsonl")); err != nil {
-					continue
-				}
-				meta, _ := session.LoadMeta(dir)
-				if meta.LastUserAt.IsZero() || time.Since(meta.LastUserAt) > olderChatRecent || session.InUse(filepath.Join(dir, "transcript.jsonl")) {
-					continue
-				}
-				c, err := copyOlderChat(ctx, dir)
-				if err != nil {
+				c, meta, ok := chats.visit(ctx, dir)
+				if !ok {
 					continue
 				}
 				// A finished copy is uploaded and released as an ordinary chat. The
@@ -106,6 +91,65 @@ func runOlderChats(ctx context.Context) {
 	}
 }
 
+// olderChats is the background copier's view of the older chats: which one is
+// due for a copy, the copy itself, and the copies that failed this launch.
+//
+// A FAILED COPY WAITS FOR ITS CHAT TO CHANGE. Every round visits every older
+// chat, and a copy seals the chat's whole folder, so an attempt costs as much as
+// the folder is big. A copy that failed was tried again on the next pass for as
+// long as the terminal stayed open. The same chat over the same folder fails the
+// same way; one whose journal has moved on, or a new launch, is tried again, and
+// opening the chat still copies it on demand (movablePlace).
+type olderChats struct {
+	copy   func(context.Context, string) (cell.Cell, error)
+	failed map[string]olderChatStamp
+}
+
+// olderChatStamp is what an original's journal looked like when its copy failed.
+type olderChatStamp struct {
+	size int64
+	mod  time.Time
+}
+
+// visit copies the chat in dir when it is a recent older chat that nothing is
+// using and that has no copy yet. It answers the copy and the original's meta.
+func (o *olderChats) visit(ctx context.Context, dir string) (cell.Cell, session.Meta, bool) {
+	copied := cell.Copied(dir)
+	if copied != dir && session.InUse(filepath.Join(copied, cell.TranscriptPath)) {
+		return cell.Cell{}, session.Meta{}, false
+	}
+	if copied != dir {
+		if _, err := os.Stat(filepath.Join(copied, cell.CopyOfferedPath)); err == nil {
+			return cell.Cell{}, session.Meta{}, false
+		}
+	}
+	journal, err := os.Stat(filepath.Join(dir, "transcript.jsonl"))
+	if err != nil {
+		return cell.Cell{}, session.Meta{}, false
+	}
+	stamp := olderChatStamp{size: journal.Size(), mod: journal.ModTime()}
+	if o.failed[dir] == stamp {
+		return cell.Cell{}, session.Meta{}, false
+	}
+	meta, _ := session.LoadMeta(dir)
+	if meta.LastUserAt.IsZero() || time.Since(meta.LastUserAt) > olderChatRecent || session.InUse(filepath.Join(dir, "transcript.jsonl")) {
+		return cell.Cell{}, session.Meta{}, false
+	}
+	c, err := o.copy(ctx, dir)
+	if err != nil {
+		// A copy cut short by the terminal closing did not fail.
+		if ctx.Err() == nil {
+			if o.failed == nil {
+				o.failed = map[string]olderChatStamp{}
+			}
+			o.failed[dir] = stamp
+		}
+		return cell.Cell{}, session.Meta{}, false
+	}
+	delete(o.failed, dir)
+	return c, meta, true
+}
+
 // movablePlace resolves a completed copy or prepares one when the person opens
 // an older chat. Selection runs on the existing asynchronous opening door.
 func movablePlace(ctx context.Context, place session.Place) (session.Place, error) {

cmd/codeaf/olderchats_retry_test.go

@@ -0,0 +1,61 @@
+package main
+
+import (
+	"context"
+	"errors"
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/Agent-Field/codeaf/internal/cell"
+	"github.com/Agent-Field/codeaf/internal/home"
+	"github.com/Agent-Field/codeaf/internal/session"
+)
+
+// TestAFailedOlderChatCopyWaitsForTheChatToChange counts copy attempts. An idle
+// terminal visits every older chat each background round, and a copy seals the
+// chat's whole folder, so a chat whose copy failed is not copied again until it
+// has changed: the same chat and folder fail the same way.
+func TestAFailedOlderChatCopyWaitsForTheChatToChange(t *testing.T) {
+	t.Setenv(home.EnvVar, t.TempDir())
+	original := filepath.Join(home.Dir(), "v3", "projects", "old", "old-session")
+	if err := os.MkdirAll(original, 0700); err != nil {
+		t.Fatal(err)
+	}
+	transcript := filepath.Join(original, "transcript.jsonl")
+	if err := os.WriteFile(transcript, []byte(`{"type":"session","version":1,"id":"old-session"}`+"\n"), 0600); err != nil {
+		t.Fatal(err)
+	}
+	if err := session.SaveMeta(original, session.Meta{ID: "old-session", LastUserAt: time.Now(), Workspace: t.TempDir()}); err != nil {
+		t.Fatal(err)
+	}
+	attempts := 0
+	chats := olderChats{copy: func(context.Context, string) (cell.Cell, error) {
+		attempts++
+		return cell.Cell{}, errors.New("files kept changing while saving this chat; try again")
+	}}
+	for range 5 {
+		if _, _, ok := chats.visit(t.Context(), original); ok {
+			t.Fatal("a failed copy was reported as made")
+		}
+	}
+	if attempts != 1 {
+		t.Fatalf("five rounds over an unchanged chat whose copy fails made %d copy attempts, want 1", attempts)
+	}
+
+	f, err := os.OpenFile(transcript, os.O_APPEND|os.O_WRONLY, 0600)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := f.WriteString(`{"type":"message","role":"user","content":"later"}` + "\n"); err != nil {
+		t.Fatal(err)
+	}
+	if err := f.Close(); err != nil {
+		t.Fatal(err)
+	}
+	chats.visit(t.Context(), original)
+	if attempts != 2 {
+		t.Fatalf("a chat that changed after its copy failed made %d copy attempts in total, want 2", attempts)
+	}
+}

docs/changes/unreleased/1980-idle-client-cpu.md

@@ -0,0 +1,14 @@
+---
+kind: fixed
+title: An idle terminal no longer reopens the key vault once per shared key
+pr: 1980
+surface: [chat]
+invalidates:
+  - "A home-state restore asked the vault about each shared key in turn and opened the whole sealed vault for every answer. It now reads the scope's removals once per restore."
+  - "The background copier tried an older chat whose copy failed again on every pass. A failed copy now waits until the chat changes, the next launch, or the person opening it."
+---
+With 963 shared memories and a 1.5 MB vault, a restore decoded about 1.4 GB of
+JSON. A terminal that pulled every round sat above one core while nobody typed.
+Measured on Spark against a copy of that profile, with the vault moving every
+round, the client went from 111 to 115% CPU down to 15%. Without that, it went
+from 15% to 11%.

internal/keys/vault.go

@@ -198,6 +198,24 @@ func (v *Vault) IDs(prefix string) ([]string, error) {
 	return out, nil
 }
 
+// Removed lists the ids that start with prefix and hold a tombstone, in order.
+// It opens the vault once, so a caller asking about many slots does not pay for
+// a whole vault per slot the way a loop over Deleted would.
+func (v *Vault) Removed(prefix string) ([]string, error) {
+	d, err := v.read()
+	if err != nil {
+		return nil, err
+	}
+	var out []string
+	for id, r := range d.Secrets {
+		if r.Deleted && strings.HasPrefix(id, prefix) {
+			out = append(out, id)
+		}
+	}
+	sort.Strings(out)
+	return out, nil
+}
+
 // Env returns NAME=value pairs for the project, sorted by name, ready to
 // append to an exec environment.
 func (v *Vault) Env(project string) ([]string, error) {

internal/vaultsync/keyed.go

@@ -6,6 +6,7 @@ import (
 	"encoding/json"
 	"errors"
 	"os"
+	"strings"
 	"time"
 
 	"github.com/Agent-Field/codeaf/internal/keys"
@@ -195,20 +196,23 @@ func (r *keyedRun) liveKeys() (map[string]string, error) {
 }
 
 // changes is what to apply here: live keys that differ, and keys the vault
-// deleted that this machine still holds and once shared.
+// deleted that this machine still holds and once shared. The tombstones are read
+// once for the whole scope: a restore follows every pull, and asking the vault
+// per known key opened it a thousand times a round for a thousand memories.
 func (r *keyedRun) changes(live map[string]string) (map[string]string, error) {
 	changes := map[string]string{}
 	for name, value := range live {
 		if r.set.Holds(name) && r.held[name] != value {
 			changes[name] = value
 		}
 	}
-	for name := range r.digests {
-		gone, err := r.v.Deleted(r.id(name))
-		if err != nil {
-			return nil, err
-		}
-		if gone && r.held[name] != "" {
+	removed, err := r.v.Removed(r.id(""))
+	if err != nil {
+		return nil, err
+	}
+	for _, id := range removed {
+		name := strings.TrimPrefix(id, r.id(""))
+		if _, known := r.digests[name]; known && r.held[name] != "" {
 			changes[name] = ""
 		}
 	}

internal/vaultsync/keyed_reads_test.go

@@ -0,0 +1,98 @@
+package vaultsync
+
+import (
+	"fmt"
+	"maps"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/Agent-Field/codeaf/internal/identity"
+	"github.com/Agent-Field/codeaf/internal/keys"
+)
+
+// countedVault counts every call into the vault file. Each one opens the whole
+// sealed vault (read, decrypt, decode), so the count is the work a step does.
+type countedVault struct {
+	VaultFile
+	calls int
+}
+
+func (c *countedVault) Export() ([]byte, error) { c.calls++; return c.VaultFile.Export() }
+func (c *countedVault) Merge(enc []byte) error  { c.calls++; return c.VaultFile.Merge(enc) }
+func (c *countedVault) Entries(p string) ([]keys.Entry, error) {
+	c.calls++
+	return c.VaultFile.Entries(p)
+}
+func (c *countedVault) Get(id string) (keys.Entry, error) { c.calls++; return c.VaultFile.Get(id) }
+func (c *countedVault) IDs(prefix string) ([]string, error) {
+	c.calls++
+	return c.VaultFile.IDs(prefix)
+}
+func (c *countedVault) PutAt(id string, e keys.Entry, at time.Time) error {
+	c.calls++
+	return c.VaultFile.PutAt(id, e, at)
+}
+func (c *countedVault) Delete(id string) error { c.calls++; return c.VaultFile.Delete(id) }
+func (c *countedVault) Deleted(id string) (bool, error) {
+	c.calls++
+	return c.VaultFile.Deleted(id)
+}
+func (c *countedVault) Removed(prefix string) ([]string, error) {
+	c.calls++
+	return c.VaultFile.Removed(prefix)
+}
+
+// heldKeys is a Keyset that holds its keys in memory.
+type heldKeys map[string]string
+
+func (h heldKeys) Values() (map[string]string, error) { return maps.Clone(h), nil }
+func (h heldKeys) Holds(string) bool                  { return true }
+func (h heldKeys) Apply(changes map[string]string) error {
+	for name, value := range changes {
+		if value == "" {
+			delete(h, name)
+		} else {
+			h[name] = value
+		}
+	}
+	return nil
+}
+
+// TestARestoreOpensTheVaultAFixedNumberOfTimes holds the idle cost of a state
+// round flat in the number of keys a machine shares. A restore runs after every
+// pull, which an idle terminal makes every few seconds; a profile with a
+// thousand shared memories once opened its whole vault once per key per round.
+func TestARestoreOpensTheVaultAFixedNumberOfTimes(t *testing.T) {
+	for _, n := range []int{16, 256} {
+		t.Run(fmt.Sprint(n, " keys"), func(t *testing.T) {
+			home := filepath.Join(t.TempDir(), "home")
+			_, err := identity.Ensure(home)
+			must(t, err)
+			v, err := keys.Open(home)
+			must(t, err)
+			set := heldKeys{}
+			for i := range n {
+				set[fmt.Sprintf("key-%04d", i)] = fmt.Sprintf("value %d", i)
+			}
+			carrier := Keys("test", set, filepath.Join(home, "test.ledger"))
+			must(t, carrier.capture(v))
+
+			counted := &countedVault{VaultFile: v}
+			must(t, carrier.restore(counted))
+			if counted.calls > 2 {
+				t.Fatalf("a restore of %d unchanged keys opened the vault %d times, want at most 2", n, counted.calls)
+			}
+
+			// Reading the tombstones once still passes a removal on.
+			must(t, v.Delete(carrier.id("key-0003")))
+			must(t, carrier.restore(v))
+			if _, still := set["key-0003"]; still {
+				t.Fatal("a key the vault removed is still held after a restore")
+			}
+			if len(set) != n-1 {
+				t.Fatalf("a restore after one removal holds %d keys, want %d", len(set), n-1)
+			}
+		})
+	}
+}

internal/vaultsync/syncer.go

@@ -33,6 +33,8 @@ type VaultFile interface {
 	PutAt(id string, e keys.Entry, at time.Time) error
 	Delete(id string) error
 	Deleted(id string) (bool, error)
+	// Removed lists the tombstoned slot ids under a prefix, in one read.
+	Removed(prefix string) ([]string, error)
 }
 
 // ErrTampered reports a vault object whose bytes do not hash to its id.