From de4fd64b94eaa57a214eaa37408bdbf8e30b99b6 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Thu, 22 Oct 2026 20:09:53 -0400 Subject: [PATCH 1/3] Recover unavailable memory helpers and clarify background engine upgrades --- cmd/codeaf/chatv3.go | 37 +- cmd/codeaf/chatv3_beside.go | 1 + cmd/codeaf/chatv3_host.go | 4 + cmd/codeaf/crew_refresh.go | 29 ++ cmd/codeaf/crew_refresh_test.go | 76 ++++ cmd/codeaf/engine.go | 55 +-- cmd/codeaf/engine_stale_test.go | 44 ++- internal/config/budget.go | 14 +- internal/e2e/engine_upgrade_e2e_test.go | 181 +++++++++ .../e2e/memory_invalid_helpers_e2e_test.go | 355 ++++++++++++++++++ .../manual/chat/running-on-another-machine.md | 3 +- .../manual/chat/staying-on-that-machine.md | 8 +- internal/manual/chat/what-i-remember.md | 33 +- internal/provider/routefailure.go | 4 + internal/provider/routefailure_test.go | 3 + internal/reflex/recovery_test.go | 120 +++++- internal/reflex/reflex.go | 231 +++++++----- internal/reflex/thinking_test.go | 2 +- internal/session/memory.go | 2 +- internal/tui3/engine_build_test.go | 31 ++ internal/tui3/hostlink.go | 6 +- internal/tui3/statusnote.go | 22 +- 22 files changed, 1092 insertions(+), 169 deletions(-) create mode 100644 cmd/codeaf/crew_refresh.go create mode 100644 cmd/codeaf/crew_refresh_test.go create mode 100644 internal/e2e/engine_upgrade_e2e_test.go create mode 100644 internal/e2e/memory_invalid_helpers_e2e_test.go create mode 100644 internal/tui3/engine_build_test.go diff --git a/cmd/codeaf/chatv3.go b/cmd/codeaf/chatv3.go index ebb483d594..3d37520281 100644 --- a/cmd/codeaf/chatv3.go +++ b/cmd/codeaf/chatv3.go @@ -2245,24 +2245,11 @@ func v3MemorySeam(brain *store.Store) tui3.MemoryStore { // that made them restart to be heard would be a knob that does nothing on the // surface that offers it. // -// The seam is a SNAPSHOT INVALIDATED BY A GENERATION COUNTER -// ([config.SettingsGeneration]), for reasons the two obvious alternatives fail: -// -// - Re-reading config on every Resolve would put a file read on the path of -// every auxiliary call — twice a turn for the reflex pair alone — for a file -// that changes once a week. internal/config caches nothing on purpose. -// - Invalidating from the settings panel's write hook would only see the -// panel. The `change_setting` tool writes the same rows from inside a turn, -// and /crew writes four of them at once. -// -// A counter bumped by the ONE persisted writer sees all of them, costs an atomic -// load per call when nothing has moved, and holds the lock only to rebuild. A -// mid-session change is honored by the NEXT crew call, which is the promise the -// settings rows now make. -// -// What it does not see is stated where the counter is: a config file edited by -// another process, and the project layer's file at all. Both land on the next -// launch, which is what every settings row did before this. +// The snapshot follows this process's settings generation and the on-disk +// stamps of both profile and project configuration. A settings panel in another +// window is a different process from the engine: its writes must reach the next +// auxiliary call without restarting the engine. Unchanged files cost stats, +// never repeated reads or JSON decoding; internal/config memoizes their values. func v3RolesSource(workspace, profileDir string) (func(string) (string, bool), error) { crew := &v3Crew{workspace: workspace, profileDir: profileDir} // The first build happens HERE rather than lazily, so a malformed pins row @@ -2283,6 +2270,7 @@ type v3Crew struct { // is impossible after the first build, so there is no "never read" state to // spell separately. generation uint64 + files [2]crewConfigStamp values map[string]string // low is [config.CreditsLowAt] as of the snapshot. It is compared on every // read beside the generation because the balance record is written by @@ -2300,10 +2288,10 @@ type v3Crew struct { err error } -// read is [roles.Source]: one key, and the fast path is an atomic load and a -// read lock. +// read is [roles.Source]: unchanged configuration costs file metadata checks +// and a read lock, never repeated file reads or JSON decoding. func (c *v3Crew) read(key string) (string, bool) { - if generation, low := c.stampNow(); config.SettingsGeneration() != generation || config.CreditsLowAt(c.profileDir) != low { + if generation, low, files := c.stampNow(); config.SettingsGeneration() != generation || config.CreditsLowAt(c.profileDir) != low || c.configFilesNow() != files { // A rebuild that fails leaves the old snapshot in place, so the error is // dropped here on purpose: this is the resolution path, and the honest // answer to "which model" is the last one that parsed. @@ -2320,16 +2308,17 @@ func (c *v3Crew) read(key string) (string, bool) { // stampNow is what the snapshot was built against: the settings generation and // whether the balance record read low. -func (c *v3Crew) stampNow() (uint64, bool) { +func (c *v3Crew) stampNow() (uint64, bool, [2]crewConfigStamp) { c.mu.RLock() defer c.mu.RUnlock() - return c.generation, c.low + return c.generation, c.low, c.files } // rebuild reads the rows and replaces the snapshot. It reads BEFORE taking the // write lock so a slow disk cannot hold a concurrent turn's resolution. func (c *v3Crew) rebuild() error { generation := config.SettingsGeneration() + files := c.configFilesNow() low := config.CreditsLowAt(c.profileDir) values, err := c.snapshot() c.mu.Lock() @@ -2338,7 +2327,7 @@ func (c *v3Crew) rebuild() error { // retried on every single call — that would put a file read back on the hot // path, which is the thing this seam exists to avoid — so the refusal is // recorded and the next write is what triggers another attempt. - c.generation, c.low, c.err = generation, low, err + c.generation, c.low, c.files, c.err = generation, low, files, err if err != nil { return err } diff --git a/cmd/codeaf/chatv3_beside.go b/cmd/codeaf/chatv3_beside.go index 10ea01135e..b3f436d857 100644 --- a/cmd/codeaf/chatv3_beside.go +++ b/cmd/codeaf/chatv3_beside.go @@ -450,6 +450,7 @@ func (b *besideAgent) Detach() error { func hostLink(seams hostSeams) tui3.LinkSeam { return tui3.LinkSeam{ Note: seams.Link, + EngineBuild: seams.EngineBuild, Ping: seams.Ping, Notice: seams.Notice, Held: hostHeld(seams), diff --git a/cmd/codeaf/chatv3_host.go b/cmd/codeaf/chatv3_host.go index 07e4835bfc..981da0ff3e 100644 --- a/cmd/codeaf/chatv3_host.go +++ b/cmd/codeaf/chatv3_host.go @@ -1033,6 +1033,9 @@ func hostAttachedNote(attached int, driving bool) string { // of facts rather than as an argument: the surface has grown // [tui3.LinkSeam] and hostOptions hands all four straight into it. type hostSeams struct { + // EngineBuild follows the client's latest welcome, including replacements. + EngineBuild func() string + // Link is the quiet sentence about the connection right now — empty // whenever there is nothing to say, which is what a status-line segment // draws as nothing at all. It reads `reconnecting to devbox — trying for up @@ -1072,6 +1075,7 @@ func newHostSeams(client *remote.Client) hostSeams { var readInitial atomic.Bool return hostSeams{ Link: client.LinkNote, Ping: client.Ping, Notice: client.TakeNotice, + EngineBuild: func() string { return client.Welcome().Build }, Held: func() ([]remote.HeldQuestion, error) { // Arrival and replay share one snapshot. Re-querying before drawing // it could hide a question re-emitted since arrival: the server has diff --git a/cmd/codeaf/crew_refresh.go b/cmd/codeaf/crew_refresh.go new file mode 100644 index 0000000000..6eb177ceb8 --- /dev/null +++ b/cmd/codeaf/crew_refresh.go @@ -0,0 +1,29 @@ +package main + +import ( + "os" + "time" + + "github.com/Agent-Field/codeaf/internal/config" +) + +// crewConfigStamp is the same freshness contract as internal/filememo: a file +// creation, removal, size change or modification time invalidates the snapshot. +// The in-process generation still covers writes on coarse timestamp clocks. +type crewConfigStamp struct { + Size int64 + Modified time.Time + Exists bool +} + +// configFilesNow notices writes from a settings window or editor in another +// process. Only metadata is read on the resolution path, never file contents. +func (c *v3Crew) configFilesNow() [2]crewConfigStamp { + var stamps [2]crewConfigStamp + for i, path := range []string{config.BudgetConfigPath(c.profileDir), config.ProjectConfigPath(c.workspace)} { + if info, err := os.Stat(path); err == nil { + stamps[i] = crewConfigStamp{Size: info.Size(), Modified: info.ModTime(), Exists: true} + } + } + return stamps +} diff --git a/cmd/codeaf/crew_refresh_test.go b/cmd/codeaf/crew_refresh_test.go new file mode 100644 index 0000000000..2b2cd4bc1a --- /dev/null +++ b/cmd/codeaf/crew_refresh_test.go @@ -0,0 +1,76 @@ +package main + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/config" + "github.com/Agent-Field/codeaf/internal/roles" +) + +// An external settings writer never increments this process's counter. Atomic +// replacement also exercises the way another terminal's settings panel saves. +func writeExternalCrewSettings(t *testing.T, path string, values map[string]string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { + t.Fatal(err) + } + data, err := json.Marshal(values) + if err != nil { + t.Fatal(err) + } + stamp := time.Now().Add(time.Second) + if old, err := os.Stat(path); err == nil { + stamp = old.ModTime().Add(time.Second) + } + tmp := path + ".external" + if err := os.WriteFile(tmp, data, 0600); err != nil { + t.Fatal(err) + } + if err := os.Chtimes(tmp, stamp, stamp); err != nil { + t.Fatal(err) + } + if err := os.Rename(tmp, path); err != nil { + t.Fatal(err) + } +} + +func TestRunningCrewReadsSettingsChangedByAnotherProcess(t *testing.T) { + profile, workspace := t.TempDir(), t.TempDir() + source, err := v3RolesSource(workspace, profile) + if err != nil { + t.Fatal(err) + } + generation := config.SettingsGeneration() + writeExternalCrewSettings(t, config.BudgetConfigPath(profile), map[string]string{config.KeyTierReflexModel: "vendor/first", config.KeyTierLowModel: "vendor/cheap"}) + if config.SettingsGeneration() != generation { + t.Fatal("fixture used in-process writer") + } + resolve := func(role roles.Role, want string) { + t.Helper() + got, err := roles.Resolve(roles.Source(source), role, "vendor/chat") + if err != nil || got != want { + t.Fatalf("role=%s model=%q want=%q error=%v", role, got, want, err) + } + } + resolve(roles.RoleReflex, "vendor/first") + resolve(roles.RoleTitle, "vendor/cheap") + // Same-length replacements must invalidate both the crew snapshot and the + // config parser without requiring a process restart or a file-size change. + writeExternalCrewSettings(t, config.BudgetConfigPath(profile), map[string]string{config.KeyTierReflexModel: "vendor/other", config.KeyTierLowModel: "vendor/newer"}) + resolve(roles.RoleReflex, "vendor/other") + resolve(roles.RoleTitle, "vendor/newer") + writeExternalCrewSettings(t, config.ProjectConfigPath(workspace), map[string]string{config.KeyTierLowModel: "vendor/project"}) + resolve(roles.RoleTitle, "vendor/project") + if err := os.Remove(config.ProjectConfigPath(workspace)); err != nil { + t.Fatal(err) + } + resolve(roles.RoleTitle, "vendor/newer") + if err := os.Remove(config.BudgetConfigPath(profile)); err != nil { + t.Fatal(err) + } + resolve(roles.RoleReflex, config.DefaultReflexModel) +} diff --git a/cmd/codeaf/engine.go b/cmd/codeaf/engine.go index 206c9efc25..01047b2240 100644 --- a/cmd/codeaf/engine.go +++ b/cmd/codeaf/engine.go @@ -396,11 +396,8 @@ func nothingHolds(workspace string) bool { // differentEngineEnvironmentSentence names the host's workspace so the command // stops the folder actually holding the old environment, including from Home. func differentEngineEnvironmentSentence(workspace string) string { - stop := "codeaf engine --stop" - if workspace = strings.TrimSpace(workspace); workspace != "" { - stop += " --workspace " + shellQuote(workspace) - } - return "the engine for this folder started with a different environment than this terminal and is still in use — close its other windows or let its work finish, or run " + stop + stop := engineStopWords(workspace) + return "the background engine for this folder started with a different environment than this terminal and is still in use — after work finishes, close every window for this workspace and reopen, or run " + stop + " (interrupts active turns and tasks)" } // sameEngineBuild is the host being this build: the same wire, the same source, @@ -461,14 +458,11 @@ func staleEngineHostSentence(busy bool, workspace string) string { if strings.TrimSpace(name) == "" { name = "that machine" } - stop := "codeaf engine --stop" - if workspace = strings.TrimSpace(workspace); workspace != "" { - stop += " --workspace " + workspace - } + stop := engineStopWords(workspace) if busy { - return fmt.Sprintf("engine: %s is still running an older codeaf and something is still going in it — let that finish, or run %s on %s", name, stop, name) + return fmt.Sprintf("engine: %s is still running an older codeaf and something is still going in it — let that finish, or run %s on %s (interrupts active turns and tasks)", name, stop, name) } - return fmt.Sprintf("engine: %s is still holding this conversation on an older codeaf — run %s on %s", name, stop, name) + return fmt.Sprintf("engine: %s is still holding this conversation on an older codeaf — run %s on %s (interrupts active turns and tasks)", name, stop, name) } // unknownOlderEngineHostSentence is what a person reads when the process holding @@ -485,7 +479,7 @@ func unknownOlderEngineHostSentence(workspace string) string { name = "that machine" } stop := engineStopWords(workspace) - return fmt.Sprintf("engine: %s is holding this conversation on an older codeaf that cannot say what it is or what it is doing, and this build will not end work it cannot see — run %s on %s if you want it stopped", name, stop, name) + return fmt.Sprintf("engine: %s is holding this conversation on an older codeaf that cannot say what it is or what it is doing, and this build will not end work it cannot see — run %s on %s if you want it stopped (interrupts active turns and tasks)", name, stop, name) } // otherProtocolEngineHostSentence is the refusal for an older engine on a wire @@ -499,14 +493,21 @@ func otherProtocolEngineHostSentence(workspace string) string { name = "that machine" } stop := engineStopWords(workspace) - return fmt.Sprintf("engine: %s is holding this conversation on a codeaf that speaks a different protocol, and something is still going in it — let that finish, or run %s on %s", name, stop, name) + return fmt.Sprintf("engine: %s is holding this conversation on a codeaf that speaks a different protocol, and something is still going in it — let that finish, or run %s on %s (interrupts active turns and tasks)", name, stop, name) } // engineStopWords is the command a refusal tells a person to run: the explicit // stop, naming and quoting the workspace so a folder with a space in it survives // being copied out of a sentence and pasted into a shell. func engineStopWords(workspace string) string { - stop := "codeaf engine --stop" + return engineStopWordsAs(enginehost.ThisBinary(), workspace) +} + +func engineStopWordsAs(binary, workspace string) string { + if strings.TrimSpace(binary) == "" { + binary = "codeaf" + } + stop := shellQuote(binary) + " engine --stop" if workspace = strings.TrimSpace(workspace); workspace != "" { stop += " --workspace " + shellQuote(workspace) } @@ -560,10 +561,11 @@ func busyEngineHostSentence(workspace string, waits bool) string { if strings.TrimSpace(name) == "" { name = "this machine" } + note := fmt.Sprintf("this window is using an older codeaf background engine on %s; restarting one window does not update it. Attached windows count as in use. After work finishes, close every window for this workspace and reopen", name) if waits { - return fmt.Sprintf("the engine on %s is an older codeaf and is still holding work — it keeps that work and picks up this build once it goes quiet", name) + note += " — the engine will step aside when all windows and work are gone" } - return fmt.Sprintf("the engine on %s is an older codeaf and is still holding work — it keeps that work, and your next launch once the work is done opens from this build", name) + return note + ". Or run " + engineStopWords(workspace) + " on " + name + " (interrupts active turns and tasks); reconnect with this build" } // replacedEngineHostSentence is the ONE LINE a takeover owes: which engine was @@ -658,10 +660,16 @@ func writeEngineStatus(out io.Writer, workspace string, held enginehost.Holder, verdict := "this build" switch { case !held.Answered: - verdict = "an older build — the next codeaf launched here replaces it" + verdict = "an older build — its activity is unknown; a new window will not replace it" case sameEngineBuild(host, me): case host.BuiltAt.Before(me.BuiltAt): - verdict = "an older build — the next codeaf launched here replaces it" + if host.Busy && host.Version != remote.Version { + verdict = "an older incompatible build — still in use; a new window cannot join it" + } else if host.Busy { + verdict = "an older build — still in use; a new window keeps using it" + } else { + verdict = "an older build — idle; the next launch replaces it" + } default: verdict = "a newer build than this binary" } @@ -681,17 +689,18 @@ func writeEngineStatus(out io.Writer, workspace string, held enginehost.Holder, if held.Answered { fmt.Fprintf(out, " windows %d attached · %s open\n", host.Surfaces, countWord(host.Conversations, "conversation", "conversations")) if host.Busy { - fmt.Fprintln(out, " working yes — a turn, a task or a question is in flight") + fmt.Fprintln(out, " in use yes — an attached window, turn, task or question keeps it running") } } if enginehost.EnvironmentDiffers(workspace) { fmt.Fprintln(out, " environment differs from this terminal's") } - stop := "codeaf engine --stop" - if strings.TrimSpace(workspace) != "" { - stop += " --workspace " + workspace + fmt.Fprintf(out, " stop it %s (interrupts active turns and tasks)\n", engineStopWords(workspace)) + if !held.Answered { + fmt.Fprintln(out, " update activity is unknown; when ready, explicitly stop this workspace engine and reconnect with this build") + } else if !sameEngineBuild(host, me) && host.BuiltAt.Before(me.BuiltAt) && host.Busy { + fmt.Fprintln(out, " update after work finishes, close every window for this workspace and reopen with this build; restarting one window is insufficient") } - fmt.Fprintf(out, " stop it %s\n", stop) } // roughAge is a duration the way a person reads one on a status line. diff --git a/cmd/codeaf/engine_stale_test.go b/cmd/codeaf/engine_stale_test.go index 73d4542e8a..3e51afdf86 100644 --- a/cmd/codeaf/engine_stale_test.go +++ b/cmd/codeaf/engine_stale_test.go @@ -268,7 +268,7 @@ func TestAnOlderEngineHoldingWorkKeepsItAndIsJoined(t *testing.T) { if err != nil { t.Fatalf("an older engine holding work was refused rather than joined: %v", err) } - for _, want := range []string{"older codeaf", "still holding work", "picks up this build once it goes quiet"} { + for _, want := range []string{"older codeaf background engine", "Attached windows count as in use", "all windows and work are gone", "interrupts active turns and tasks", engineStopWords(workspace)} { if !strings.Contains(note, want) { t.Fatalf("the busy line %q does not say %q", note, want) } @@ -336,7 +336,7 @@ func TestAnOlderEngineThatRefusedTheStandDownKeepsItsWork(t *testing.T) { if err != nil { t.Fatalf("a host that kept its work was refused rather than joined: %v", err) } - if !strings.Contains(note, "older codeaf") || !strings.Contains(note, "still holding work") { + if !strings.Contains(note, "older codeaf") || !strings.Contains(note, "Attached windows count as in use") { t.Fatalf("the busy line %q does not say what happened", note) } if !hostAnswers(workspace) { @@ -368,7 +368,7 @@ func TestAnOldHostThatIgnoresTheNoteIsNotPromisedAQuietMoment(t *testing.T) { if strings.Contains(note, "once it goes quiet") { t.Fatalf("the line promises a quiet-moment handover this host was never told about: %q", note) } - for _, want := range []string{"older codeaf", "still holding work", "your next launch once the work is done"} { + for _, want := range []string{"older codeaf", "close every window", "restarting one window does not update it"} { if !strings.Contains(note, want) { t.Fatalf("the line %q does not say %q", note, want) } @@ -430,8 +430,8 @@ func TestAnOlderBuildWithNoStampHoldingWorkIsJoined(t *testing.T) { if !strings.Contains(note, "older codeaf") { t.Fatalf("the notice did not say which engine this is: %q", note) } - if strings.Contains(note, "--stop") { - t.Fatalf("the notice sends somebody off to stop something by hand: %q", note) + if !strings.Contains(note, "After work finishes, close every window") || !strings.Contains(note, "interrupts active turns and tasks") { + t.Fatalf("the notice does not offer a safe update and warn about explicit stop: %q", note) } if !hostAnswers(workspace) || !host.waitingSaid() || host.forced() { t.Fatal("a stampless older build was ended rather than joined") @@ -789,7 +789,7 @@ func TestStatusNamesTheEngineHoldingTheWorkspace(t *testing.T) { "build a1b2c3d4 built 2026-09-21 09:00", "(43h00m ago)", "2 attached · 3 conversations open", - "stop it codeaf engine --stop --workspace " + workspace, + "stop it " + engineStopWords(workspace), } { if !strings.Contains(said, want) { t.Fatalf("status does not say %q:\n%s", want, said) @@ -815,3 +815,35 @@ func TestStatusSaysNoneWhenNothingHoldsTheWorkspace(t *testing.T) { t.Fatalf("status of an empty workspace said %q", got) } } + +func TestEngineStopGuidanceQuotesTheInstalledExecutableAndWorkspace(t *testing.T) { + got := engineStopWordsAs("/Applications/Dev Build/devaf", "/Users/somebody/work's folder") + want := shellQuote("/Applications/Dev Build/devaf") + " engine --stop --workspace " + shellQuote("/Users/somebody/work's folder") + if got != want { + t.Fatalf("restart command = %q, want %q", got, want) + } +} + +func TestEngineStatusUpgradeGuidanceMatchesAdmission(t *testing.T) { + shortEngineHome(t) + for _, tc := range []struct { + name string + answered, busy bool + want string + }{ + {"busy", true, true, "a new window keeps using it"}, + {"idle", true, false, "idle; the next launch replaces it"}, + {"unknown", false, false, "a new window will not replace it"}, + } { + t.Run(tc.name, func(t *testing.T) { + var out strings.Builder + writeEngineStatus(&out, "/work/with space", enginehost.Holder{Answered: tc.answered, Self: remote.HostSelf{Version: remote.Version, BuiltAt: earlier, Busy: tc.busy, Surfaces: 1}}, window("today", later), time.Now()) + if !strings.Contains(out.String(), tc.want) { + t.Fatal(out.String()) + } + if strings.Contains(out.String(), "a turn, a task or a question is in flight") { + t.Fatal("attached windows falsely reported as active work") + } + }) + } +} diff --git a/internal/config/budget.go b/internal/config/budget.go index d60e4bc4b4..4d6131e35d 100644 --- a/internal/config/budget.go +++ b/internal/config/budget.go @@ -65,9 +65,8 @@ func WriteDailyBudgetUSD(profileDir string, amount float64) error { // internal/filememo spends one stat to decide whether it may answer, and // [SettingsGeneration] is folded in beside it so a value this process persisted // can never be served back stale, whatever a filesystem's timestamp resolution -// is. What the memo does not see is what the counter below already documents it -// cannot: a config.json edited by hand in another process, which has always -// landed on the next launch. +// is. File metadata also notices ordinary writes by another process; the +// counter covers this process's writes where timestamp granularity cannot. var profileConfigMemo = filememo.Stamped(SettingsGeneration, func(_ string, data []byte, missing bool) (map[string]json.RawMessage, error) { // A file that is not there yet is an empty object, not an error. @@ -251,11 +250,10 @@ func errorKey(updates map[string]any) string { // stale, because it re-stats the file on every read and reads this counter // beside it.) // -// WHAT IT DOES NOT SEE, stated plainly: a config.json edited by hand in another -// process, and a project file (`/.codeaf/config.json`) edited by -// anything. Both are the same as the behaviour before a counter existed — those -// changes have always landed on the next launch — and a counter that pretended -// otherwise would need a watcher on two files per session. +// The counter alone does not see another process's writes. Live snapshot +// readers must pair it with file metadata: the v3 crew source does this for +// both profile and project configuration, and profileConfigMemo does it for +// parsed profile values. No watcher or repeated JSON decode is required. var settingsGeneration atomic.Uint64 diff --git a/internal/e2e/engine_upgrade_e2e_test.go b/internal/e2e/engine_upgrade_e2e_test.go new file mode 100644 index 0000000000..86e851fd9d --- /dev/null +++ b/internal/e2e/engine_upgrade_e2e_test.go @@ -0,0 +1,181 @@ +//go:build e2e + +package e2e + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/config" +) + +// The old binary is built separately with make build and an earlier build stamp. +// Both windows and the background engine are real; no protocol answers or UI +// frames are fabricated. Only this fixture's workspace is explicitly stopped. +func TestOlderEngineUpgradeOnTheRealTerminal(t *testing.T) { + oldBinary := os.Getenv("CODEAF_E2E_OLD_BINARY") + if oldBinary == "" { + t.Skip("set CODEAF_E2E_OLD_BINARY to an older make-build binary") + } + requireTmuxAndKey(t) + const model = "deepseek/deepseek-v4.1-flash" + if e2eModel != model { + t.Fatalf("requires CODEAF_E2E_MODEL=%s", model) + } + home := newHome(t, map[string]any{"memory.enabled": false}) + ws := newWorkspace(t, "engine-upgrade", false) + env := []string{config.APIKeyEnv + "=" + liveKey(t), "CODEAF_BASE_URL=" + config.DefaultBaseURL} + old := startUpgradeOldWindow(t, oldBinary, env, home, ws) + old.skipSetup(t) + prompt := "Create upgrade-proof.txt containing exactly amber lighthouse. Do not modify any other file, delegate, or use memory tools." + old.lit(prompt) + old.keys("Enter") + if answer, _ := privateMemoryAwaitAnswer(home, prompt, 2*time.Minute); answer == "" { + t.Fatal("old window did not finish the real file request") + } + assertUpgradeFile(t, ws) + before := upgradeEngineCommand(t, binary(t), env, home, ws, "--status") + pid := upgradePID(before) + if pid == "" || !strings.Contains(before, "older build") || !strings.Contains(before, "still in use") { + t.Fatalf("fixture did not hold an older busy engine: %s", before) + } + current := startWithEnv(t, env, "engine_upgrade_current", home, ws, 180, 52, "chat", "--model", model) + current.skipSetup(t) + if strings.Contains(current.capture(), "Move this conversation here?") { + current.keys("1", "Enter") + time.Sleep(time.Second) + } + screen := current.waitFor(15*time.Second, "older codeaf background engine", "restarting one window", "close every window") + if !strings.Contains(screen, "older codeaf background engine") { + t.Fatal("matched-shape current window omitted older engine guidance") + } + t.Logf("Actual terminal explains the older engine and safe update:\n%s", screen) + if !strings.Contains(before, "restarting one window is insufficient") || !strings.Contains(before, "close every window") { + t.Fatalf("engine status omitted safe upgrade guidance: %s", before) + } + during := upgradeEngineCommand(t, binary(t), env, home, ws, "--status") + if upgradePID(during) != pid { + t.Fatal("opening a new window interrupted the older busy engine") + } + current.lit("/status") + current.keys("Enter") + status := current.waitFor(15*time.Second, "window build", "engine build") + if !strings.Contains(status, "engine build") { + t.Fatal("matched-shape attached window omitted engine build status") + } + t.Logf("Actual status distinguishes window and engine builds:\n%s", status) + // The old surface must be closed before the explicit stop, so its old binary + // cannot win the redial race and put the workspace back on the old build. + old.kill() + stopped := upgradeEngineCommand(t, binary(t), env, home, ws, "--stop") + t.Logf("Explicit stop of the isolated fixture engine: %s", strings.TrimSpace(stopped)) + deadline := time.Now().Add(30 * time.Second) + var after string + for { + after = upgradeEngineCommand(t, binary(t), env, home, ws, "--status") + if strings.Contains(after, "engine: this build") && upgradePID(after) != "" && upgradePID(after) != pid { + break + } + if time.Now().After(deadline) { + t.Fatalf("current window did not reconnect to current engine: %s", after) + } + time.Sleep(pollEvery) + } + current.keys("Escape") + current.lit("/status") + current.keys("Enter") + status = current.waitFor(15*time.Second, "window build", "engine build") + // The current build's revision is printed by the same shipped binary. Its + // presence on the engine-build line proves the reconnect refreshed the getter. + version := upgradeEngineVersion(t, binary(t), env, home) + fields := strings.Fields(version) + if len(fields) < 2 { + t.Fatalf("bad version output: %s", version) + } + engineLine := "" + for _, line := range strings.Split(status, "\n") { + if strings.Contains(line, "engine build") { + engineLine = line + } + } + if !strings.Contains(engineLine, fields[1]) { + t.Fatalf("reconnected status retained stale engine build: %s; current %s", engineLine, version) + } + assertUpgradeFile(t, ws) + for _, m := range callModels(t, home) { + if m != model { + t.Fatalf("unexpected actual model %q", m) + } + } + t.Logf("PASS: busy older engine kept its PID; window explains update; explicit stop reconnects to this build; original file preserved.\n%s", status) +} + +func startUpgradeOldWindow(t *testing.T, oldBinary string, env []string, home, ws string) *rig { + t.Helper() + g := guardHost(t, home) + name := fmt.Sprintf("p%d-engine_upgrade_old", os.Getpid()) + command := []string{"env", "-u", "CODEAF_TASK_BELT"} + command = append(command, env...) + command = append(command, g.tokens(env)...) + command = append(command, "CODEAF_HOME="+home, "TERM=xterm-256color", oldBinary, "chat", "--model", e2eModel) + quoted := make([]string, len(command)) + for i, p := range command { + quoted[i] = shellQuote(p) + } + if out, err := guardedCommand(t, context.Background(), home, os.Environ(), "tmux", "new-session", "-d", "-s", name, "-c", ws, "sleep 600").CombinedOutput(); err != nil { + t.Fatalf("old tmux: %v %s", err, out) + } + r := &rig{t: t, name: name, home: home, host: g, ws: ws} + t.Cleanup(func() { + if t.Failed() { + r.dump() + } + r.kill() + }) + r.resize(180, 52) + if out, err := guardedCommand(t, context.Background(), home, os.Environ(), "tmux", "respawn-window", "-k", "-t", name, "-c", ws, strings.Join(quoted, " ")).CombinedOutput(); err != nil { + t.Fatalf("old launch: %v %s", err, out) + } + r.waitFor(45*time.Second, "codeaf") + return r +} + +func upgradeEngineCommand(t *testing.T, bin string, env []string, home, ws, flag string) string { + t.Helper() + cmd := guardedCommand(t, context.Background(), home, append(append(os.Environ(), env...), "CODEAF_HOME="+home), bin, "engine", flag, "--workspace", ws) + out, err := cmd.CombinedOutput() + if err != nil && flag != "--status" { + t.Fatalf("engine %s: %v %s", flag, err, out) + } + return string(out) +} +func upgradeEngineVersion(t *testing.T, bin string, env []string, home string) string { + t.Helper() + cmd := guardedCommand(t, context.Background(), home, append(append(os.Environ(), env...), "CODEAF_HOME="+home), bin, "version") + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatal(err) + } + return string(out) +} +func upgradePID(status string) string { + for _, line := range strings.Split(status, "\n") { + fields := strings.Fields(line) + if len(fields) == 2 && fields[0] == "pid" { + return fields[1] + } + } + return "" +} +func assertUpgradeFile(t *testing.T, ws string) { + t.Helper() + raw, err := os.ReadFile(filepath.Join(ws, "upgrade-proof.txt")) + if err != nil || strings.TrimSpace(string(raw)) != "amber lighthouse" { + t.Fatalf("upgrade lost original mock file: %q %v", raw, err) + } +} diff --git a/internal/e2e/memory_invalid_helpers_e2e_test.go b/internal/e2e/memory_invalid_helpers_e2e_test.go new file mode 100644 index 0000000000..9533657f5e --- /dev/null +++ b/internal/e2e/memory_invalid_helpers_e2e_test.go @@ -0,0 +1,355 @@ +//go:build e2e + +package e2e + +import ( + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "net/http/httputil" + "net/url" + "os" + "path/filepath" + "strings" + "sync" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/calllog" + "github.com/Agent-Field/codeaf/internal/config" + "github.com/Agent-Field/codeaf/internal/gitidentity" + "github.com/Agent-Field/codeaf/internal/roles" + "github.com/Agent-Field/codeaf/internal/store" +) + +// Invalid helper IDs are rejected by this local fault injector, never sent to +// OpenRouter. The real terminal must recover through the conversation model, +// save automatically, recall in a fresh chat, correct, and apply the saved rule. +func TestMemoryRecoversFromInvalidHelpersOnTheRealTerminal(t *testing.T) { + requireTmuxAndKey(t) + const model = "deepseek/deepseek-v4.1-flash" + if e2eModel != model { + t.Fatalf("requires CODEAF_E2E_MODEL=%s", model) + } + target, _ := url.Parse(config.DefaultBaseURL) + proxy := httputil.NewSingleHostReverseProxy(target) + director := proxy.Director + proxy.Director = func(r *http.Request) { director(r); r.Host = target.Host } + proxy.FlushInterval = -1 + var mu sync.Mutex + rejected := map[string]int{} + forwarded := 0 + var observed []string + const primary = "fixture/unavailable-memory-primary" + const low = "fixture/unavailable-memory-low" + const updated = "fixture/unavailable-memory-updated" + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "/chat/completions") { + body, err := io.ReadAll(r.Body) + if err != nil { + http.Error(w, "read", 400) + return + } + r.Body = io.NopCloser(strings.NewReader(string(body))) + var request struct { + Model string `json:"model"` + Messages []struct { + Content json.RawMessage `json:"content"` + } `json:"messages"` + } + if json.Unmarshal(body, &request) != nil { + http.Error(w, "json", 400) + return + } + isMemory := len(request.Messages) > 0 && strings.Contains(string(request.Messages[0].Content), "You read one exchange between a person and an assistant") + mu.Lock() + if request.Model == primary || request.Model == low || request.Model == updated { + rejected[request.Model]++ + mu.Unlock() + w.Header().Set("Content-Type", "application/json") + if request.Model == low { + w.WriteHeader(http.StatusBadRequest) + _, _ = io.WriteString(w, `{"error":{"message":"fixture/unavailable-memory-low is not a valid model ID","code":400}}`) + } else { + w.WriteHeader(http.StatusNotFound) + _, _ = io.WriteString(w, `{"error":{"message":"No endpoints found for fixture model","code":404}}`) + } + return + } + if request.Model != model { + mu.Unlock() + http.Error(w, "unexpected model blocked by acceptance fixture", 400) + return + } + observed = append(observed, request.Model) + if isMemory { + forwarded++ + } + mu.Unlock() + } + // Strip the fixture prefix; everything after the injected outage is an + // authenticated request to the actual OpenRouter endpoint. + r.URL.Path = strings.TrimPrefix(r.URL.Path, "/fixture") + proxy.ServeHTTP(w, r) + })) + defer server.Close() + var pins []string + for _, role := range textRoles { + chosen := model + if role == roles.RoleReflex { + chosen = primary + } + pins = append(pins, string(role)+":"+chosen) + } + home := newHome(t, map[string]any{ + "memory.enabled": true, + config.KeyTierReflexModel: primary, + config.KeyTierLowModel: low, + config.KeyModelRoles: strings.Join(pins, ","), + config.KeyModelFallbacks: "", + }) + ws := newWorkspace(t, "juniper-recovery", false) + project, err := gitidentity.ProjectKey(ws) + if err != nil { + t.Fatal(err) + } + st, err := store.Open(filepath.Join(home, "graph.db")) + if err != nil { + t.Fatal(err) + } + defer st.Close() + r := startWithEnv(t, []string{config.APIKeyEnv + "=" + liveKey(t), "CODEAF_BASE_URL=" + server.URL + "/fixture"}, "memory_invalid_helpers", home, ws, 150, 45, "chat", "--model", model, "--debug") + r.skipSetup(t) + prompt := "For the fictional Juniper demo, our standing packaging convention is violet envelopes for weekend parcels. Please acknowledge in one sentence without calling any tools." + r.lit(prompt) + r.keys("Enter") + answer, tools := privateMemoryAwaitAnswer(home, prompt, 2*time.Minute) + if answer == "" { + t.Fatal("chat did not finish during invalid helper recovery") + } + for _, tool := range tools { + if strings.Contains(tool, "remember") { + t.Fatal("main turn bypassed background recovery") + } + } + deadline := time.Now().Add(2 * time.Minute) + for { + rows, err := st.MemoriesFor([]string{store.OwnerProject(project), store.OwnerUser}) + if err != nil { + t.Fatal(err) + } + n := 0 + for _, m := range rows { + if m.Status == store.MemoryActive && strings.Contains(strings.ToLower(m.Text), "violet") && strings.Contains(strings.ToLower(m.Text), "weekend") { + if m.Origin != store.OriginNoticed { + t.Fatalf("memory was not saved by background noticing: origin=%s", m.Origin) + } + n++ + } + } + if n > 1 { + t.Fatal("recovery duplicated the memory") + } + if n == 1 { + break + } + if time.Now().After(deadline) { + t.Fatal("background recovery never saved original packaging convention") + } + time.Sleep(pollEvery) + } + + // A different process edits the profile while the original engine remains + // attached. A NEW unavailable ID must be attempted, proving this was a + // fresh settings read rather than the cached original failed helper. + stateEnv := []string{config.APIKeyEnv + "=" + liveKey(t), "CODEAF_BASE_URL=" + server.URL + "/fixture"} + beforePID := upgradePID(upgradeEngineCommand(t, binary(t), stateEnv, home, ws, "--status")) + configPath := filepath.Join(home, "config.json") + rawConfig, err := os.ReadFile(configPath) + if err != nil { + t.Fatal(err) + } + var profile map[string]any + if err := json.Unmarshal(rawConfig, &profile); err != nil { + t.Fatal(err) + } + profile[config.KeyTierReflexModel] = updated + profile[config.KeyModelRoles] = strings.ReplaceAll(profile[config.KeyModelRoles].(string), primary, updated) + changedConfig, err := json.Marshal(profile) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(configPath+".updated", changedConfig, 0o600); err != nil { + t.Fatal(err) + } + if err := os.Rename(configPath+".updated", configPath); err != nil { + t.Fatal(err) + } + newRule := "For the fictional Juniper demo, our standing label convention is silver text on auburn labels. Reply only with a brief acknowledgment and call no tools; background memory handles this." + r.lit(newRule) + r.keys("Enter") + if answer, tools := privateMemoryAwaitAnswer(home, newRule, 2*time.Minute); answer == "" { + t.Fatal("hot-settings chat did not finish") + } else { + for _, tool := range tools { + if strings.Contains(tool, "remember") { + t.Fatal("foreground tool bypassed hot-settings background capture") + } + } + } + deadline = time.Now().Add(90 * time.Second) + for { + rows, err := st.MemoriesFor([]string{store.OwnerProject(project), store.OwnerUser}) + if err != nil { + t.Fatal(err) + } + saved := false + for _, m := range rows { + if m.Status == store.MemoryActive && m.Origin == store.OriginNoticed && strings.Contains(strings.ToLower(m.Text), "auburn") { + saved = true + } + } + mu.Lock() + changedAttempt := rejected[updated] > 0 + mu.Unlock() + if saved && changedAttempt { + break + } + if time.Now().After(deadline) { + t.Fatalf("external profile edit not recovered: saved=%t new helper attempted=%t", saved, changedAttempt) + } + time.Sleep(pollEvery) + } + if beforePID == "" || upgradePID(upgradeEngineCommand(t, binary(t), stateEnv, home, ws, "--status")) != beforePID { + t.Fatal("external settings recovery restarted the engine") + } + r.lit("/new") + r.keys("Enter") + time.Sleep(time.Second) + recall := "For the fictional Juniper demo, which envelopes do we use for weekend parcels? Answer from saved memory, without tools." + r.lit(recall) + r.keys("Enter") + recalled, _ := privateMemoryAwaitAnswer(home, recall, 2*time.Minute) + if !strings.Contains(strings.ToLower(recalled), "violet") { + t.Fatalf("fresh chat failed recall: %s", recalled) + } + if strings.Contains(r.capture(), "couldn't save this exchange") { + t.Fatal("recovered memory displayed terminal failure") + } + correction := "For the fictional Juniper demo, our standing weekend-parcel packaging convention is now copper envelopes, replacing violet envelopes. Reply with only a one-sentence acknowledgment. Do not call tools, including remember or forget; background memory handles this independently." + r.lit(correction) + r.keys("Enter") + completed, correctionTools := privateMemoryAwaitAnswer(home, correction, 2*time.Minute) + if completed == "" { + t.Fatal("correction did not finish") + } + for _, tool := range correctionTools { + if strings.Contains(tool, "remember") || strings.Contains(tool, "forget") { + t.Fatal("main turn bypassed background correction") + } + } + deadline = time.Now().Add(90 * time.Second) + for { + rows, err := st.MemoriesFor([]string{store.OwnerProject(project), store.OwnerUser}) + if err != nil { + t.Fatal(err) + } + copper, violet := 0, 0 + for _, m := range rows { + if m.Status != store.MemoryActive { + continue + } + text := strings.ToLower(m.Text) + if strings.Contains(text, "weekend") { + if strings.Contains(text, "copper") { + if m.Origin != store.OriginNoticed { + t.Fatalf("correction bypassed background noticing: %s", m.Origin) + } + copper++ + } + if strings.Contains(text, "violet") && !strings.Contains(text, "copper") { + violet++ + } + } + } + if copper == 1 && violet == 0 { + break + } + if time.Now().After(deadline) { + t.Fatalf("correction not applied: copper=%d old=%d", copper, violet) + } + time.Sleep(pollEvery) + } + r.lit("/new") + r.keys("Enter") + time.Sleep(time.Second) + apply := "For the fictional Juniper demo, create packaging.json with a weekend_envelope_color field set from our current convention. Use saved memory and do not ask me to repeat it." + r.lit(apply) + r.keys("Enter") + if completed, _ := privateMemoryAwaitAnswer(home, apply, 2*time.Minute); completed == "" { + t.Fatal("application did not finish") + } + body, err := os.ReadFile(filepath.Join(ws, "packaging.json")) + if err != nil { + t.Fatal(err) + } + var packaging struct { + Color string `json:"weekend_envelope_color"` + } + if err := json.Unmarshal(body, &packaging); err != nil || !strings.EqualFold(packaging.Color, "copper") { + t.Fatalf("saved correction not applied: %s (%v)", body, err) + } + mu.Lock() + defer mu.Unlock() + if rejected[primary] == 0 || rejected[low] == 0 || forwarded == 0 { + t.Fatalf("local primary rejections=%d local fallback rejections=%d live memory requests=%d", rejected[primary], rejected[low], forwarded) + } + + if len(observed) == 0 { + t.Fatal("no requests observed") + } + for _, m := range observed { + if m != model { + t.Fatalf("unexpected actual model %q", m) + } + } + models := callModels(t, home) + if len(models) == 0 { + t.Fatal("no model receipts") + } + for _, m := range models { + if m != model && m != primary && m != low && m != updated { + t.Fatalf("unexpected receipt model %q", m) + } + } + rawReceipts, err := os.ReadFile(filepath.Join(home, "logs", "calls.jsonl")) + if err != nil { + t.Fatal(err) + } + successful := 0 + for _, line := range strings.Split(string(rawReceipts), "\n") { + if strings.TrimSpace(line) == "" { + continue + } + var receipt calllog.Record + if err := json.Unmarshal([]byte(line), &receipt); err != nil { + t.Fatal(err) + } + if receipt.Status >= 200 && receipt.Status < 300 { + successful++ + if receipt.Model != model { + t.Fatalf("successful receipt used unexpected model %q", receipt.Model) + } + } + if (receipt.Model == primary || receipt.Model == low || receipt.Model == updated) && receipt.Status != 0 && receipt.Status != http.StatusNotFound && !(receipt.Model == low && receipt.Status == http.StatusBadRequest) { + t.Fatalf("locally rejected fixture got unexpected status %d", receipt.Status) + } + } + if successful == 0 { + t.Fatal("no successful real-model receipt") + } + + t.Logf("PASS: two locally rejected helper models recovered in background; external profile edit recovered without restart; one saved convention recalled, corrected, and applied to an actual file across fresh chats; %d actual OpenRouter requests used %s; %d total receipts include locally injected failures", len(observed), model, len(models)) + t.Logf("Live terminal after fresh-chat recall:\n%s", r.capture()) +} diff --git a/internal/manual/chat/running-on-another-machine.md b/internal/manual/chat/running-on-another-machine.md index 5d52e2ad24..c40309deb6 100644 --- a/internal/manual/chat/running-on-another-machine.md +++ b/internal/manual/chat/running-on-another-machine.md @@ -172,8 +172,7 @@ as another build. Four things can be true: old process keeps what it was holding and steps aside at its first quiet moment; a host built before this ask ignores the note, so for one of those the line says the work is kept and **your next safe launch once the work is done and every window has left** opens from - the new build. An attached idle window counts as holding work, so close it before the - handover. + the new build. An attached idle window counts as holding work. After turns and tasks finish, close every window for that workspace and reopen. Restarting just one window keeps joining the old engine while another window remains. The startup note names the machine and an explicit stop command using its installed executable; that command interrupts active turns and tasks, so use it only when ready. - **It is an older build, holding nothing.** It hands the workspace over at once and the window says `replaced the older engine on spark (pid , , ) — this build holds the workspace now`. diff --git a/internal/manual/chat/staying-on-that-machine.md b/internal/manual/chat/staying-on-that-machine.md index d25e5eced1..64690cceb1 100644 --- a/internal/manual/chat/staying-on-that-machine.md +++ b/internal/manual/chat/staying-on-that-machine.md @@ -474,7 +474,9 @@ A plain `codeaf` does not run your conversation inside the window — the sessio in a process of its own, so it survives the terminal closing. That process also outlives the build that started it. -**An older engine holding work is joined, and steps aside at its first quiet moment.** Your +**A new window can still use an old background engine. Restarting one window does not update that engine.** + +An older engine holding work is joined, and steps aside at its first quiet moment. Your window attaches and your work carries on; the old process keeps what it was holding and is asked to retire once nothing is in flight. If it was holding nothing, the workspace is handed over at once and you are told in one line: @@ -490,6 +492,10 @@ waits until you close it; only a stalled window whose link is gone is ignored. A before this ask ignores it, so for one of those the old process keeps its work and **your next safe launch — after the work is done and every window has left — opens from this build**. +For a safe update, let active turns and tasks finish, close **every window for this workspace**, then reopen with the updated binary. `engine --status --workspace ` names the running build and attached windows. Attached windows count as in use even when no reply is running; status does not treat that alone as proof of an active turn. + +The startup note also gives an explicit, shell-quoted `engine --stop --workspace ` command using the installed executable (for example `devaf`). Run it on the named machine only when ready: **it interrupts active turns and tasks in that workspace**. Reconnect afterward with the updated build. Other workspace engines are unaffected. + When both ends support restart detection, that reconnect retires the old reply channels before displaying new replies. If an answer was still open, the window says `the engine restarted; the previous answer stopped`. Your saved conversation remains; diff --git a/internal/manual/chat/what-i-remember.md b/internal/manual/chat/what-i-remember.md index 5de978b16a..d38283006a 100644 --- a/internal/manual/chat/what-i-remember.md +++ b/internal/manual/chat/what-i-remember.md @@ -389,12 +389,19 @@ every line's words, where it came from and whether it was active. A line an old store kept as "project" without naming the project is kept under the Memories tab's `couldn't tell which project` group until you move it. -A window held by an engine keeps that engine until a newer build replaces it. -Update codeaf on the machine holding the workspace and reconnect; on that same -machine a newer build replaces the older engine as the window connects. For an -engine on another machine, update codeaf there too, then run -`codeaf engine --stop --workspace ` there when its work is safe and -reconnect. `codeaf engine --status` names the engine. +The window and the background engine can be on different builds. Updating the +executable and reopening one window does not replace an older engine while +workspace windows remain attached. `/status` shows **window build** and **engine +build**, and follows a replacement after reconnection. A newer window can still +be served by old memory code until that engine changes. + +Let current work finish, then close all windows attached to that workspace and +reopen codeaf. To restart explicitly, run the exact workspace-specific command +shown by `codeaf engine --status --workspace ` on the machine holding +it. The command uses the installed executable, including `devaf` when that is +what you installed. Stopping an engine interrupts its active turns and tasks; +other workspaces keep their own engines. “In use” can mean attached windows, +not necessarily a running turn. ## Does memory work on the ordinary launch, over --host, and on a smaller model? @@ -468,11 +475,15 @@ once, keeps the conversation's own running summary of where the work stands, and `/settings` → Models (`google/gemini-2.5-flash` by default; nothing set means the conversation's own model), sends no token ceiling or reasoning switch, and is charged to the session total, so `/cost` includes it. An answer it cannot read -costs one retry; a model that answers nothing is replaced for the session by the -**small work** model, with one line saying so. A stalled or temporarily failed -request reserves time for one small-work attempt within the same 45-second -limit. If that also fails temporarily, the original exchange is tried once more -after two seconds in the background. Turning noticing off does not +costs one repair. An empty or unavailable helper is skipped for that chat; +recovery tries the configured **small work** model, then the current chat model, +without changing your saved preferences. Attempts share 45 seconds, reserving +time for each remaining distinct model. A notice identifies a substitute only +after it returns a readable answer. `/settings` → Models lets you fix the helper; +the running engine picks up saved helper changes on its next call without a +restart. If all attempts fail temporarily, the original exchange is tried once +more after two seconds in the background. Authentication, payment, quota and +policy refusals do not start another model chain. Turning noticing off does not remove the call, because the running summary still needs it; it only stops lines being kept. diff --git a/internal/provider/routefailure.go b/internal/provider/routefailure.go index bf6c9423ae..aa3aadf3b4 100644 --- a/internal/provider/routefailure.go +++ b/internal/provider/routefailure.go @@ -104,6 +104,10 @@ func RouteFailureOf(err error) (RouteFailure, time.Time) { return RouteUnavailable, time.Time{} case refusal.Status == http.StatusForbidden || refusal.PlanUnavailable || holdsAny(said, forbiddenPhrases): return RouteForbidden, time.Time{} + // OpenRouter rejects an unknown model slug with 400 rather than 404. + // Match its model-specific refusal, never a generic malformed request. + case refusal.Status == http.StatusBadRequest && (strings.Contains(strings.ToLower(refusal.Message), "is not a valid model id") || strings.EqualFold(refusal.Code, "model_not_found")): + return RouteUnavailable, time.Time{} case refusal.Status == http.StatusNotFound: return RouteUnavailable, time.Time{} case refusal.Status >= http.StatusInternalServerError: diff --git a/internal/provider/routefailure_test.go b/internal/provider/routefailure_test.go index 765b500483..95ac464cc9 100644 --- a/internal/provider/routefailure_test.go +++ b/internal/provider/routefailure_test.go @@ -25,6 +25,9 @@ func TestRouteFailureReadsEachProvidersWordsAsOneKind(t *testing.T) { {"free pool for harnesses only", 403, `{"error":{"message":"thinkingmachines/inkling-small:free is only available on agentic harnesses","code":403}}`, RouteForbidden}, {"privacy excluded every endpoint", 404, `{"error":{"message":"No endpoints found matching your data policy (Free model publication). Configure: https://openrouter.ai/settings/privacy","code":404}}`, RouteForbidden}, {"rate limited", 429, `{"error":{"message":"Rate limit exceeded: free-models-per-day. ","code":429}}`, RouteQuota}, + {"openrouter invalid slug", 400, `{"error":{"message":"fixture/no-model is not a valid model ID","code":400}}`, RouteUnavailable}, + {"model code", 400, `{"error":{"message":"unknown model","code":"model_not_found"}}`, RouteUnavailable}, + {"invalid model option is request error", 400, `{"error":{"message":"max_tokens is not a valid model option","code":400}}`, ""}, {"model gone", 404, `{"error":{"code":"NOT_FOUND","message":"Model not found, inaccessible, and/or not deployed"}}`, RouteUnavailable}, {"upstream 5xx", 503, `{"error":{"message":"Service Unavailable","code":503}}`, RouteTransient}, {"our own bytes", 400, `{"error":{"message":"messages: at least one message is required","code":400}}`, ""}, diff --git a/internal/reflex/recovery_test.go b/internal/reflex/recovery_test.go index 5094558f4c..e84d939008 100644 --- a/internal/reflex/recovery_test.go +++ b/internal/reflex/recovery_test.go @@ -3,12 +3,14 @@ package reflex import ( "context" "errors" + "fmt" "reflect" "testing" "testing/synctest" "time" "github.com/Agent-Field/agentfield/sdk/go/ai" + "github.com/Agent-Field/codeaf/internal/provider" "github.com/Agent-Field/codeaf/internal/roles" ) @@ -51,7 +53,11 @@ func TestStalledMemoryRequestLeavesTimeForFallback(t *testing.T) { time.Sleep(time.Second) return reply(`{"keep":"Use tabs","scope":"user"}`), nil } - client := (&Session{}).Bind(c, "primary", "fallback", func(string) { t.Error("transient recovery announced permanent failure") }) + client := (&Session{}).Bind(c, "primary", "fallback", func(text string) { + if text != "memory helper recovered using fallback; your model settings are unchanged" { + t.Fatal(text) + } + }) found, err := Notice(context.Background(), client, "I use tabs", "okay", nil, true) if err != nil || found.Keep != "Use tabs" { t.Fatalf("found=%+v error=%v", found, err) @@ -123,3 +129,115 @@ func TestPermanentMemoryFailureDoesNotStartFallback(t *testing.T) { t.Fatalf("calls=%v err=%v", c.models, err) } } + +func TestUnavailableMemoryModelsRecoverToConversationAndSkipBrokenRoles(t *testing.T) { + c := &recoveryCompleter{} + c.run = func(ctx context.Context, model string) (*ai.Response, error) { + if model != "conversation" { + return nil, &provider.APIError{Status: 404, Message: "model unavailable"} + } + return reply(`{"keep":"Use tabs","scope":"user"}`), nil + } + var notices []string + client := (&Session{}).Bind(c, "primary", "low", func(s string) { notices = append(notices, s) }, "conversation") + for range 2 { + got, err := Notice(context.Background(), client, "I use tabs", "okay", nil, true) + if err != nil || got.Keep != "Use tabs" { + t.Fatalf("got=%+v err=%v", got, err) + } + } + if !reflect.DeepEqual(c.models, []string{"primary", "low", "conversation", "conversation"}) { + t.Fatal(c.models) + } + if len(notices) != 1 { + t.Fatal(notices) + } +} + +func TestMemoryRecoveryDoesNotEvadeAccountOrPolicyRefusals(t *testing.T) { + for _, status := range []int{401, 402, 403, 429, 400} { + t.Run(fmt.Sprint(status), func(t *testing.T) { + c := &recoveryCompleter{run: func(context.Context, string) (*ai.Response, error) { + return nil, &provider.APIError{Status: status, Message: "request refused"} + }} + _, err := Notice(context.Background(), (&Session{}).Bind(c, "primary", "low", nil, "conversation"), "tabs", "okay", nil, true) + if err == nil || len(c.models) != 1 { + t.Fatalf("models=%v err=%v", c.models, err) + } + }) + } +} + +func TestMemoryRecoveryDeduplicatesUnavailableModels(t *testing.T) { + c := &recoveryCompleter{run: func(context.Context, string) (*ai.Response, error) { return nil, &provider.APIError{Status: 404} }} + _, err := Notice(context.Background(), (&Session{}).Bind(c, "same", "same", nil, "same"), "tabs", "okay", nil, true) + if err == nil || len(c.models) != 1 { + t.Fatalf("models=%v err=%v", c.models, err) + } +} + +func TestThreeStalledMemoryModelsShareOverallPatience(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + start := time.Now() + c := &recoveryCompleter{run: func(ctx context.Context, _ string) (*ai.Response, error) { <-ctx.Done(); return nil, ctx.Err() }} + _, err := Notice(context.Background(), (&Session{}).Bind(c, "primary", "low", nil, "conversation"), "tabs", "okay", nil, true) + if !errors.Is(err, context.DeadlineExceeded) || len(c.models) != 3 || time.Since(start) != roles.PatienceFor(roles.RoleReflex) { + t.Fatalf("models=%v elapsed=%s err=%v", c.models, time.Since(start), err) + } + }) +} + +func TestMemoryRecoveryAnnouncesOnlyAfterValidJSON(t *testing.T) { + c := &recoveryCompleter{} + c.run = func(context.Context, string) (*ai.Response, error) { + if len(c.models) == 1 { + return nil, &provider.APIError{Status: 404} + } + return reply("not json"), nil + } + var notices []string + _, err := Notice(context.Background(), (&Session{}).Bind(c, "primary", "low", func(s string) { notices = append(notices, s) }, "conversation"), "tabs", "okay", nil, true) + if err == nil || len(notices) != 0 { + t.Fatalf("notices=%v err=%v", notices, err) + } +} + +// A slow primary followed by a stale low-tier slug was the real product gap: +// the original exchange still has a third, bounded path to its chat model. +func TestTimedOutPrimaryAndInvalidFallbackStillKeepOriginalExchange(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + c := &recoveryCompleter{} + c.run = func(ctx context.Context, model string) (*ai.Response, error) { + switch model { + case "primary": + <-ctx.Done() + return nil, ctx.Err() + case "low": + return nil, &provider.APIError{Status: 400, Message: "fixture/withdrawn is not a valid model ID"} + default: + if ctx.Err() != nil { + t.Fatal("conversation inherited cancelled attempt") + } + return reply(`{"keep":"Use tabs","scope":"user"}`), nil + } + } + got, err := Notice(context.Background(), (&Session{}).Bind(c, "primary", "low", nil, "conversation"), "Use tabs", "okay", nil, true) + if err != nil || got.Keep != "Use tabs" || !reflect.DeepEqual(c.models, []string{"primary", "low", "conversation"}) { + t.Fatalf("got=%+v models=%v err=%v", got, c.models, err) + } + }) +} + +func TestCompletedMemoryAnswerSurvivesOwnerShutdown(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + c := &recoveryCompleter{run: func(context.Context, string) (*ai.Response, error) { + // The response is already complete when shutdown cancellation arrives. + cancel() + return reply(`{"keep":"Use tabs","scope":"user"}`), nil + }} + got, err := Notice(ctx, (&Session{}).Bind(c, "primary", "low", nil, "conversation"), "Use tabs", "okay", nil, true) + if err != nil || got.Keep != "Use tabs" || len(c.models) != 1 { + t.Fatalf("got=%+v calls=%v err=%v", got, c.models, err) + } +} diff --git a/internal/reflex/reflex.go b/internal/reflex/reflex.go index 403b77e11d..6a371afb86 100644 --- a/internal/reflex/reflex.go +++ b/internal/reflex/reflex.go @@ -24,8 +24,8 @@ // // - REFLEX NEVER BREAKS A TURN. A model that answers with prose, with a code // fence, or with a word this package has never heard of costs ONE repair -// retry. A model that answers nothing gets the session's low-tier fallback. -// A transient request failure also gets one bounded fallback attempt. +// retry. An empty, unavailable or transiently failed model gets bounded +// recovery through the configured low tier and current conversation model. // Remaining failures return [ErrReflexFailed]; the owner may retry the // original exchange in the background without interrupting its turn. // @@ -168,10 +168,11 @@ func Bind(c Completer, model string) Completer { return bound{inner: c, model: model, session: &Session{}} } -// Bind returns a completer that starts on model and, once that model has -// answered nothing at all, uses fallback for the rest of this session. notice is called only on the transition and never while a lock is -// held. -func (s *Session) Bind(c Completer, model, fallback string, notice func(string)) Completer { +// Bind starts on model, recovering through fallback and the optional current +// conversation model within one shared deadline. Empty or unavailable models +// are skipped for this session; transient failures do not change preferences. +// notice reports a substitute once after its answer validates, outside locks. +func (s *Session) Bind(c Completer, model, fallback string, notice func(string), conversation ...string) Completer { model = strings.TrimSpace(model) if model == "" { return c @@ -179,15 +180,20 @@ func (s *Session) Bind(c Completer, model, fallback string, notice func(string)) if s == nil { s = &Session{} } - return bound{inner: c, model: model, fallback: strings.TrimSpace(fallback), session: s, notice: notice} + last := "" + if len(conversation) > 0 { + last = strings.TrimSpace(conversation[0]) + } + return bound{inner: c, model: model, fallback: strings.TrimSpace(fallback), conversation: last, session: s, notice: notice} } type bound struct { - inner Completer - model string - fallback string - session *Session - notice func(string) + inner Completer + model string + fallback string + conversation string + session *Session + notice func(string) } func (b bound) CompleteWithMessages(ctx context.Context, messages []ai.Message, options ...ai.Option) (*ai.Response, error) { @@ -200,43 +206,71 @@ func (b bound) CompleteWithMessages(ctx context.Context, messages []ai.Message, return b.inner.CompleteWithMessages(ctx, messages, pinned...) } -func (b bound) activeModel() string { - if b.session == nil { - return b.model +// candidates preserves role preferences while skipping models proved unusable in +// this conversation. No preference is overwritten by a temporary recovery. +func (b bound) candidates() []string { + var models []string + seen := map[string]bool{} + if b.session != nil { + b.session.mu.Lock() + defer b.session.mu.Unlock() + } + for _, model := range []string{b.model, b.fallback, b.conversation} { + key := normalizeModel(model) + if key == "" || seen[key] { + continue + } + seen[key] = true + if b.session != nil && b.session.unusable[key] { + continue + } + models = append(models, model) } - b.session.mu.Lock() - defer b.session.mu.Unlock() - if b.session.unusable[normalizeModel(b.model)] && !sameModel(b.model, b.fallback) { - return b.fallback + return models +} + +func (b bound) activeModel() string { + models := b.candidates() + if len(models) > 0 { + return models[0] } return b.model } -// abandon moves this session off the primary model and reports whether there is -// a distinct fallback to ask. The calm line is emitted once, after the state is -// visible to every concurrent reflex call. -func (b bound) abandon(model string) bool { - if b.session == nil || sameModel(model, b.fallback) || !sameModel(model, b.model) { - return false +func (b bound) abandon(model string) { + if b.session == nil { + return } - key := normalizeModel(b.model) b.session.mu.Lock() + defer b.session.mu.Unlock() if b.session.unusable == nil { b.session.unusable = make(map[string]bool) } + b.session.unusable[normalizeModel(model)] = true +} + +// recovered is called only after a response passes the reflex JSON reader. +// It reports model substitution, never claims that a memory has been stored. +func (b bound) recovered(model string) { + if model == "" || b.session == nil || b.notice == nil || sameModel(model, b.model) { + return + } + key := normalizeModel(b.model) + ">" + normalizeModel(model) + b.session.mu.Lock() if b.session.notified == nil { b.session.notified = make(map[string]bool) } - b.session.unusable[key] = true say := !b.session.notified[key] b.session.notified[key] = true b.session.mu.Unlock() - if say && b.notice != nil { - b.notice("the reflex model answers nothing; using " + b.fallback + " for this session") + if say { + b.notice("memory helper recovered using " + model + "; your model settings are unchanged") } - return true } +type recoverySelection struct{ model string } +type recoverySelectionKey struct{} + func normalizeModel(model string) string { return strings.ToLower(strings.TrimPrefix(strings.TrimSpace(model), "~")) } @@ -349,12 +383,20 @@ func ask(ctx context.Context, c Completer, system, user string, read func(reply // share the same tier patience across retries rather than wait on HTTP caps. ctx, cancel := context.WithTimeout(ctx, roles.PatienceFor(roles.RoleReflex)) defer cancel() + selection := &recoverySelection{} + ctx = context.WithValue(ctx, recoverySelectionKey{}, selection) + announce := func() { + if b, ok := c.(bound); ok { + b.recovered(selection.model) + } + } messages := []ai.Message{message("system", system), message("user", user)} reply, err := call(ctx, c, messages) if err != nil { return fmt.Errorf("%w: %w", ErrReflexFailed, err) } if read(reply) == nil { + announce() return nil } if strings.TrimSpace(reply) == "" { @@ -375,6 +417,7 @@ func ask(ctx context.Context, c Completer, system, user string, read func(reply if err := read(second); err != nil { return fmt.Errorf("%w: %w", ErrReflexFailed, err) } + announce() return nil } @@ -398,9 +441,9 @@ func ask(ctx context.Context, c Completer, system, user string, read func(reply // // WHAT SURVIVES IS THE PART THAT WAS NEVER ABOUT GENERATION: an answer this // package cannot use must not break a turn. A model that says nothing at all -// abandons this session's reflex model and asks the configured low tier once. -// A transient request failure also tries that fallback, without declaring the -// primary unusable for the rest of the session. Both attempts share one bound. +// skips that model for this session and tries the configured recovery models. +// A transient failure also recovers without declaring the primary unusable. +// Every attempt shares the same owner and overall deadline. func call(ctx context.Context, c Completer, messages []ai.Message) (string, error) { // Every reflex request in the process passes through here, so this is the // one place its rows in the model-call log get their word. @@ -413,64 +456,85 @@ func call(ctx context.Context, c Completer, messages []ai.Message) (string, erro if response == nil { return "", errors.New("the model answered nothing") } - if !AnsweredNothing(response) { - return response.Text(), nil - } - - pinned, ok := c.(bound) - if !ok || !pinned.abandon(boundModel(c)) { - return response.Text(), nil - } - response, err = c.CompleteWithMessages(ctx, messages) - if err != nil { - return "", err - } - if response == nil { - return "", errors.New("the fallback model answered nothing") - } return response.Text(), nil } -// completeWithRecovery reserves time for a transient failure's fallback. -// It returns a response without decoding or applying session failover policy. +// completeWithRecovery shares the owner's deadline across at most three unique +// configured models. Only transient and model-unavailable failures may advance; +// account, payment, quota and policy refusals stay actionable failures. func completeWithRecovery(ctx context.Context, c Completer, messages []ai.Message) (*ai.Response, error) { - // Reserve half of the remaining patience for recovery. Only a bound client - // knows where to send that recovery; unbound callers keep their deadline. - attemptCtx := ctx - cancel := func() {} - pinned, canRecover := c.(bound) - if canRecover && pinned.fallback != "" { - if deadline, ok := ctx.Deadline(); ok { - attemptCtx, cancel = context.WithTimeout(ctx, time.Until(deadline)/2) + pinned, ok := c.(bound) + if !ok { + return c.CompleteWithMessages(ctx, messages) + } + models := pinned.candidates() + if len(models) == 0 { + return nil, errors.New("memory helper models are unavailable; choose a working memory model in model settings") + } + // Preserve one transient retry when both configured roles name the same model. + sameRetry := len(models) == 1 && pinned.fallback != "" && sameModel(models[0], pinned.fallback) + if sameRetry { + models = append(models, models[0]) + } + var last error + for i, model := range models { + if ctx.Err() != nil { + return nil, ctx.Err() } - } - response, err := c.CompleteWithMessages(attemptCtx, messages) - cancel() - if err != nil { - // A cancelled owner never starts another paid request. A transient - // failure tries the small-work model once, with the RESERVED parent - // deadline rather than the primary attempt's expired child context. - kind, _ := provider.RouteFailureOf(err) - if ctx.Err() != nil || !canRecover || pinned.fallback == "" || kind != provider.RouteTransient { - return nil, err + response, err := pinned.completeAttempt(ctx, messages, model, len(models)-i) + // A completed usable answer may arrive as its owner shuts down. Finish + // processing it; cancellation forbids starting another paid attempt. + if err == nil && !AnsweredNothing(response) { + if selected, ok := ctx.Value(recoverySelectionKey{}).(*recoverySelection); ok { + selected.model = model + } + return response, nil } - fallback := pinned - fallback.model = pinned.fallback - fallback.session = nil - response, err = fallback.CompleteWithMessages(ctx, messages) - if err != nil { - return nil, err + if ctx.Err() != nil { + return nil, ctx.Err() } - if response == nil || AnsweredNothing(response) { - return nil, errors.New("the fallback model answered nothing") + var next bool + next, last = pinned.recoverable(model, err, sameRetry) + if !next { + return nil, last } - return response, nil } - return response, nil + return nil, last } -// AnsweredNothing is the one failure this package can do anything about: a call -// that came back with no visible text at all. +// completeAttempt reserves an equal share of the remaining owner deadline for +// each remaining candidate; a cancelled child never contaminates its sibling. +func (b bound) completeAttempt(ctx context.Context, messages []ai.Message, model string, remaining int) (*ai.Response, error) { + attemptCtx := ctx + cancel := func() {} + if deadline, ok := ctx.Deadline(); ok && remaining > 1 { + attemptCtx, cancel = context.WithTimeout(ctx, time.Until(deadline)/time.Duration(remaining)) + } + defer cancel() + b.model, b.session = model, nil + return b.CompleteWithMessages(attemptCtx, messages) +} + +// recoverable advances only for model availability or transient transport +// failures. The same-model compatibility retry applies to transients alone. +func (b bound) recoverable(model string, err error, sameRetry bool) (bool, error) { + if err == nil { + b.abandon(model) + return !sameRetry, errors.New("the memory helper answered nothing") + } + kind, _ := provider.RouteFailureOf(err) + switch kind { + case provider.RouteUnavailable: + b.abandon(model) + return !sameRetry, err + case provider.RouteTransient: + return true, err + default: + return false, err + } +} + +// AnsweredNothing identifies a call that came back with no visible text at all. // // IT USED TO BE "EMPTY AT THE CEILING WE SENT", and that reading died with the // ceiling. What is left is the honest half of it — there is nothing to decode, @@ -482,13 +546,6 @@ func AnsweredNothing(response *ai.Response) bool { return response == nil || strings.TrimSpace(response.Text()) == "" } -func boundModel(c Completer) string { - if pinned, ok := c.(bound); ok { - return pinned.activeModel() - } - return "" -} - // decode reads the model's reply as one JSON object, through the shared // provider decoder — which already tolerates a ```json fence and prose either // side of it by taking the first balanced {...} it can parse, and refuses to diff --git a/internal/reflex/thinking_test.go b/internal/reflex/thinking_test.go index 4e0908757b..9f93e42471 100644 --- a/internal/reflex/thinking_test.go +++ b/internal/reflex/thinking_test.go @@ -99,7 +99,7 @@ func TestABoundEmptyAnswerFallsBackOnceAndSaysSoOnce(t *testing.T) { t.Fatalf("call %d added max_tokens", call+1) } } - if len(notices) != 1 || notices[0] != "the reflex model answers nothing; using "+low+" for this session" { + if len(notices) != 1 || notices[0] != "memory helper recovered using "+low+"; your model settings are unchanged" { t.Fatalf("notices = %q, want the one fallback line", notices) } diff --git a/internal/session/memory.go b/internal/session/memory.go index 7e75fef899..feb70480b4 100644 --- a/internal/session/memory.go +++ b/internal/session/memory.go @@ -1004,7 +1004,7 @@ func (a *Agent) reflexClient() reflex.Completer { if routed == nil { return nil } - return a.memory.reflex.Bind(billedCompleter{agent: a, inner: routed}, named, fallback, a.sayMemory) + return a.memory.reflex.Bind(billedCompleter{agent: a, inner: routed}, named, fallback, a.sayMemory, model) } // billedCompleter is what makes a reflex call cost something a person can see: diff --git a/internal/tui3/engine_build_test.go b/internal/tui3/engine_build_test.go new file mode 100644 index 0000000000..05575a5acf --- /dev/null +++ b/internal/tui3/engine_build_test.go @@ -0,0 +1,31 @@ +package tui3 + +import ( + "strings" + "testing" + + "github.com/Agent-Field/codeaf/internal/buildinfo" +) + +func TestStatusFollowsEngineReplacementWithoutRestartingTheWindow(t *testing.T) { + a := newTestApp(&fakeAgent{model: "m"}) + held := "older-engine-build" + a.build = held + a.link.EngineBuild = func() string { return held } + a.slash("/status") + text := lastNote(t, a) + for _, want := range []string{"window build", "engine build", held, buildinfo.String()} { + if !strings.Contains(text, want) { + t.Fatalf("missing %q in status %s", want, text) + } + } + held = "replacement-engine-build" + a.slash("/status") + text = lastNote(t, a) + if !strings.Contains(text, held) || strings.Contains(text, "older-engine-build") { + t.Fatalf("status kept the old launch welcome: %s", text) + } + if json := a.statusJSON(); !strings.Contains(json, `"engine build":"replacement-engine-build"`) { + t.Fatal(json) + } +} diff --git a/internal/tui3/hostlink.go b/internal/tui3/hostlink.go index b93bf9c7b9..037f635e55 100644 --- a/internal/tui3/hostlink.go +++ b/internal/tui3/hostlink.go @@ -53,6 +53,10 @@ import ( // session, every test, and every headless frame. Nothing half-works there: no // segment is drawn, no notice is looked for, and no question is asked about. type LinkSeam struct { + // EngineBuild reads the latest welcome already held by the client. It + // changes after redial onto a replacement engine and never touches the wire. + EngineBuild func() string + // Note is the quiet true sentence about the link right now, and the empty // string whenever there is nothing to say — which is almost always. It // reads `reconnecting to devbox — trying for up to 5 minutes` while a @@ -326,7 +330,7 @@ func (a *app) takeLinkNotice() { // from before the status line's news crossed a connection. It is in the // engine-host sentences' own voice (cmd/codeaf's busyEngineHostSentence): the // machine is older, nothing is wrong, and it says when that changes. -const newsSilenceNote = "this conversation's engine is an older codeaf, so the host and tok/s are not shown — they come back once it picks up this build" +const newsSilenceNote = "this window uses an older background engine; host and tok/s are unavailable · /status shows the engine and window builds; reopening one window may keep the same engine" // sayNewsSilence says [newsSilenceNote] once per window, the first time a turn // that produced an answer ends on an engine that has sent no news. diff --git a/internal/tui3/statusnote.go b/internal/tui3/statusnote.go index b46ea3c6e1..d897c5eed6 100644 --- a/internal/tui3/statusnote.go +++ b/internal/tui3/statusnote.go @@ -6,6 +6,7 @@ import ( "github.com/charmbracelet/x/ansi" + "github.com/Agent-Field/codeaf/internal/buildinfo" "github.com/Agent-Field/codeaf/internal/session" ) @@ -122,9 +123,7 @@ func (a *app) statusItems() []deckItem { // row and phone sheet, where an immutable revision would spend a row all // session, and appears here whole when a person asks which codeaf is // holding this conversation. - if a.build != "" { - items = append(items, deckItem{label: "build", value: a.build}) - } + items = append(items, a.statusBuildItems()...) if a.file != "" { // AND THE JOURNAL IS ON WHOSE DISK. A session file is the one path on // this list a person is actively invited to copy, and over a connection it @@ -135,6 +134,23 @@ func (a *app) statusItems() []deckItem { return items } +// statusBuildItems distinguishes the program drawing the window from the +// background process serving it. A reconnect may replace that process without +// rebuilding the window, so the current welcome wins over launch-time facts. +func (a *app) statusBuildItems() []deckItem { + if a.link.EngineBuild != nil { + items := []deckItem{{label: "window build", value: buildinfo.String()}} + if held := strings.TrimSpace(a.link.EngineBuild()); held != "" { + items = append(items, deckItem{label: "engine build", value: held}) + } + return items + } + if a.build != "" { + return []deckItem{{label: "build", value: a.build}} + } + return nil +} + // statusJSON is /status --json: [app.statusItems] serialized as ONE JSON object // with the labels as keys and the values as strings, in the order the text form // prints them. From 3bcb07f6c24d1abe05d11307d8d7691b554db770 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Thu, 22 Oct 2026 20:18:08 -0400 Subject: [PATCH 2/3] Verify real engine recovery and keep restart commands copyable --- cmd/codeaf/engine.go | 2 +- cmd/codeaf/engine_environment_test.go | 19 ++++++++-- internal/e2e/engine_upgrade_e2e_test.go | 37 ++++++++++++++++++- .../e2e/memory_invalid_helpers_e2e_test.go | 14 +++++-- internal/e2e/memory_recovery_e2e_test.go | 2 +- 5 files changed, 63 insertions(+), 11 deletions(-) diff --git a/cmd/codeaf/engine.go b/cmd/codeaf/engine.go index 01047b2240..225456f1ba 100644 --- a/cmd/codeaf/engine.go +++ b/cmd/codeaf/engine.go @@ -397,7 +397,7 @@ func nothingHolds(workspace string) bool { // stops the folder actually holding the old environment, including from Home. func differentEngineEnvironmentSentence(workspace string) string { stop := engineStopWords(workspace) - return "the background engine for this folder started with a different environment than this terminal and is still in use — after work finishes, close every window for this workspace and reopen, or run " + stop + " (interrupts active turns and tasks)" + return "the background engine for this folder started with a different environment than this terminal and is still in use — after work finishes, close every window for this workspace and reopen. To stop now (interrupts active turns and tasks), run " + stop } // sameEngineBuild is the host being this build: the same wire, the same source, diff --git a/cmd/codeaf/engine_environment_test.go b/cmd/codeaf/engine_environment_test.go index 670c12b430..8947402b17 100644 --- a/cmd/codeaf/engine_environment_test.go +++ b/cmd/codeaf/engine_environment_test.go @@ -460,12 +460,23 @@ func TestEnvironmentStopCommandQuotesTheWorkspaceForTheShell(t *testing.T) { for _, workspace := range []string{"/tmp/my project", "/tmp/it's project"} { t.Run(workspace, func(t *testing.T) { note := differentEngineEnvironmentSentence(workspace) - _, command, ok := strings.Cut(note, ", or run ") - if !ok || !strings.Contains(note, "and is still in use — close its other windows or let its work finish") { + _, command, ok := strings.Cut(note, "), run ") + if !ok || !strings.Contains(note, "after work finishes, close every window for this workspace and reopen") || !strings.Contains(note, "To stop now (interrupts active turns and tasks)") { t.Fatalf("busy note does not explain the recovery: %s", note) } - // A shell function records the arguments instead of stopping a host. - out, err := exec.Command("bash", "-c", "codeaf() { printf '%s\\n' \"$@\"; }; "+command).Output() + // Keep the production command's complete shell argument text. Only + // replace its executable with a recorder, so this test cannot stop a + // real engine and also exercises executable paths needing quoting. + recorder := filepath.Join(t.TempDir(), "devaf's test recorder") + if err := os.WriteFile(recorder, []byte("#!/bin/sh\nprintf '%s\\n' \"$@\"\n"), 0o755); err != nil { + t.Fatal(err) + } + binary := shellQuote(enginehost.ThisBinary()) + if !strings.HasPrefix(command, binary+" engine --stop ") { + t.Fatalf("stop command does not use the installed executable: %q", command) + } + command = shellQuote(recorder) + strings.TrimPrefix(command, binary) + out, err := exec.Command("bash", "-c", command).Output() if err != nil || string(out) != "engine\n--stop\n--workspace\n"+workspace+"\n" { t.Fatalf("copied command arguments = %q, error %v", out, err) } diff --git a/internal/e2e/engine_upgrade_e2e_test.go b/internal/e2e/engine_upgrade_e2e_test.go index 86e851fd9d..4baf7ba5fc 100644 --- a/internal/e2e/engine_upgrade_e2e_test.go +++ b/internal/e2e/engine_upgrade_e2e_test.go @@ -27,9 +27,10 @@ func TestOlderEngineUpgradeOnTheRealTerminal(t *testing.T) { if e2eModel != model { t.Fatalf("requires CODEAF_E2E_MODEL=%s", model) } - home := newHome(t, map[string]any{"memory.enabled": false}) + home := recoveryShortHome(t, newHome(t, map[string]any{"memory.enabled": false})) ws := newWorkspace(t, "engine-upgrade", false) env := []string{config.APIKeyEnv + "=" + liveKey(t), "CODEAF_BASE_URL=" + config.DefaultBaseURL} + stopRecoveryEngineOnCleanup(t, env, home, ws) old := startUpgradeOldWindow(t, oldBinary, env, home, ws) old.skipSetup(t) prompt := "Create upgrade-proof.txt containing exactly amber lighthouse. Do not modify any other file, delegate, or use memory tools." @@ -179,3 +180,37 @@ func assertUpgradeFile(t *testing.T, ws string) { t.Fatalf("upgrade lost original mock file: %q %v", raw, err) } } + +// Engine sockets need a short state root on both Linux and macOS. Keep every +// pinned model and credential from the guarded profile, without shortHome's +// historical model defaults. +func recoveryShortHome(t *testing.T, original string) string { + t.Helper() + home := filepath.Join(shortRoot(t), "h") + if err := os.Mkdir(home, 0o700); err != nil { + t.Fatal(err) + } + raw, err := os.ReadFile(filepath.Join(original, "config.json")) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(home, "config.json"), raw, 0o600); err != nil { + t.Fatal(err) + } + return home +} + +// Register before starting windows: LIFO cleanup closes every fixture surface +// first, then stops its engine while the socket and state root still exist. +func stopRecoveryEngineOnCleanup(t *testing.T, env []string, home, ws string) { + t.Helper() + t.Cleanup(func() { + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + cmd := guardedCommand(t, ctx, home, append(append(os.Environ(), env...), "CODEAF_HOME="+home), binary(t), "engine", "--stop", "--workspace", ws) + out, err := cmd.CombinedOutput() + if err != nil { + t.Errorf("fixture engine cleanup: %v %s", err, out) + } + }) +} diff --git a/internal/e2e/memory_invalid_helpers_e2e_test.go b/internal/e2e/memory_invalid_helpers_e2e_test.go index 9533657f5e..caafe334af 100644 --- a/internal/e2e/memory_invalid_helpers_e2e_test.go +++ b/internal/e2e/memory_invalid_helpers_e2e_test.go @@ -109,6 +109,7 @@ func TestMemoryRecoversFromInvalidHelpersOnTheRealTerminal(t *testing.T) { config.KeyModelRoles: strings.Join(pins, ","), config.KeyModelFallbacks: "", }) + home = recoveryShortHome(t, home) ws := newWorkspace(t, "juniper-recovery", false) project, err := gitidentity.ProjectKey(ws) if err != nil { @@ -119,7 +120,9 @@ func TestMemoryRecoversFromInvalidHelpersOnTheRealTerminal(t *testing.T) { t.Fatal(err) } defer st.Close() - r := startWithEnv(t, []string{config.APIKeyEnv + "=" + liveKey(t), "CODEAF_BASE_URL=" + server.URL + "/fixture"}, "memory_invalid_helpers", home, ws, 150, 45, "chat", "--model", model, "--debug") + stateEnv := []string{config.APIKeyEnv + "=" + liveKey(t), "CODEAF_BASE_URL=" + server.URL + "/fixture"} + stopRecoveryEngineOnCleanup(t, stateEnv, home, ws) + r := startWithEnv(t, stateEnv, "memory_invalid_helpers", home, ws, 150, 45, "chat", "--model", model) r.skipSetup(t) prompt := "For the fictional Juniper demo, our standing packaging convention is violet envelopes for weekend parcels. Please acknowledge in one sentence without calling any tools." r.lit(prompt) @@ -163,8 +166,11 @@ func TestMemoryRecoversFromInvalidHelpersOnTheRealTerminal(t *testing.T) { // A different process edits the profile while the original engine remains // attached. A NEW unavailable ID must be attempted, proving this was a // fresh settings read rather than the cached original failed helper. - stateEnv := []string{config.APIKeyEnv + "=" + liveKey(t), "CODEAF_BASE_URL=" + server.URL + "/fixture"} - beforePID := upgradePID(upgradeEngineCommand(t, binary(t), stateEnv, home, ws, "--status")) + beforeStatus := upgradeEngineCommand(t, binary(t), stateEnv, home, ws, "--status") + beforePID := upgradePID(beforeStatus) + if beforePID == "" { + t.Fatalf("fixture did not start a background engine: %s", beforeStatus) + } configPath := filepath.Join(home, "config.json") rawConfig, err := os.ReadFile(configPath) if err != nil { @@ -284,7 +290,7 @@ func TestMemoryRecoversFromInvalidHelpersOnTheRealTerminal(t *testing.T) { r.lit("/new") r.keys("Enter") time.Sleep(time.Second) - apply := "For the fictional Juniper demo, create packaging.json with a weekend_envelope_color field set from our current convention. Use saved memory and do not ask me to repeat it." + apply := "For the fictional Juniper demo, use the write or bash tool to create packaging.json in this project with a weekend_envelope_color field set from our current convention, then read the actual file to verify it exists. Use saved memory and do not ask me to repeat it. Do not merely display JSON in your reply." r.lit(apply) r.keys("Enter") if completed, _ := privateMemoryAwaitAnswer(home, apply, 2*time.Minute); completed == "" { diff --git a/internal/e2e/memory_recovery_e2e_test.go b/internal/e2e/memory_recovery_e2e_test.go index c80f15410d..3121d36e5e 100644 --- a/internal/e2e/memory_recovery_e2e_test.go +++ b/internal/e2e/memory_recovery_e2e_test.go @@ -177,7 +177,7 @@ func TestMemoryRecoversFromTimeoutOnTheRealTerminal(t *testing.T) { r.lit("/new") r.keys("Enter") time.Sleep(time.Second) - apply := "For the fictional Juniper demo, create packaging.json with a weekend_envelope_color field set from our current convention. Use saved memory and do not ask me to repeat it." + apply := "For the fictional Juniper demo, use the write or bash tool to create packaging.json in this project with a weekend_envelope_color field set from our current convention, then read the actual file to verify it exists. Use saved memory and do not ask me to repeat it. Do not merely display JSON in your reply." r.lit(apply) r.keys("Enter") if completed, _ := privateMemoryAwaitAnswer(home, apply, 2*time.Minute); completed == "" { From f7abe6b71d3896253ee6ac7ff74852e19566211d Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Thu, 22 Oct 2026 20:23:52 -0400 Subject: [PATCH 3/3] Document memory recovery and engine upgrade behavior for PR 1903 --- .../1903-memory-helper-and-engine-recovery.md | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 docs/changes/unreleased/1903-memory-helper-and-engine-recovery.md diff --git a/docs/changes/unreleased/1903-memory-helper-and-engine-recovery.md b/docs/changes/unreleased/1903-memory-helper-and-engine-recovery.md new file mode 100644 index 0000000000..07005d2b25 --- /dev/null +++ b/docs/changes/unreleased/1903-memory-helper-and-engine-recovery.md @@ -0,0 +1,26 @@ +--- +kind: fixed +title: recover unavailable memory helpers and explain background engine upgrades +pr: 1903 +surface: [chat, engine] +invalidates: + - >- + An unavailable memory helper and unavailable small-work fallback could end + automatic capture. Recovery now also tries the current chat model within + the same 45-second budget, skips unavailable models for that chat, and + discloses a validated substitution without rewriting model preferences. + - >- + Running engines could keep model settings changed by another process until + restart. Helper resolution now refreshes after profile or project settings + change, so the next call uses the saved configuration. + - >- + Reopening an updated window does not necessarily update its background + engine. Startup guidance explains closing all workspace windows after work + finishes or explicitly stopping that workspace; /status shows both builds + and follows the engine after reconnection. +--- + +Busy engines keep their work. Targeted restart instructions use the installed +executable and quote paths correctly, with an explicit interruption warning. +Authentication, payment, quota and policy refusals do not advance the recovery +chain. Background retries remain bounded to the open chat's lifetime.