catalog: the lazy warm is owned, cancelled and joined at close, so it cannot write under a later home

cmd/codeaf/chatv3_guard_class_test.go

@@ -14,8 +14,8 @@ import (
 // TestV3ProcessGuardGoClass is the inventory law for goroutines started by a
 // v3 process or launch. A long-lived writer must identify both its stop and the
 // closeAll join. chatv3/models and engine/models name the process waiter, joined
-// since #1179; the internal/catalog catalog/warm goroutine each one starts is a
-// separate open writer one package down, owned by the catalog-warm-close cell.
+// since #1179, and the internal/catalog warm each one starts, which closeAll now
+// cancels and joins through Models.Close.
 // The remaining known-open row (chatv3/sweep-home) names the later cell that owns it;
 // deleting a row is part of landing that cell.
 func TestV3ProcessGuardGoClass(t *testing.T) {
@@ -26,8 +26,8 @@ func TestV3ProcessGuardGoClass(t *testing.T) {
 		"chatv3/standing":         {kind: "joined writer", stop: "v3Process.standingStop", join: "v3Process.closeAll calls stopStandingTicks"},
 		"chatv3/standing-stop":    {kind: "joined helper", stop: "standingStop is closed", join: "chatv3/standing closes standingDone after this helper returns"},
 		"pool/judge-sweep":        {kind: "joined writer", stop: "pool errand context", join: "v3Process.closeAll calls stopPoolErrands"},
-		"chatv3/models":           {kind: "joined waiter, downstream catalog/warm still open", stop: "warmModels waiter on the pool errand context", join: "v3Process.closeAll calls stopPoolErrands, joining the waiter since #1179; the internal/catalog catalog/warm it starts is not yet joined, owned by the catalog-warm-close cell"},
-		"engine/models":           {kind: "joined waiter, downstream catalog/warm still open", stop: "warmModels waiter on the pool errand context", join: "v3Process.closeAll calls stopPoolErrands, joining the waiter since #1179; the internal/catalog catalog/warm it starts is not yet joined, owned by the catalog-warm-close cell"},
+		"chatv3/models":           {kind: "joined writer", stop: "warmModels waiter on the pool errand context, and the catalog's own warm context", join: "v3Process.closeAll calls stopPoolErrands for the waiter (since #1179) and Models.Close, which cancels and joins the internal/catalog warm"},
+		"engine/models":           {kind: "joined writer", stop: "warmModels waiter on the pool errand context, and the catalog's own warm context", join: "v3Process.closeAll calls stopPoolErrands for the waiter (since #1179) and Models.Close, which cancels and joins the internal/catalog warm"},
 		"chatv3/sweep-home":       {kind: "KNOWN-OPEN writer", owner: "later home-sweep ownership cell (class target three)"},
 		"chatv3/background":       {kind: "one-shot", stop: "repairBackgroundChecks returns after one bounded Drift/Install pass"},
 		"chatv3/once-questions":   {kind: "one-shot", stop: "agent.Close closes the WatchQuestions channel consumed by the range"},

cmd/codeaf/chatv3_process.go

@@ -60,6 +60,10 @@ type v3Process struct {
 	// Models is ONE lazy warm and one cache on disk. N catalogs would be N
 	// network round trips for one answer.
 	Models *catalog.Catalog
+	// processCtx is the lifetime shared by background work owned by this process.
+	// processStop closes that lifetime before closeAll joins each owned worker.
+	processCtx  context.Context
+	processStop context.CancelFunc
 	// Shelf holds Models until somebody asks /model for today's list, and the
 	// refreshed catalog after (chatv3_modelshelf.go). The picker and the two
 	// session readers that answer about a model somebody may have just picked
@@ -180,7 +184,8 @@ func openV3ProcessWith(door string, askKey bool) (*v3Process, error) {
 	discovery := catalog.Options{
 		BaseURL: settings.BaseURL, APIKey: settings.APIKey, Dir: settings.ProfileDir,
 	}
-	models := catalog.LoadLazy(context.Background(), discovery)
+	processCtx, processStop := context.WithCancel(context.Background())
+	models := catalog.LoadLazy(processCtx, discovery)
 	// A tier row that says auto is answered from this catalog (config.AutoModels):
 	// the same non-blocking read, never a fetch, and set once at start-up.
 	config.AutoModels = models.ModelsNow
@@ -192,6 +197,8 @@ func openV3ProcessWith(door string, askKey bool) (*v3Process, error) {
 		ProfileDir:        settings.ProfileDir,
 		UnreadProfileKeys: append([]string(nil), settings.UnreadProfileKeys...),
 		Models:            models,
+		processCtx:        processCtx,
+		processStop:       processStop,
 		Shelf:             shelf,
 		Harnesses:         subharness.Default(),
 		Memory:            v3Memory(settings.ProfileDir),
@@ -412,6 +419,16 @@ func (p *v3Process) closeAll() {
 		return
 	}
 
+	// Cancel the process lifetime first, then join the catalog warm. Catalog.Close
+	// also cancels its derived context, and its join is what makes the promise
+	// that no cache write can happen after closeAll returns airtight.
+	if p.processStop != nil {
+		p.processStop()
+	}
+	if p.Models != nil {
+		p.Models.Close()
+	}
+
 	// The process starts its place sweep before it opens any shared state. Seal
 	// its note before closing that state so a late error cannot write after
 	// close; the walk itself is not waited on, so quit does not grow with the

cmd/codeaf/engine.go

@@ -145,6 +145,7 @@ func runRemoteEngine(args []string) error {
 	// its own. The close is the same door [v3Process.closeAll] uses; it drains
 	// the queue before joining, so the last row is on disk as well.
 	session.CloseUsage()
+	closeEngineProcess()
 	return quietRefusal(err)
 }
 
@@ -520,6 +521,7 @@ func runEngineHost(workspaceFlag, sessionFlag string) error {
 	// (The refusal path above never booted a conversation, so its registry is
 	// empty and this close is a no-op there. It is one owner door, not two.)
 	session.CloseUsage()
+	closeEngineProcess()
 	if errors.Is(err, enginehost.ErrHostRunning) {
 		return nil
 	}
@@ -949,6 +951,15 @@ func openEngineProcess() (*v3Process, error) {
 	return engineProcess.proc, engineProcess.err
 }
 
+// closeEngineProcess closes the once-per-process resources after the serving
+// road has returned. In particular, closeAll cancels and joins model discovery
+// before the engine process can exit or its profile can be reused.
+func closeEngineProcess() {
+	if engineProcess.proc != nil {
+		engineProcess.proc.closeAll()
+	}
+}
+
 // engineStandingItems and engineStandingSave are the two standing doors, or nil.
 //
 // NIL IS THE AMBIENT SIDE OFF AND IT IS NEVER A CLOSURE THAT FAILS, which is

docs/changes/unreleased/1274-catalog-lazy-warm-close.md

@@ -0,0 +1,13 @@
+---
+kind: fixed
+title: closing a process cancels and joins lazy catalog warming before a late cache write
+pr: 1274
+surface: [engine]
+invalidates:
+  - "Lazy catalog warming could outlive process close and write its cache afterward, including beneath a home selected by a later launch. Close now cancels and joins the warm, and a fetch that succeeds after cancellation does not write the cache."
+  - "A later process launch still warms its catalog normally."
+---
+
+# Lazy catalog warming stops cleanly at close
+
+A process close now cancels and joins its in-flight lazy catalog warm. If a fetch succeeds after cancellation, catalog loading checks the canceled context before writing the cache, so no cache write can occur after close returns. A later process launch can still warm its catalog normally.

internal/catalog/catalog.go

@@ -257,6 +257,10 @@ type Options struct {
 type Catalog struct {
 	ready   *rows
 	resolve func() *rows
+	// cancel and warmDone give a lazy catalog ownership of its background
+	// discovery. Close cancels the fetch and joins the goroutine before returning.
+	cancel   context.CancelFunc
+	warmDone chan struct{}
 	// warm is the resolved value published the instant resolution finishes, so
 	// a question that must not wait can still be answered once the answer
 	// exists. [Catalog.rows] blocks on the future; [Catalog.rowsNow] reads this
@@ -366,9 +370,14 @@ func loadOrFallback(ctx context.Context, options Options) (resolved *rows, err e
 // the surface is up, so the goroutine warms the value while the caller carries
 // on, and only a question that genuinely arrives first ever blocks.
 func LoadLazy(ctx context.Context, options Options) *Catalog {
-	resolved := &Catalog{warmed: make(chan struct{})}
+	warmCtx, cancel := context.WithCancel(ctx)
+	resolved := &Catalog{
+		warmed:   make(chan struct{}),
+		cancel:   cancel,
+		warmDone: make(chan struct{}),
+	}
 	resolve := sync.OnceValue(func() *rows {
-		loaded, _ := loadOrFallback(ctx, options)
+		loaded, _ := loadOrFallback(warmCtx, options)
 		resolved.warm.Store(loaded)
 		// The wait door ([Catalog.Warmed]) reads the close, not the value, and
 		// the two land together so a caller that arrived between them would
@@ -377,10 +386,23 @@ func LoadLazy(ctx context.Context, options Options) *Catalog {
 		return loaded
 	})
 	resolved.resolve = resolve
-	guard.Go("catalog/warm", func() { resolve() })
+	guard.Go("catalog/warm", func() {
+		defer close(resolved.warmDone)
+		resolve()
+	})
 	return resolved
 }
 
+// Close cancels and joins a lazy catalog warm. It is safe to call more than
+// once. Eager and zero catalogs have no background work and return immediately.
+func (c *Catalog) Close() {
+	if c == nil || c.cancel == nil {
+		return
+	}
+	c.cancel()
+	<-c.warmDone
+}
+
 // load resolves one catalog, and reports why the fetch failed when it spent the
 // network and did not land. A fresh cache that needed no fetch is not a failure
 // and answers nil.
@@ -399,6 +421,11 @@ func load(ctx context.Context, options Options) (*rows, error) {
 
 	// fetch refuses an empty listing itself, so a nil error here is always rows.
 	models, err := fetch(ctx, options)
+	if err == nil {
+		// A transport may report success after cancellation. Closing the owner
+		// must still prevent that late response from changing persistent state.
+		err = ctx.Err()
+	}
 	if err == nil {
 		fetchedAt := now().UTC()
 		if path != "" {

internal/catalog/lazy_test.go

@@ -2,8 +2,12 @@ package catalog
 
 import (
 	"context"
+	"io"
 	"net/http"
 	"net/http/httptest"
+	"os"
+	"path/filepath"
+	"strings"
 	"sync"
 	"testing"
 	"time"
@@ -150,3 +154,85 @@ func TestIndexKeepsTheFirstRowForANormalizedID(t *testing.T) {
 		t.Fatal("Supports must read the same row Model does")
 	}
 }
+
+func TestCloseJoinsLateSuccessfulWarmWithoutWritingEitherHome(t *testing.T) {
+	oldHome := t.TempDir()
+	newHome := t.TempDir()
+	t.Setenv("CODEAF_HOME", oldHome)
+
+	started := make(chan struct{})
+	release := make(chan struct{})
+	client := &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) {
+		close(started)
+		<-release
+		return &http.Response{
+			StatusCode: http.StatusOK,
+			Status:     http.StatusText(http.StatusOK),
+			Header:     make(http.Header),
+			Body:       io.NopCloser(strings.NewReader(catalogPayload)),
+		}, nil
+	})}
+	first := LoadLazy(context.Background(), Options{
+		BaseURL: "https://catalog.example/v1", HTTPClient: client,
+	})
+	<-started
+	t.Setenv("CODEAF_HOME", newHome)
+
+	closed := make(chan struct{})
+	go func() {
+		first.Close()
+		close(closed)
+	}()
+	select {
+	case <-closed:
+		t.Fatal("Close returned while the warm fetch was still running")
+	case <-time.After(50 * time.Millisecond):
+	}
+	close(release)
+	select {
+	case <-closed:
+	case <-time.After(5 * time.Second):
+		t.Fatal("Close did not join the completed warm fetch")
+	}
+	assertEmptyTree(t, oldHome)
+	assertEmptyTree(t, newHome)
+
+	second := LoadLazy(context.Background(), Options{
+		BaseURL:    "https://catalog.example/v1",
+		HTTPClient: catalogClient(t, http.StatusOK, catalogPayload, nil),
+	})
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+	if !second.Warmed(ctx) {
+		t.Fatal("a second launch did not warm")
+	}
+	second.Close()
+	entries, err := os.ReadDir(newHome)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(entries) == 0 {
+		t.Fatal("the second launch did not write its cache")
+	}
+}
+
+func assertEmptyTree(t *testing.T, root string) {
+	t.Helper()
+	var found string
+	err := filepath.WalkDir(root, func(path string, entry os.DirEntry, err error) error {
+		if err != nil {
+			return err
+		}
+		if path != root {
+			found = path
+			return filepath.SkipAll
+		}
+		return nil
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if found != "" {
+		t.Fatalf("late warm wrote %s", found)
+	}
+}