From 95661193d1cc6ad3c5fe87f7d25eb85ce490628d Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 6 Nov 2026 09:15:41 -0500 Subject: [PATCH 1/6] Models: a fallback model is for that run only, never the saved choice On a local launch whose configured model is not listed (no key for its provider, say, with only a local Ollama running), the terminal picks the first listed model so the conversation can run. It skipped the profile's talk row, but it switched through the ordinary model setter, so the fallback became the conversation's saved model: the agent reopened on it (also in the app, where the configured provider worked), and errand routes that fall to the conversation's model followed it. The chat manual already said automatic replacements do not overwrite a saved preference. The conversation now has a run-only model door (session UseModelForNow, carried over the linked-local wire, older engines keep the old setter). Requests run on the fallback while the saved model, provider, identity and main/fallback pair keep naming the configured choice; the app reports the configured model. The terminal's availability reconcile, the implicit credit-default follow, the move after disconnecting the running model's service, and the linked-local source refresh use it, and every later reconcile returns to the configured model as soon as it is listed again. An explicit choice, including choosing the fallback that is already running, and a fresh service connect still save as before. A chat continued from another computer keeps its existing replacement rule. Co-Authored-By: Claude Opus 5.5 --- cmd/codeaf/app.go | 3 + cmd/codeaf/app_model_for_now_test.go | 136 +++++++++++++ cmd/codeaf/chatv3_local.go | 12 +- cmd/codeaf/chatv3_model_for_now_test.go | 43 +++++ internal/manual/chat/models-and-cost.md | 6 +- internal/manual/chat/services.md | 3 + internal/remote/model_for_now.go | 51 +++++ internal/remote/model_for_now_test.go | 77 ++++++++ internal/remote/server.go | 3 + internal/session/agent.go | 12 ++ internal/session/conversation_models.go | 4 +- internal/session/loop.go | 6 +- internal/session/model_for_now.go | 80 ++++++++ internal/session/model_for_now_test.go | 179 +++++++++++++++++ internal/session/model_identity.go | 2 +- internal/session/placemeta.go | 5 +- internal/session/session.go | 3 + internal/tui3/credits.go | 4 +- internal/tui3/model_for_now.go | 20 ++ internal/tui3/model_for_now_test.go | 247 ++++++++++++++++++++++++ internal/tui3/modeldefault.go | 12 +- internal/tui3/modelservices.go | 8 +- internal/tui3/onboarding.go | 2 +- internal/tui3/palette.go | 10 +- 24 files changed, 898 insertions(+), 30 deletions(-) create mode 100644 cmd/codeaf/app_model_for_now_test.go create mode 100644 cmd/codeaf/chatv3_model_for_now_test.go create mode 100644 internal/remote/model_for_now.go create mode 100644 internal/remote/model_for_now_test.go create mode 100644 internal/session/model_for_now.go create mode 100644 internal/session/model_for_now_test.go create mode 100644 internal/tui3/model_for_now.go create mode 100644 internal/tui3/model_for_now_test.go diff --git a/cmd/codeaf/app.go b/cmd/codeaf/app.go index 516e8eec01..e070380edf 100644 --- a/cmd/codeaf/app.go +++ b/cmd/codeaf/app.go @@ -1939,6 +1939,9 @@ func (b *appBackend) hold(a *appAgent, conn *appConn, workspace string) appwire. if door, ok := conn.Conv.(interface{ ModelIdentity() session.ModelIdentity }); ok { state := door.ModelIdentity() modelIdentity = &state + if state.Selected != "" { + model = state.Selected + } } effort := conn.Conv.ReasoningFor(model) if dial, ok := conn.Conv.(interface{ ConversationEffort() string }); ok { diff --git a/cmd/codeaf/app_model_for_now_test.go b/cmd/codeaf/app_model_for_now_test.go new file mode 100644 index 0000000000..f097eaad0a --- /dev/null +++ b/cmd/codeaf/app_model_for_now_test.go @@ -0,0 +1,136 @@ +package main + +import ( + "bytes" + "context" + "os" + "path/filepath" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/appwire" + "github.com/Agent-Field/codeaf/internal/config" + "github.com/Agent-Field/codeaf/internal/home" + "github.com/Agent-Field/codeaf/internal/modelsource/sourcestub" + "github.com/Agent-Field/codeaf/internal/session" +) + +// C1, C2, C3: the app reopens the saved choice and title route, leaving settings +// and setup state byte-identical across an automatic run-only selection. +func TestAppReopensConfiguredModelAfterRunOnlyReplacement(t *testing.T) { + t.Setenv(home.EnvVar, t.TempDir()) + profile := t.TempDir() + t.Setenv(config.ProfileDirEnv, profile) + dir, workspace := t.TempDir(), t.TempDir() + path := filepath.Join(dir, "transcript.jsonl") + provider := sourcestub.New("vendor/configured", "available/model", "vendor/backup") + defer provider.Close() + settings := []byte(`{ + "model.talk": "vendor/configured", + "models.tiers.low": "", + "models.tiers.reflex": "vendor/reflex", + "models.tiers.worker": "vendor/worker", + "models.tiers.high": "vendor/checker", + "models.tiers.mastermind": "vendor/planner", + "models.roles": "planner:vendor/planner", + "task.model": "vendor/task", + "models.tasks.mode": "auto", + "models.tasks.one": "vendor/one" +} +`) + if err := os.WriteFile(config.BudgetConfigPath(profile), settings, 0600); err != nil { + t.Fatal(err) + } + rolesSource, err := v3RolesSource(workspace, profile) + if err != nil { + t.Fatal(err) + } + cfg := session.Config{Workspace: workspace, Place: session.Place{Dir: dir, Workspace: workspace}, SessionFile: path, + Model: "vendor/configured", APIKey: "fixture", BaseURL: provider.URL(), ProfileDir: profile, System: "SYSTEM", + RolesSource: rolesSource, + ConversationModels: &session.ConversationModels{Main: "vendor/configured", Fallback: "vendor/backup"}, + } + a, err := session.New(cfg) + if err != nil { + t.Fatal(err) + } + defer a.Close() + var reopened *session.Agent + b := newFakeBackend(newAppRecorder(), func(ask appOpenAsk) (*appConn, error) { + meta, err := session.LoadMeta(dir) + if err != nil { + return nil, err + } + fresh := cfg + fresh.Model = meta.Model + reopened, err = session.New(fresh) + if err != nil { + return nil, err + } + return &appConn{Conv: reopened, Workspace: workspace, Transcript: path, Release: func() { _ = reopened.Close() }}, nil + }) + defer b.Close() + b.state = &appState{path: filepath.Join(t.TempDir(), appStateName)} + if err := b.state.write(func(f *appStateFile) error { + f.Setup = &appwire.Setup{DefaultModel: cfg.Model} + return nil + }); err != nil { + t.Fatal(err) + } + stateBefore, err := os.ReadFile(b.state.where()) + if err != nil { + t.Fatal(err) + } + if err := a.UseModelForNow("available/model"); err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + defer cancel() + events, err := a.Submit(ctx, "respond on the available model") + if err != nil { + t.Fatal(err) + } + for event := range events { + if event.Kind == session.EventError { + t.Fatalf("scripted turn failed: %v", event.Err) + } + } + if err := a.Close(); err != nil { + t.Fatal(err) + } + a.SettleWrites() + profileAfter, err := os.ReadFile(config.BudgetConfigPath(profile)) + if err != nil || !bytes.Equal(settings, profileAfter) { + t.Fatalf("fallback wrote profile: %s, %v", profileAfter, err) + } + stateAfter, err := os.ReadFile(b.state.where()) + if err != nil || !bytes.Equal(stateBefore, stateAfter) { + t.Fatalf("fallback wrote app setup: %s, %v", stateAfter, err) + } + row, ok := session.ReadRows([]string{path})[filepath.Clean(path)] + if !ok { + t.Fatal("saved conversation is missing from app inventory") + } + listed := appRowView(row) + if listed.Model != cfg.Model || listed.ModelIdentity == nil || listed.ModelIdentity.Selected != cfg.Model { + t.Fatalf("agents.list row=%+v", listed) + } + b.remember(row) + opened, err := b.OpenAgent(listed.ID) + if err != nil || opened.Model != cfg.Model { + t.Fatalf("agents.open=%+v, %v", opened, err) + } + routes, _, err := b.errandRoutes(listed.ID) + if err != nil { + t.Fatal(err) + } + for _, route := range routes { + if route.Role == "title" { + if route.Model != cfg.Model { + t.Fatalf("agents.errand.models title=%+v", route) + } + return + } + } + t.Fatal("agents.errand.models omitted title") +} diff --git a/cmd/codeaf/chatv3_local.go b/cmd/codeaf/chatv3_local.go index b5fc049365..887157f3b0 100644 --- a/cmd/codeaf/chatv3_local.go +++ b/cmd/codeaf/chatv3_local.go @@ -452,15 +452,19 @@ func localDoors(options *tui3.Options, welcome remote.Welcome, settings config.C } if options.Agent != nil { // THE ENGINE RESOLVES THE PROFILE. A connected service is already on - // disk when this callback runs; setting the current model through the - // existing wire door makes the engine re-read that disk without sending - // a key or address through a second protocol. + // disk when this callback runs. Reapplying the run-only model makes the + // engine re-read that disk without saving an automatic replacement or + // sending a key or address through the wire. stock := options.ApplyModelSources options.ApplyModelSources = func(sources modelsource.Set) { if stock != nil { stock(sources) } - options.Agent.SetModel(options.Agent.Model()) + if door, ok := options.Agent.(interface{ UseModelForNow(string) error }); ok { + _ = door.UseModelForNow(options.Agent.Model()) + } else { + options.Agent.SetModel(options.Agent.Model()) + } } } } diff --git a/cmd/codeaf/chatv3_model_for_now_test.go b/cmd/codeaf/chatv3_model_for_now_test.go new file mode 100644 index 0000000000..118794c586 --- /dev/null +++ b/cmd/codeaf/chatv3_model_for_now_test.go @@ -0,0 +1,43 @@ +package main + +import ( + "testing" + + "github.com/Agent-Field/codeaf/internal/config" + "github.com/Agent-Field/codeaf/internal/modelsource" + "github.com/Agent-Field/codeaf/internal/remote" + "github.com/Agent-Field/codeaf/internal/session" + "github.com/Agent-Field/codeaf/internal/tui3" +) + +// C1, C2, C4: refreshing linked-local provider sources must not confirm the +// temporary model as a saved choice before catalog reconciliation can restore it. +func TestLinkedLocalSourceRefreshKeepsRunOnlyModelTemporary(t *testing.T) { + dir, profile := t.TempDir(), t.TempDir() + agent, err := session.New(session.Config{Workspace: t.TempDir(), Place: session.Place{Dir: dir}, Model: "vendor/configured", APIKey: "fixture"}) + if err != nil { + t.Fatal(err) + } + defer agent.Close() + loop, err := remote.Loopback(remote.Hello{Version: remote.Version}, remote.Options{Boot: func(remote.Hello) (*remote.Engine, error) { + return &remote.Engine{Agent: agent, RefreshModelSources: func() { agent.SetSources(modelsource.Set{}) }}, nil + }}) + if err != nil { + t.Fatal(err) + } + defer loop.Close() + if err := loop.Client.Agent().UseModelForNow("available/model"); err != nil { + t.Fatal(err) + } + options := tui3.Options{Agent: loop.Client.Agent(), ProfileDir: profile} + localDoors(&options, remote.Welcome{ProfileDir: profile}, config.Config{ProfileDir: profile}) + options.ApplyModelSources(modelsource.Set{}) + agent.SettleWrites() + meta, err := session.LoadMeta(dir) + if err != nil || meta.Model != "vendor/configured" || agent.Model() != "available/model" || loop.Client.Agent().ConfiguredModel() != "vendor/configured" { + t.Fatalf("source refresh persisted the run-only model: %+v, %v", meta, err) + } + if config.ChatModelAt(profile) != "" { + t.Fatal("source refresh wrote a talk preference") + } +} diff --git a/internal/manual/chat/models-and-cost.md b/internal/manual/chat/models-and-cost.md index 8051237d5d..c01d0631eb 100644 --- a/internal/manual/chat/models-and-cost.md +++ b/internal/manual/chat/models-and-cost.md @@ -121,7 +121,11 @@ On a local launch, the opening model comes from the same available chat list as is kept only if that model is listed. Otherwise codeaf selects the first available chat model in provider order. With only Ollama connected, that is an installed Ollama model. With several providers, the default still belongs to their combined list. -Automatic replacements do not overwrite your saved preference. +Availability replacements apply only to this run. They do not overwrite the +conversation's saved model, main/fallback pair or profile model settings. When +the configured model is listed again, codeaf returns to it automatically. +Choosing a model yourself, including the replacement already running, saves that +choice as usual. While every catalog is cold or empty, no default model is displayed and a message stays in the draft. The picker opens with: diff --git a/internal/manual/chat/services.md b/internal/manual/chat/services.md index c4780263d3..3eb86649e0 100644 --- a/internal/manual/chat/services.md +++ b/internal/manual/chat/services.md @@ -220,6 +220,9 @@ A provider answering the current turn cannot be cut: the disconnected sentence first and then says `this conversation was on deepseek-direct/deepseek-v4-pro · it is now on ~deepseek/deepseek-v4-flash-latest`, or, when nothing can replace it, `this conversation was on deepseek-direct/deepseek-v4-pro and nothing else here can take it · connect a provider or pick a model`. +The replacement is for this run only; it leaves the saved model and profile model +settings unchanged. The configured model returns when it is available again. +Choosing the replacement yourself saves it, even if it is already running. OpenRouter uses the same two-Enter confirmation. A saved profile key is cleared from the profile and the running session; its models disappear, while Ollama and other connected diff --git a/internal/remote/model_for_now.go b/internal/remote/model_for_now.go new file mode 100644 index 0000000000..d371af8675 --- /dev/null +++ b/internal/remote/model_for_now.go @@ -0,0 +1,51 @@ +package remote + +import ( + "errors" + "fmt" +) + +const MethodUseModelForNow = "UseModelForNow" + +const modelForNowUnsupported = "run-only model selection is unsupported by this engine" + +// ConfiguredModel reads the saved selection, rather than this run's replacement. +func (a *Agent) ConfiguredModel() string { + if selected := a.ModelIdentity().Selected; selected != "" { + return selected + } + return a.Model() +} + +// UseModelForNow is acknowledged before the surface sends its next request. +// Older engines retain their existing SetModel behavior. +func (a *Agent) UseModelForNow(model string) error { + _, err := a.c.call(nil, MethodUseModelForNow, model) + if err != nil && (err.Error() == fmt.Sprintf("engine: no such method %q", MethodUseModelForNow) || err.Error() == modelForNowUnsupported) { + a.SetModel(model) + return nil + } + return err +} + +func (s *server) modelForNowCall(agent WrappedAgent, call Frame) ([]byte, bool, error) { + if call.Method != MethodUseModelForNow { + return nil, false, nil + } + model, err := arg[string](call) + if err != nil { + return nil, true, err + } + door, ok := agent.(interface{ UseModelForNow(string) error }) + if !ok { + return nil, true, errors.New(modelForNowUnsupported) + } + if refresh := s.session.engine.RefreshModelSources; refresh != nil { + refresh() + } + if err := door.UseModelForNow(model); err != nil { + return nil, true, err + } + s.session.announce() + return nil, true, nil +} diff --git a/internal/remote/model_for_now_test.go b/internal/remote/model_for_now_test.go new file mode 100644 index 0000000000..9953ea4c2c --- /dev/null +++ b/internal/remote/model_for_now_test.go @@ -0,0 +1,77 @@ +package remote + +import ( + "fmt" + "testing" + + "github.com/Agent-Field/codeaf/internal/session" +) + +// C1, C4, C5: the linked-local wire distinguishes temporary and explicit picks. +func TestRunOnlyModelCrossesTheWireWithoutReplacingTheSavedPair(t *testing.T) { + dir := t.TempDir() + a, err := session.New(session.Config{Workspace: t.TempDir(), Place: session.Place{Dir: dir}, Model: "vendor/configured", APIKey: "fixture", + ConversationModels: &session.ConversationModels{Main: "vendor/configured", Fallback: "vendor/backup"}, + }) + if err != nil { + t.Fatal(err) + } + defer a.Close() + loop, err := Loopback(Hello{Version: Version}, Options{Boot: func(Hello) (*Engine, error) { return &Engine{Agent: a}, nil }}) + if err != nil { + t.Fatal(err) + } + defer loop.Close() + agent := loop.Client.Agent() + for _, model := range []string{"available/model", "vendor/configured", "available/model"} { + if err := agent.UseModelForNow(model); err != nil { + t.Fatal(err) + } + if agent.Model() != model || agent.ConfiguredModel() != "vendor/configured" { + t.Fatalf("live=%q configured=%q", agent.Model(), agent.ConfiguredModel()) + } + a.SettleWrites() + meta, err := session.LoadMeta(dir) + if err != nil || meta.Model != "vendor/configured" || meta.ModelIdentity == nil || meta.ModelIdentity.Models == nil || meta.ModelIdentity.Models.Main != "vendor/configured" || meta.ModelIdentity.Models.Fallback != "vendor/backup" { + t.Fatalf("saved pair=%+v identity=%+v error=%v", meta, meta.ModelIdentity, err) + } + } + agent.SetModel("available/model") + a.SettleWrites() + meta, err := session.LoadMeta(dir) + if err != nil || meta.Model != "available/model" || agent.ConfiguredModel() != "available/model" { + t.Fatalf("explicit pick=%+v error=%v", meta, err) + } +} + +// C6: an engine without the additive door retains the old setter behavior. +func TestRunOnlyModelFallsBackToTheOlderEngineSetter(t *testing.T) { + for _, refusal := range []string{fmt.Sprintf("engine: no such method %q", MethodUseModelForNow), modelForNowUnsupported} { + t.Run(refusal, func(t *testing.T) { + client, engine := newEngine(t) + engine.fails[MethodUseModelForNow] = refusal + engine.answers[MethodSetModel] = nil + if err := client.Agent().UseModelForNow("available/model"); err != nil { + t.Fatal(err) + } + if client.Agent().Model() != "available/model" { + t.Fatal("older engine setter was not used") + } + if len(engine.calls(MethodUseModelForNow)) != 1 || len(engine.calls(MethodSetModel)) != 1 || client.CallsMade() != 2 { + t.Fatal("older engine did not receive the temporary request followed by SetModel") + } + }) + } +} + +// C2, C6: other failures must never turn a temporary pick into a durable one. +func TestRunOnlyModelFailureDoesNotCallThePersistentSetter(t *testing.T) { + client, engine := newEngine(t) + engine.fails[MethodUseModelForNow] = "connection unavailable" + if err := client.Agent().UseModelForNow("available/model"); err == nil { + t.Fatal("failed selection reported success") + } + if len(engine.calls(MethodUseModelForNow)) != 1 || len(engine.calls(MethodSetModel)) != 0 { + t.Fatal("a failed temporary request reached SetModel") + } +} diff --git a/internal/remote/server.go b/internal/remote/server.go index 3ee94610bd..07302f61eb 100644 --- a/internal/remote/server.go +++ b/internal/remote/server.go @@ -3616,6 +3616,9 @@ func (s *server) invoke(call Frame) (out json.RawMessage, err error) { // bargain: a surface that meets it says the sentence its place has always // said rather than waiting on a call nobody is going to answer // (wire_places.go states the law). + if payload, handled, err := s.modelForNowCall(agent, call); handled { + return payload, err + } if payload, handled, err := s.placesCall(call); handled { return payload, err } diff --git a/internal/session/agent.go b/internal/session/agent.go index 81d64c8f64..1f45163151 100644 --- a/internal/session/agent.go +++ b/internal/session/agent.go @@ -191,6 +191,9 @@ func newAgent(config Config, client Completer) (*Agent, error) { } if config.Place.Dir != "" { if meta, err := LoadMeta(config.Place.Dir); err == nil { + if meta.Model == config.Model { + agent.modelForNowProvider = meta.ModelProvider + } if meta.ModelIdentity != nil { agent.modelIdentity = *meta.ModelIdentity agent.modelIdentity.Active, agent.modelIdentity.Pending = "", false @@ -797,6 +800,9 @@ func (a *Agent) SetModel(model string) { a.setModel(model) } func (a *Agent) setModel(model string) ModelLanding { if pair, ok := a.ConversationModelPair(); ok { pair.Main = model + if pair.Main == pair.Fallback { + pair.Fallback = "" + } _, _ = a.SetConversationModels(pair) return ModelLandsNextRequest } @@ -821,6 +827,8 @@ func (a *Agent) setModel(model string) ModelLanding { // that said `switching now` and a step that changed nothing (steer.go's // [Agent.ridingNowLocked]). changed := a.ridingNowLocked() != model + a.modelForNowConfigured = "" + a.modelForNowProvider = "" a.model = model if a.clientPool != nil { a.clientPool.setSeat(model) @@ -865,6 +873,10 @@ func (a *Agent) SetSources(sources modelsource.Set) { return } a.mu.Lock() + // A disconnected source must not erase the saved model's provider identity. + if a.modelForNowProvider == "" { + a.modelForNowProvider = a.config.serviceFor(a.configuredModelLocked()).Source.ID + } a.config.Sources = sources a.clientPool.setSources(sources) if !a.running { diff --git a/internal/session/conversation_models.go b/internal/session/conversation_models.go index 4dc9de4761..2dcb7f9f87 100644 --- a/internal/session/conversation_models.go +++ b/internal/session/conversation_models.go @@ -93,12 +93,14 @@ func (a *Agent) setConversationModels(pair ConversationModels, mainNotice string return nil, errors.New("this conversation was launched with one-model; a fallback is unavailable") } hadPair := a.conversationModels != nil - previous := ConversationModels{Main: a.model} + previous := ConversationModels{Main: a.configuredModelLocked()} if a.conversationModels != nil { previous = *a.conversationModels } active := a.modelIdentity.Active != "" a.conversationModels = &pair + a.modelForNowConfigured = "" + a.modelForNowProvider = "" a.model = pair.Main if a.clientPool != nil { a.clientPool.setSeat(pair.Main) diff --git a/internal/session/loop.go b/internal/session/loop.go index 9c41b25384..99db6b1d27 100644 --- a/internal/session/loop.go +++ b/internal/session/loop.go @@ -1926,7 +1926,7 @@ func (a *Agent) completeWithRetry(ctx context.Context, hub *eventHub, model stri func (a *Agent) completeWithRetryReasoning(ctx context.Context, hub *eventHub, model string, rung effort.Rung, partial *partialBuffer, reached *reachedThePerson, reasoning *reasoningBuffer, warm *warmBatch, forming *formingBatch, frozenToolHistory int) (*ai.Response, string, error) { // The current provider call is never cancelled by a selection. Every later // outbound attempt observes the newly acknowledged pair. - if pair, ok := a.ConversationModelPair(); ok { + if pair, ok := a.requestModelPair(); ok { a.mu.Lock() a.fallbackNoticeOwed = true a.mu.Unlock() @@ -2093,7 +2093,7 @@ func (a *Agent) completeWithRetryReasoning(ctx context.Context, hub *eventHub, m // a request boundary: no cancellation and no stale retry chain. { before, paired := requestConversationModels(ctx) - if latest, ok := a.ConversationModelPair(); ok && (!paired || latest != before) { + if latest, ok := a.requestModelPair(); ok && (!paired || latest != before) { ctx = context.WithValue(ctx, conversationModelsKey{}, latest) if latest.Main != before.Main || model == before.Fallback { takeTheModel(latest.Main, true) @@ -2333,7 +2333,7 @@ func (a *Agent) completeWithRetryReasoning(ctx context.Context, hub *eventHub, m // unasked in the same session, and that is the road this is. { before, paired := requestConversationModels(ctx) - if latest, ok := a.ConversationModelPair(); ok && (!paired || latest != before) { + if latest, ok := a.requestModelPair(); ok && (!paired || latest != before) { ctx = context.WithValue(ctx, conversationModelsKey{}, latest) if latest.Main != before.Main { takeTheModel(latest.Main, true) diff --git a/internal/session/model_for_now.go b/internal/session/model_for_now.go new file mode 100644 index 0000000000..aff284f7ba --- /dev/null +++ b/internal/session/model_for_now.go @@ -0,0 +1,80 @@ +package session + +import ( + "errors" + "strings" +) + +// ConfiguredModel is the saved choice, even while this run needs another model. +func (a *Agent) ConfiguredModel() string { + a.mu.Lock() + defer a.mu.Unlock() + return a.configuredModelLocked() +} + +func (a *Agent) configuredModelLocked() string { + if a.modelForNowConfigured != "" { + return a.modelForNowConfigured + } + return a.model +} + +// UseModelForNow changes future requests without replacing the saved choice or +// pair. Choosing the configured model again removes the temporary selection. +func (a *Agent) UseModelForNow(model string) error { + model = strings.TrimSpace(model) + if model == "" { + return errors.New("choose a model") + } + a.modelChanges.Lock() + defer a.modelChanges.Unlock() + a.mu.Lock() + configured := a.configuredModelLocked() + if a.modelForNowProvider == "" { + a.modelForNowProvider = a.config.serviceFor(configured).Source.ID + } + a.modelForNowConfigured = configured + if model == configured { + a.modelForNowConfigured = "" + } + a.model = model + if a.running { + // Let the current call finish; the next request takes this model. + a.spokenModel = model + } + if a.clientPool != nil { + a.clientPool.setSeat(model) + } + if !a.running { + a.rebindClientLocked(model) + } + if a.config.ContextWindowFor != nil { + if window := a.config.ContextWindowFor(model); window > 0 { + a.contextWindow.Store(int64(window)) + } + } + a.noteModelWindow(model) + a.scrubBlindImagePartsLocked(model) + a.followModelOnThePageLocked() + a.mu.Unlock() + a.publishModelIdentity() + return nil +} + +// Request routing uses a copy of the saved pair so a temporary main never +// enters metadata or a later explicit edit of the conversation's fallback. +func (a *Agent) requestModelPair() (ConversationModels, bool) { + a.mu.Lock() + defer a.mu.Unlock() + if a.conversationModels == nil { + return ConversationModels{}, false + } + pair := *a.conversationModels + if a.modelForNowConfigured != "" { + pair.Main = a.model + if pair.Fallback == pair.Main { + pair.Fallback = "" + } + } + return pair, true +} diff --git a/internal/session/model_for_now_test.go b/internal/session/model_for_now_test.go new file mode 100644 index 0000000000..93cd5bc1ff --- /dev/null +++ b/internal/session/model_for_now_test.go @@ -0,0 +1,179 @@ +package session + +import ( + "path/filepath" + "testing" + + "github.com/Agent-Field/codeaf/internal/modelsource" +) + +// C1, C3, C4: a run-only model serves requests without replacing saved choices. +func TestRunOnlyModelSurvivesSavingAsTheConfiguredChoice(t *testing.T) { + for _, paired := range []bool{false, true} { + t.Run(map[bool]string{false: "single", true: "pair"}[paired], func(t *testing.T) { + path := filepath.Join(t.TempDir(), "transcript.jsonl") + c := chained(nil) + a, _ := newTestAgent(t, c, func(cfg *Config) { + cfg.SessionFile, cfg.Place = path, Place{Dir: filepath.Dir(path)} + cloud := modelsource.DefaultSource("http://127.0.0.1:1") + local := modelsource.Source{ID: "local", Written: "available", Address: "http://127.0.0.1:2", KeyOptional: true} + cfg.Sources = modelsource.NewSet(modelsource.Connected{Source: cloud, Key: "fixture", Address: cloud.Address}, modelsource.Connected{Source: local, Address: local.Address}) + if paired { + cfg.ConversationModels = &ConversationModels{Main: cfg.Model, Fallback: "chosen/backup"} + } + }) + a.stampModel() + a.SettleWrites() + before, err := LoadMeta(filepath.Dir(path)) + if err != nil { + t.Fatal(err) + } + if err := a.UseModelForNow("available/model"); err != nil { + t.Fatal(err) + } + if a.config.serviceFor(a.Model()).Source.ID == before.ModelProvider { + t.Fatal("fixture must replace the configured provider as well as its model") + } + runModelForNowTurn(t, a) + if c.model(0) != "model" || a.Model() != "available/model" || a.ConfiguredModel() != "test/model" { + t.Fatalf("request=%q configured=%q", c.model(0), a.ConfiguredModel()) + } + if err := a.Close(); err != nil { + t.Fatal(err) + } + a.SettleWrites() + meta, err := LoadMeta(filepath.Dir(path)) + if err != nil { + t.Fatal(err) + } + if meta.Model != before.Model || meta.ModelProvider != before.ModelProvider || meta.ModelIdentity == nil || meta.ModelIdentity.Selected != before.Model { + t.Fatalf("saved choice changed: before=%+v after=%+v identity=%+v", before, meta, meta.ModelIdentity) + } + if paired && (meta.ModelIdentity.Models == nil || *meta.ModelIdentity.Models != *before.ModelIdentity.Models) { + t.Fatalf("saved pair changed: %+v", meta.ModelIdentity.Models) + } + fresh := chained(nil) + b, _ := newTestAgent(t, fresh, func(cfg *Config) { + cfg.Model, cfg.Place = meta.Model, Place{Dir: filepath.Dir(path)} + }) + if b.Model() != "test/model" || b.ModelIdentity().Selected != "test/model" { + t.Fatalf("reopened on %q: %+v", b.Model(), b.ModelIdentity()) + } + routes, _ := b.ErrandRoutes() + found := false + for _, route := range routes { + if route.Role == "title" { + found = true + if route.Model != "test/model" { + t.Fatalf("reopened title route: %+v", route) + } + } + } + if !found { + t.Fatal("title route missing") + } + runModelForNowTurn(t, b) + if fresh.model(0) != "test/model" { + t.Fatalf("fresh request=%q", fresh.model(0)) + } + }) + } +} + +// C5: an explicit pair edit clears the temporary main, and promoting its +// current fallback through SetModel saves that model as the new main. +func TestExplicitPairSelectionClearsTheRunOnlyMain(t *testing.T) { + a, _ := newTestAgent(t, chained(nil), func(cfg *Config) { + cfg.Place = Place{Dir: t.TempDir()} + cfg.ConversationModels = &ConversationModels{Main: cfg.Model, Fallback: "available/model"} + }) + if err := a.UseModelForNow("available/model"); err != nil { + t.Fatal(err) + } + pair := ConversationModels{Main: "test/model", Fallback: "available/model"} + if _, err := a.SetConversationModels(pair); err != nil { + t.Fatal(err) + } + if a.Model() != "test/model" || a.ConfiguredModel() != "test/model" { + t.Fatal("explicit pair edit left a temporary main") + } + if err := a.UseModelForNow("available/model"); err != nil { + t.Fatal(err) + } + a.SetModel("available/model") + a.SettleWrites() + meta, err := LoadMeta(a.config.Place.Dir) + if err != nil || meta.Model != "available/model" || meta.ModelIdentity.Models.Main != "available/model" || meta.ModelIdentity.Models.Fallback != "" { + t.Fatalf("explicit fallback promotion=%+v %v", meta, err) + } +} + +// C4, C5: restoration is transient; confirming the running fallback is durable. +func TestExplicitModelChoiceClearsRunOnlyModel(t *testing.T) { + for _, paired := range []bool{false, true} { + t.Run(map[bool]string{false: "single", true: "pair"}[paired], func(t *testing.T) { + c := chained(nil) + a, _ := newTestAgent(t, c, func(cfg *Config) { + cfg.Place = Place{Dir: t.TempDir()} + if paired { + cfg.ConversationModels = &ConversationModels{Main: cfg.Model, Fallback: "chosen/backup"} + } + }) + for _, model := range []string{"available/model", "test/model", "available/model"} { + if err := a.UseModelForNow(model); err != nil { + t.Fatal(err) + } + runModelForNowTurn(t, a) + } + if c.model(1) != "test/model" { + t.Fatalf("restored request=%q", c.model(1)) + } + a.SetModel("available/model") + a.SettleWrites() + meta, err := LoadMeta(a.config.Place.Dir) + if err != nil || meta.Model != "available/model" || a.ConfiguredModel() != "available/model" { + t.Fatalf("explicit choice not saved: %+v %v", meta, err) + } + if paired && (meta.ModelIdentity.Models.Main != "available/model" || meta.ModelIdentity.Models.Fallback != "chosen/backup") { + t.Fatalf("explicit pair=%+v", meta.ModelIdentity.Models) + } + }) + } +} + +func runModelForNowTurn(t *testing.T, a *Agent) { + t.Helper() + events, err := a.Submit(t.Context(), "respond") + if err != nil { + t.Fatal(err) + } + if got := collect(t, events); len(textsOfKind(got, EventError)) != 0 { + t.Fatalf("turn failed: %v", textsOfKind(got, EventError)) + } +} + +// C1: removing a provider must preserve its identity beside the saved model. +func TestRunOnlyModelKeepsConfiguredProviderWhenDisconnected(t *testing.T) { + cloud := modelsource.DefaultSource("http://127.0.0.1:1") + local := modelsource.Source{ID: "local", Written: "available", Address: "http://127.0.0.1:2", KeyOptional: true} + remaining := modelsource.NewSet(modelsource.Connected{Source: cloud, Key: "fixture", Address: cloud.Address}) + a, _ := newTestAgent(t, chained(nil), func(cfg *Config) { + cfg.Model, cfg.Place = "available/configured", Place{Dir: t.TempDir()} + cfg.Sources = modelsource.NewSet(modelsource.Connected{Source: cloud, Key: "fixture", Address: cloud.Address}, modelsource.Connected{Source: local, Address: local.Address}) + }) + a.stampModel() + a.SettleWrites() + a.SetSources(remaining) + // Linked-local source refresh reapplies the live model before reconciliation. + if err := a.UseModelForNow(a.Model()); err != nil { + t.Fatal(err) + } + if err := a.UseModelForNow("vendor/replacement"); err != nil { + t.Fatal(err) + } + a.SettleWrites() + meta, err := LoadMeta(a.config.Place.Dir) + if err != nil || meta.Model != "available/configured" || meta.ModelProvider != "local" { + t.Fatalf("disconnected model identity=%+v, %v", meta, err) + } +} diff --git a/internal/session/model_identity.go b/internal/session/model_identity.go index 7bf1abcde8..65fb609cab 100644 --- a/internal/session/model_identity.go +++ b/internal/session/model_identity.go @@ -23,7 +23,7 @@ type ModelIdentity struct { func (a *Agent) modelIdentityLocked() ModelIdentity { s := a.modelIdentity - s.Selected = a.model + s.Selected = a.configuredModelLocked() if a.conversationModels != nil { pair := *a.conversationModels s.Models = &pair diff --git a/internal/session/placemeta.go b/internal/session/placemeta.go index 8bb1522395..d0827080e3 100644 --- a/internal/session/placemeta.go +++ b/internal/session/placemeta.go @@ -336,9 +336,12 @@ func (a *Agent) fillMetaLocked(meta Meta) Meta { if meta.Created.IsZero() { meta.Created = time.Now() } - if model := strings.TrimSpace(a.model); model != "" { + if model := strings.TrimSpace(a.configuredModelLocked()); model != "" { meta.Model = model meta.ModelProvider = a.config.serviceFor(model).Source.ID + if a.modelForNowProvider != "" { + meta.ModelProvider = a.modelForNowProvider + } } // The rung is written whatever it is, absence included, because absence is // a value a person can choose their way back to: a session dialled to max diff --git a/internal/session/session.go b/internal/session/session.go index 0f4f7aba69..9ac864ffc3 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -3161,6 +3161,9 @@ type Agent struct { // the one call that must always be answerable. mu sync.Mutex model string + // modelForNowConfigured retains the saved choice while model serves this run. + modelForNowConfigured string + modelForNowProvider string // reasoning is how hard each model is asked to think, by model id, and it // is a MAP rather than a field for the reason agent.go's block states: the // level is a choice about a model, and a /model switch must not carry one diff --git a/internal/tui3/credits.go b/internal/tui3/credits.go index 0e74eba3b2..fdd2b9b468 100644 --- a/internal/tui3/credits.go +++ b/internal/tui3/credits.go @@ -167,9 +167,7 @@ func (a *app) refreshCreditWarnings() { (a.model == config.DefaultModel || a.model == config.FreeChatModel) && a.freshAndEmpty() && config.ChatModelAt(a.profileDir) == "" && (!a.requireListedModel || a.isAvailableModel(want)) { - a.creditSwitching = true - a.switchModel(want, 0) - a.creditSwitching = false + a.useModelForNow(want, 0) } a.chatCreditWarning, a.homeCreditWarning = "", "" if a.readCredits == nil { diff --git a/internal/tui3/model_for_now.go b/internal/tui3/model_for_now.go new file mode 100644 index 0000000000..ae538d71c7 --- /dev/null +++ b/internal/tui3/model_for_now.go @@ -0,0 +1,20 @@ +package tui3 + +// configuredModel keeps reconciliation anchored to the person's saved choice. +// Agents predating the run-only door keep the existing live-model behavior. +func (a *app) configuredModel() string { + if door, ok := a.agent.(interface{ ConfiguredModel() string }); ok { + return door.ConfiguredModel() + } + if a.model != "" { + return a.model + } + return a.agent.Model() +} + +func (a *app) useModelForNow(id string, window int) { + wasSwitching := a.creditSwitching + a.creditSwitching = true + a.switchModel(id, window) + a.creditSwitching = wasSwitching +} diff --git a/internal/tui3/model_for_now_test.go b/internal/tui3/model_for_now_test.go new file mode 100644 index 0000000000..e70155480b --- /dev/null +++ b/internal/tui3/model_for_now_test.go @@ -0,0 +1,247 @@ +package tui3 + +import ( + "bytes" + "context" + "os" + "testing" + + "github.com/Agent-Field/codeaf/internal/config" + "github.com/Agent-Field/codeaf/internal/credits" + "github.com/Agent-Field/codeaf/internal/modelsource" + "github.com/Agent-Field/codeaf/internal/modelsource/sourcestub" + "github.com/Agent-Field/codeaf/internal/session" +) + +type modelForNowAgent struct { + *fakeAgent + configured string + temporary []string +} + +func (a *modelForNowAgent) ConfiguredModel() string { return a.configured } +func (a *modelForNowAgent) UseModelForNow(model string) error { + a.temporary = append(a.temporary, model) + a.fakeAgent.SetModel(model) + return nil +} +func (a *modelForNowAgent) SetModel(model string) { + a.configured = model + a.fakeAgent.SetModel(model) +} + +// C2, C4, C5: availability reconciles to the saved model without profile writes. +func TestListedModelReplacementKeepsProfileAndReturnsToConfiguredModel(t *testing.T) { + t.Setenv("CODEAF_HOME", t.TempDir()) + dir := t.TempDir() + profile := []byte(`{ + "model.talk": "vendor/configured", + "models.tiers.reflex": "vendor/reflex", + "models.tiers.low": "", + "models.tiers.worker": "vendor/worker", + "models.tiers.high": "vendor/high", + "models.tiers.mastermind": "vendor/planner", + "models.roles": "title:vendor/title", + "task.model": "vendor/task" +} +`) + if err := os.WriteFile(config.BudgetConfigPath(dir), profile, 0600); err != nil { + t.Fatal(err) + } + rows := []Model{{ID: "vendor/replacement", ContextLength: 32768}} + agent := &modelForNowAgent{fakeAgent: &fakeAgent{model: "vendor/configured"}, configured: "vendor/configured"} + a := newApp(t.Context(), Options{Agent: agent, ProfileDir: dir, Sources: modelsource.NewSet(testDefaultService("key")), RequireListedModel: true, + Models: func() []Model { return rows }, SaveModel: func(id string) error { return config.WriteChatModel(dir, id) }, + }) + if a.model != "vendor/replacement" || agent.Model() != a.model || agent.ConfiguredModel() != "vendor/configured" || agent.window != 32768 { + t.Fatalf("replacement=%q/%q configured=%q window=%d", a.model, agent.Model(), agent.ConfiguredModel(), agent.window) + } + rows = append(rows, Model{ID: "vendor/configured", ContextLength: 65536}) + a.state = stateWorking + a.serviceModelsLanded(modelsource.DefaultID, config.DefaultBaseURL) + if a.model != "vendor/replacement" { + t.Fatal("catalog recovery changed an answering request") + } + a.settle() + if a.model != "vendor/configured" || agent.Model() != a.model || agent.window != 65536 { + t.Fatalf("recovered=%q/%q window=%d", a.model, agent.Model(), agent.window) + } + got, err := os.ReadFile(config.BudgetConfigPath(dir)) + if err != nil || !bytes.Equal(got, profile) { + t.Fatalf("automatic selection changed profile: %s, %v", got, err) + } + rows = rows[:1] + a.serviceModelsLanded(modelsource.DefaultID, config.DefaultBaseURL) + a.slash("/model vendor/replacement") + if agent.ConfiguredModel() != "vendor/replacement" || config.ChatModelAt(dir) != "vendor/replacement" || a.modelWord() != "vendor/replacement" { + t.Fatal("confirming the fallback did not save the explicit choice and HUD") + } + if note := noteSaying(t, a, "model ·"); note != "model · vendor/replacement" { + t.Fatalf("explicit model note=%q", note) + } +} + +// C1, C4: catalog recovery changes the next real, scripted provider request. +func TestCatalogRecoveryUsesConfiguredModelOnTheNextRequest(t *testing.T) { + t.Setenv("CODEAF_HOME", t.TempDir()) + server := sourcestub.New("configured", "replacement") + defer server.Close() + service := testDirectService(server.URL()) + sources := modelsource.NewSet(testDefaultService(""), service) + agent, err := session.New(session.Config{Workspace: t.TempDir(), Model: "deepseek-direct/configured", Sources: sources}) + if err != nil { + t.Fatal(err) + } + defer agent.Close() + rows := []Model{{ID: "replacement"}} + a := newApp(t.Context(), Options{Agent: agent, ProfileDir: t.TempDir(), Sources: sources, RequireListedModel: true, + ModelsForService: func(modelsource.Connected) []Model { return rows }, + }) + drainModelServiceTurn(t, agent, "use the available model") + rows = append(rows, Model{ID: "configured"}) + a.serviceModelsLanded(service.Source.ID, service.Address) + drainModelServiceTurn(t, agent, "use the recovered model") + requests := completionRequests(server) + if len(requests) != 2 || completionModel(t, requests[0]) != "replacement" || completionModel(t, requests[1]) != "configured" { + t.Fatalf("provider requests=%+v", requests) + } +} + +// C2, C5: setup only saves an actual pick, including the current run-only model. +func TestSetupCanExplicitlyChooseTheRunOnlyModel(t *testing.T) { + dir := t.TempDir() + agent := &modelForNowAgent{fakeAgent: &fakeAgent{model: "vendor/configured"}, configured: "vendor/configured"} + a := newApp(t.Context(), Options{Agent: agent, ProfileDir: dir, SaveModel: func(id string) error { return config.WriteChatModel(dir, id) }}) + a.useModelForNow("vendor/replacement", 0) + if config.ChatModelAt(dir) != "" { + t.Fatal("an automatic setup model was saved") + } + a.takeSetupModel("vendor/replacement") + if agent.ConfiguredModel() != "vendor/replacement" || config.ChatModelAt(dir) != "vendor/replacement" { + t.Fatal("setup ignored an explicit pick of the running fallback") + } +} + +// C2: automatic credit defaults change this run, without changing its choice. +func TestCreditDefaultUsesTheRunOnlyDoor(t *testing.T) { + t.Setenv("CODEAF_HOME", t.TempDir()) + agent := &modelForNowAgent{fakeAgent: &fakeAgent{model: config.DefaultModel}, configured: config.DefaultModel} + a := newApp(t.Context(), Options{Agent: agent, ProfileDir: t.TempDir(), ImplicitTalk: true}) + if err := config.SetCrewFreeRoutes(a.profileDir, true); err != nil { + t.Fatal(err) + } + seedLowCredits(t, a) + a.readCredits = func(context.Context) (credits.Reading, error) { return credits.Reading{Known: true, Low: true}, nil } + a.refreshCreditWarnings() + if agent.Model() != config.FreeChatModel || agent.ConfiguredModel() != config.DefaultModel || config.ChatModelAt(a.profileDir) != "" || len(agent.temporary) == 0 { + t.Fatalf("credit default=%q configured=%q", agent.Model(), agent.ConfiguredModel()) + } +} + +// C6: remote doors leave the engine's model alone even when it is not listed. +func TestRemoteCatalogDoesNotSelectARunOnlyModel(t *testing.T) { + agent := &modelForNowAgent{fakeAgent: &fakeAgent{model: "engine/model"}, configured: "engine/model"} + a := newApp(t.Context(), Options{Agent: agent, ProfileDir: t.TempDir(), Models: func() []Model { return []Model{{ID: "other/model"}} }}) + a.ensureAvailableModel() + if agent.Model() != "engine/model" || len(agent.temporary) != 0 { + t.Fatal("a remote catalog changed the engine selection") + } +} + +// C5, C6; fix round 1 items 1 and 4: connecting or reconnecting under the same +// name is a persistent choice, idle or deferred, even on the running fallback. +func TestServiceConnectionPersistsIdleAndDeferredMoves(t *testing.T) { + for _, testCase := range []struct { + name string + working bool + running bool + }{ + {name: "idle"}, + {name: "deferred", working: true}, + {name: "idle on the fallback", running: true}, + {name: "deferred on the fallback", working: true, running: true}, + } { + t.Run(testCase.name, func(t *testing.T) { + for _, connection := range []string{"connect", "reconnect"} { + t.Run(connection, func(t *testing.T) { + a, dir := newZAIConnectTestApp(t, "") + configured := a.model + want := "z-ai-direct/glm-5.3" + if connection == "reconnect" { + connectZAIFromPanel(t, a, "plan-test-key") + a.switchModel(configured, 0) + } + agent := &modelForNowAgent{fakeAgent: &fakeAgent{model: configured}, configured: configured} + a.agent = agent + a.requireListedModel = true + if err := config.WriteChatModel(dir, configured); err != nil { + t.Fatal(err) + } + if testCase.running { + a.useModelForNow(want, 0) + } + temporary := len(agent.temporary) + opening := a.model + if testCase.working { + a.state = stateWorking + } + if connection == "reconnect" { + a.adoptModelConnectResult(modelConnectResultMsg{ + service: "z-ai", written: "z-ai-direct", renamedFrom: "z-ai-direct", + outcome: modelsource.Outcome{Kind: modelsource.OutcomeConnected}, + models: a.sourceModels["z-ai"], + }) + } else { + connectZAIFromPanel(t, a, "plan-test-key") + } + if testCase.working { + if a.model != opening || agent.ConfiguredModel() != configured || config.ChatModelAt(dir) != configured || a.deferredModelServiceModel != want { + t.Fatal("connection changed the answering turn or lost its deferred choice") + } + a.settle() + } + if a.model != want || agent.Model() != want || agent.ConfiguredModel() != want || config.ChatModelAt(dir) != want || a.deferredModelServiceModel != "" || len(agent.temporary) != temporary { + t.Fatal("connection did not persist its preferred model") + } + }) + } + }) + } +} + +// C1, C2, C5; fix round 1 items 3 and 4: disconnecting keeps the configured +// choice until the person explicitly confirms the running fallback, idle or deferred. +func TestDisconnectMoveStaysTemporaryUntilExplicitlyChosen(t *testing.T) { + for _, working := range []bool{false, true} { + name := "idle" + if working { + name = "deferred" + } + t.Run(name, func(t *testing.T) { + a, dir := newZAIConnectTestApp(t, "sk-default-1234567890") + fallback := a.model + connectZAIFromPanel(t, a, "plan-test-key") + configured := a.model + agent := &modelForNowAgent{fakeAgent: &fakeAgent{model: configured}, configured: configured} + a.agent = agent + a.requireListedModel = true + a.disconnectModelService("z-ai") + if a.model != fallback || agent.Model() != fallback || agent.ConfiguredModel() != configured || config.ChatModelAt(dir) != configured || len(agent.temporary) == 0 { + t.Fatal("disconnect did not keep its replacement temporary") + } + if working { + a.state = stateWorking + } + a.moveConversationOrDefer(fallback, "openrouter") + if working { + if agent.ConfiguredModel() != configured || config.ChatModelAt(dir) != configured || a.deferredModelServiceModel != fallback { + t.Fatal("explicit fallback choice did not wait for the turn boundary") + } + a.settle() + } + if a.model != fallback || agent.ConfiguredModel() != fallback || config.ChatModelAt(dir) != fallback || a.deferredModelServiceModel != "" { + t.Fatal("explicit confirmation of the running fallback did not persist") + } + }) + } +} diff --git a/internal/tui3/modeldefault.go b/internal/tui3/modeldefault.go index 7905a60ce0..3e3ccd2088 100644 --- a/internal/tui3/modeldefault.go +++ b/internal/tui3/modeldefault.go @@ -49,10 +49,7 @@ func (a *app) ensureAvailableModel() bool { if a.state == stateWorking { return a.model != "" } - preferred := a.model - if preferred == "" { - preferred = a.agent.Model() - } + preferred := a.configuredModel() model, ok := availableConversationModel(preferred, a.modelList()) if !ok { a.model, a.ctxWindow = "", 0 @@ -60,11 +57,8 @@ func (a *app) ensureAvailableModel() bool { return false } if model.ID != a.model { - wasSwitching := a.creditSwitching - a.creditSwitching = true - a.switchModel(model.ID, model.ContextLength) - a.creditSwitching = wasSwitching + a.useModelForNow(model.ID, model.ContextLength) a.hudStale = true } - return true + return a.model == model.ID } diff --git a/internal/tui3/modelservices.go b/internal/tui3/modelservices.go index 182d5274a4..efcbb948de 100644 --- a/internal/tui3/modelservices.go +++ b/internal/tui3/modelservices.go @@ -950,7 +950,7 @@ func (a *app) adoptModelConnectResult(msg modelConnectResultMsg) { a.deferredModelServiceModel = "" } preferred := connected.Source.PreferredModel(connected.Door, listedModelIDs(a.sourceModels[msg.service])) - if next := connected.Qualify(preferred); next != "" && next != strings.TrimSpace(a.model) { + if next := connected.Qualify(preferred); next != "" && (next != strings.TrimSpace(a.model) || next != a.configuredModel()) { if a.state == stateWorking { a.deferredModelServiceModel = next } else { @@ -1112,7 +1112,7 @@ func listedModelIDs(models []Model) []string { // automatic moves cannot drift into two accounts of what happened. func (a *app) moveConversationToConnectedModel(next string) { next = strings.TrimSpace(next) - if next == "" || next == strings.TrimSpace(a.model) { + if next == "" || next == strings.TrimSpace(a.model) && next == a.configuredModel() { return } was := a.model @@ -1172,7 +1172,7 @@ func (a *app) moveConversationOrDefer(id, written string) { // Routing folds case, so an id differing from the running model only in // case is the same pick — the deferred guard above already compares with // EqualFold, and the idle road does too. - if !strings.EqualFold(id, strings.TrimSpace(a.model)) { + if !strings.EqualFold(id, strings.TrimSpace(a.model)) || !strings.EqualFold(id, a.configuredModel()) { a.moveConversationToConnectedModel(id) } } @@ -1303,7 +1303,7 @@ func (a *app) disconnectModelService(id string) { if !conversationUsed { // The disconnect acknowledgement is the whole answer. } else if next, ok := a.reachableModelAfterDisconnect(); ok { - a.switchModel(next, 0) + a.useModelForNow(next, 0) a.modelServiceFollowup(serviceMovedWord(was, next)) } else { a.modelServiceFollowup(serviceStrandedWord(was)) diff --git a/internal/tui3/onboarding.go b/internal/tui3/onboarding.go index 2d6ce93c74..c7e440a3a0 100644 --- a/internal/tui3/onboarding.go +++ b/internal/tui3/onboarding.go @@ -591,7 +591,7 @@ func (a *app) commitSetupControls() bool { func (a *app) takeSetupModel(id string) { s := &a.setup id = strings.TrimSpace(id) - if id == "" || id == a.model { + if id == "" || id == a.model && id == a.configuredModel() { return } row, ok := a.registry().Row(config.ModelSettingKey(talkSlot)) diff --git a/internal/tui3/palette.go b/internal/tui3/palette.go index 8d80f9468a..f21ca987a6 100644 --- a/internal/tui3/palette.go +++ b/internal/tui3/palette.go @@ -3106,7 +3106,13 @@ func (a *app) windowFor(id string) int { // fires at a fraction of it, so a session that switched to a 1M model without // saying so would keep compacting as if it were on the 128k one it started on. func (a *app) switchModel(id string, window int) { - a.agent.SetModel(id) + if door, ok := a.agent.(interface{ UseModelForNow(string) error }); a.creditSwitching && ok { + if err := door.UseModelForNow(id); err != nil { + return + } + } else { + a.agent.SetModel(id) + } a.model = a.agent.Model() if a.model == "" { a.model = id @@ -3207,7 +3213,7 @@ func (a *app) pickerKey(msg tea.KeyPressMsg) tea.Cmd { // one of them asked for that model on that machine, and pinning a // lane under a model they are not talking to would be a setting // that took effect the next time they happened to switch. - if chosen.ID != a.model { + if chosen.ID != a.model || chosen.ID != a.configuredModel() { a.switchModel(chosen.ID, chosen.ContextLength) } a.applyLaneChoice(chosen.ID, row, a.pick.lanes) From 914e6fb9f631c8107de533ef83cbede444cf63f1 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 6 Nov 2026 09:16:16 -0500 Subject: [PATCH 2/6] docs: change entry for run-only fallback models (#2465) Co-Authored-By: Claude Opus 5.5 --- .../unreleased/2465-fallback-model-run-only.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 docs/changes/unreleased/2465-fallback-model-run-only.md diff --git a/docs/changes/unreleased/2465-fallback-model-run-only.md b/docs/changes/unreleased/2465-fallback-model-run-only.md new file mode 100644 index 0000000000..b6b0ef3c4c --- /dev/null +++ b/docs/changes/unreleased/2465-fallback-model-run-only.md @@ -0,0 +1,13 @@ +--- +kind: fixed +title: a fallback model is used for that run only and never becomes the conversation's saved model +pr: 2465 +surface: [chat, engine, remote, app, docs] +invalidates: + - "Was true: when a local launch replaced an unlisted configured model with the first listed model, the replacement became the conversation's saved model and the agent reopened on it. Is true now: the replacement serves requests only, the saved model stays the configured one, and the conversation returns to it as soon as it is listed." +--- + +The same holds after disconnecting the service of the running model and for the implicit +credit-default follow. Choosing a model yourself, including the replacement that is already +running, and connecting a service still save the choice. The desktop app reports the +configured model. A chat continued from another computer keeps its existing replacement rule. From 5a49b2437da0fe9ee1e78a0852a7376d07b959ee Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 6 Nov 2026 09:44:15 -0500 Subject: [PATCH 3/6] Models: the run-only switch is asked off the update loop; renames, queued words and new chats keep the saved choice Review round: - The terminal asks the engine for the run-only model through its ordered off-loop command, as every engine door must; sends wait for the acknowledged switch, and an explicit pick queued behind it supersedes it. - Renaming a service relabels the saved choice and a run-only model separately, so a rename never saves the fallback. - An explicit choice clears a run-only model word still queued for the next request. - start_chat with no model gives the new chat its parent's configured model; the new chat applies its own availability reconcile. - The implicit credit-default follow is restored as it was: it is the build's default moving, not an availability fallback. Existing tests gain only the acknowledgment step the asynchronous switch needs; no assertion changed. Co-Authored-By: Claude Opus 5.5 --- .../2465-fallback-model-run-only.md | 4 +- .../manual/chat/conversations-and-teams.md | 5 +- internal/manual/chat/models-and-cost.md | 4 +- internal/manual/chat/services.md | 3 +- internal/session/agent.go | 3 + internal/session/conversation_models.go | 3 + internal/session/model_for_now_test.go | 67 +++++++ internal/session/start_chat_tool_test.go | 41 +++++ internal/session/tools_start_chat.go | 4 +- internal/tui3/app.go | 10 +- internal/tui3/credits.go | 4 +- internal/tui3/input.go | 3 + internal/tui3/model_change_test.go | 169 ++++++++++++++++++ internal/tui3/model_for_now.go | 100 ++++++++++- internal/tui3/model_for_now_test.go | 65 +++++-- internal/tui3/model_rename.go | 71 ++++++++ internal/tui3/model_rename_test.go | 95 ++++++++++ internal/tui3/modeldefault.go | 9 +- internal/tui3/modeldefault_test.go | 5 + internal/tui3/modelservices.go | 7 + internal/tui3/modelservices_test.go | 2 + internal/tui3/palette.go | 18 +- internal/tui3/provideraccess_test.go | 1 + internal/tui3/startchat.go | 25 ++- internal/tui3/startchat_test.go | 96 ++++++++++ 25 files changed, 777 insertions(+), 37 deletions(-) create mode 100644 internal/tui3/model_change_test.go create mode 100644 internal/tui3/model_rename.go create mode 100644 internal/tui3/model_rename_test.go diff --git a/docs/changes/unreleased/2465-fallback-model-run-only.md b/docs/changes/unreleased/2465-fallback-model-run-only.md index b6b0ef3c4c..6c05b920bd 100644 --- a/docs/changes/unreleased/2465-fallback-model-run-only.md +++ b/docs/changes/unreleased/2465-fallback-model-run-only.md @@ -7,7 +7,7 @@ invalidates: - "Was true: when a local launch replaced an unlisted configured model with the first listed model, the replacement became the conversation's saved model and the agent reopened on it. Is true now: the replacement serves requests only, the saved model stays the configured one, and the conversation returns to it as soon as it is listed." --- -The same holds after disconnecting the service of the running model and for the implicit -credit-default follow. Choosing a model yourself, including the replacement that is already +The same holds after disconnecting the service of the running model. An untouched chat +still saves the build's default when it follows a credit reading. Choosing a model yourself, including the replacement that is already running, and connecting a service still save the choice. The desktop app reports the configured model. A chat continued from another computer keeps its existing replacement rule. diff --git a/internal/manual/chat/conversations-and-teams.md b/internal/manual/chat/conversations-and-teams.md index 57370deb1f..129c11cb7f 100644 --- a/internal/manual/chat/conversations-and-teams.md +++ b/internal/manual/chat/conversations-and-teams.md @@ -436,8 +436,9 @@ Use the Memories tab to forget a saved memory separately. Deleting a team also p Ask “start another chat on kimi-k3 to read the README.” Load the `chats` capability group if needed. The `start_chat` tool takes `brief`, the complete first message, and `model`, a catalog ID or unique model name. -Omitting `model` uses the requesting chat's current model. An unknown or ambiguous -name is refused before any chat opens. The new chat opens behind the current one, +Omitting `model` saves the requesting chat's configured model in the new chat. +The new chat applies its own temporary availability replacement before sending. +An unknown or ambiguous name is refused before any chat opens. The new chat opens behind the current one, with its model and approval posture set before its first message. Your focus stays in the original chat. Its receipt confirms the request; the terminal reports whether it actually opened. This needs an attached interactive terminal that can open chats; app-only sessions do not offer this tool. diff --git a/internal/manual/chat/models-and-cost.md b/internal/manual/chat/models-and-cost.md index c01d0631eb..8220979937 100644 --- a/internal/manual/chat/models-and-cost.md +++ b/internal/manual/chat/models-and-cost.md @@ -131,7 +131,9 @@ While every catalog is cold or empty, no default model is displayed and a messag stays in the draft. The picker opens with: `no available model · connect a provider or refresh /model`. When a provider's list arrives, codeaf selects an available model. Press Enter again -to send the draft. Refreshing a list or removing a connection also replaces a model +to send the draft after the model selection succeeds. A failed selection +reports `could not change model · ` and leaves the draft unsent. +Refreshing a list or removing a connection also replaces a model that is no longer listed; existing requests finish on the model they started with. Remote sessions keep the engine's opening model; this laptop's catalog does not diff --git a/internal/manual/chat/services.md b/internal/manual/chat/services.md index 3eb86649e0..18a0892159 100644 --- a/internal/manual/chat/services.md +++ b/internal/manual/chat/services.md @@ -309,7 +309,8 @@ empty key box keeps the saved key. A changed name is a rename: every model id al name is re-spelled with the new one, the conversation's own pick first (a turn still answering is waited out), and with it the stored ones: reasoning levels, the worker, checker and planner pins, role pins, the fallback chain and the capability slots. A rename changes a label and nothing -else; it does not move the conversation onto a different model. `ctrl+r` on the row +else; it carries saved and temporary selections separately, keeping a temporary +replacement unsaved. `ctrl+r` on the row reconnects with the saved details. The `+ add a provider` row checks the address and asks for a name, requesting a key only when the server requires authentication, so the tab never sends you to `/connect` to add one. The `active provider` row reads diff --git a/internal/session/agent.go b/internal/session/agent.go index 1f45163151..e9ccb04f68 100644 --- a/internal/session/agent.go +++ b/internal/session/agent.go @@ -827,6 +827,9 @@ func (a *Agent) setModel(model string) ModelLanding { // that said `switching now` and a step that changed nothing (steer.go's // [Agent.ridingNowLocked]). changed := a.ridingNowLocked() != model + // An explicit confirmation also withdraws a queued temporary choice. + // This only changes the next request; the current call keeps answering. + a.spokenModel = "" a.modelForNowConfigured = "" a.modelForNowProvider = "" a.model = model diff --git a/internal/session/conversation_models.go b/internal/session/conversation_models.go index 2dcb7f9f87..0c09edc900 100644 --- a/internal/session/conversation_models.go +++ b/internal/session/conversation_models.go @@ -99,6 +99,9 @@ func (a *Agent) setConversationModels(pair ConversationModels, mainNotice string } active := a.modelIdentity.Active != "" a.conversationModels = &pair + // The pair is the next request's selection, including when its main did + // not change. A queued temporary main must not outlive this explicit pick. + a.spokenModel = "" a.modelForNowConfigured = "" a.modelForNowProvider = "" a.model = pair.Main diff --git a/internal/session/model_for_now_test.go b/internal/session/model_for_now_test.go index 93cd5bc1ff..695da2a676 100644 --- a/internal/session/model_for_now_test.go +++ b/internal/session/model_for_now_test.go @@ -1,12 +1,79 @@ package session import ( + "context" "path/filepath" "testing" + "time" + "github.com/Agent-Field/agentfield/sdk/go/ai" "github.com/Agent-Field/codeaf/internal/modelsource" ) +// C5: an explicit single or pair pick supersedes a queued temporary model +// before the next tool step, without cutting the request already on that pick. +func TestExplicitChoiceSupersedesQueuedRunOnlyModel(t *testing.T) { + for _, paired := range []bool{false, true} { + t.Run(map[bool]string{false: "single", true: "pair"}[paired], func(t *testing.T) { + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + defer cancel() + entered, release := make(chan struct{}), make(chan struct{}) + cut := make(chan struct{}, 1) + c := chained(nil, + func(callCtx context.Context, _ []ai.Message) (*ai.Response, error) { + close(entered) + select { + case <-release: + if callCtx.Err() != nil { + cut <- struct{}{} + } + return toolResponse("read", "read_file", `{"path":"README.md"}`), nil + case <-callCtx.Done(): + cut <- struct{}{} + return nil, context.Cause(callCtx) + } + }, + func(context.Context, []ai.Message) (*ai.Response, error) { return textResponse("done"), nil }, + ) + a, _ := newTestAgent(t, c, func(cfg *Config) { + if paired { + cfg.ConversationModels = &ConversationModels{Main: cfg.Model, Fallback: "chosen/backup"} + } + }) + stream, err := a.Submit(ctx, "read README") + if err != nil { + t.Fatal(err) + } + select { + case <-entered: + case <-ctx.Done(): + t.Fatal("first request did not start") + } + if err := a.UseModelForNow("available/model"); err != nil { + t.Fatal(err) + } + if paired { + _, err = a.SetConversationModels(ConversationModels{Main: "test/model", Fallback: "chosen/backup"}) + } else { + a.SetModel("test/model") + } + if err != nil { + t.Fatal(err) + } + close(release) + if events := collect(t, stream); len(textsOfKind(events, EventError)) != 0 { + t.Fatalf("turn failed: %v", textsOfKind(events, EventError)) + } + if len(cut) != 0 { + t.Fatal("confirming the active model cut its in-flight request") + } + if c.requests() != 2 || c.model(0) != "test/model" || c.model(1) != "test/model" || a.ConfiguredModel() != "test/model" { + t.Fatalf("requests=%d models=%q/%q configured=%q", c.requests(), c.model(0), c.model(1), a.ConfiguredModel()) + } + }) + } +} + // C1, C3, C4: a run-only model serves requests without replacing saved choices. func TestRunOnlyModelSurvivesSavingAsTheConfiguredChoice(t *testing.T) { for _, paired := range []bool{false, true} { diff --git a/internal/session/start_chat_tool_test.go b/internal/session/start_chat_tool_test.go index d57708015a..d370e35a07 100644 --- a/internal/session/start_chat_tool_test.go +++ b/internal/session/start_chat_tool_test.go @@ -44,6 +44,47 @@ func TestStartChatResolvesModelAndEmitsOnlyARequest(t *testing.T) { } } +// C1, C5: omitting a new chat's model inherits the configured selection of a +// temporary parent; an explicit model still names the person's chosen model. +func TestStartChatInheritsConfiguredModelFromTemporaryParent(t *testing.T) { + for _, explicit := range []bool{false, true} { + t.Run(map[bool]string{false: "inherited", true: "explicit"}[explicit], func(t *testing.T) { + args, want := `{"brief":"Read README"}`, "test/model" + if explicit { + args, want = `{"brief":"Read README","model":"chosen/model"}`, "chosen/model" + } + c := chained(nil, + func(context.Context, []ai.Message) (*ai.Response, error) { + return toolResponse("new-chat", "start_chat", args), nil + }, + func(context.Context, []ai.Message) (*ai.Response, error) { return textResponse("Requested."), nil }, + ) + a, _ := newTestAgent(t, c, func(cfg *Config) { + cfg.Interactive = true + cfg.Place = Place{Dir: t.TempDir()} + cfg.TaskModels = func() []string { return []string{"chosen/model"} } + }) + if err := a.UseModelForNow("available/model"); err != nil { + t.Fatal(err) + } + _, _ = a.loadCapability("chats") + stream, err := a.Submit(t.Context(), "start another chat") + if err != nil { + t.Fatal(err) + } + var request *ChatStart + for _, event := range collect(t, stream) { + if event.Kind == EventChatStart { + request = event.ChatStart + } + } + if request == nil || request.Model != want || a.Model() != "available/model" || a.ConfiguredModel() != "test/model" { + t.Fatalf("request=%+v live=%q configured=%q", request, a.Model(), a.ConfiguredModel()) + } + }) + } +} + func TestStartChatRefusesUnknownAndAmbiguousModels(t *testing.T) { agent, _ := newTestAgent(t, &scriptedCompleter{}, func(c *Config) { c.TaskModels = func() []string { return []string{"moonshotai/kimi-k3", "moonshotai/kimi-k3.1-pro"} } diff --git a/internal/session/tools_start_chat.go b/internal/session/tools_start_chat.go index 25d1a4053c..63730dfff7 100644 --- a/internal/session/tools_start_chat.go +++ b/internal/session/tools_start_chat.go @@ -26,7 +26,7 @@ func (a *Agent) startChatTools() []bare.Tool { if !a.config.Interactive || a.config.InTask || a.config.HandOverOnAsk || strings.TrimSpace(a.config.Place.Dir) == "" { return nil } - return []bare.Tool{{Name: "start_chat", Description: "Request another independent chat in the attached terminal, keeping this chat in front. Give its complete first message and, when the person names one, its model. The terminal reports whether it opened; this receipt only confirms the request. Use team_start for a teammate, quick_task for a result returned here, or propose_task for checked work in a folder of its own.", Schema: json.RawMessage(`{"type":"object","properties":{"brief":{"type":"string","description":"Complete first message for the new chat."},"model":{"type":"string","description":"Catalog ID or unique model name requested for this chat. Omit to use this chat's model."}},"required":["brief"],"additionalProperties":false}`), Execute: a.startChatTool}} + return []bare.Tool{{Name: "start_chat", Description: "Request another independent chat in the attached terminal, keeping this chat in front. Give its complete first message and, when the person names one, its model. The terminal reports whether it opened; this receipt only confirms the request. Use team_start for a teammate, quick_task for a result returned here, or propose_task for checked work in a folder of its own.", Schema: json.RawMessage(`{"type":"object","properties":{"brief":{"type":"string","description":"Complete first message for the new chat."},"model":{"type":"string","description":"Catalog ID or unique model name requested for this chat. Omit to use this chat's configured model."}},"required":["brief"],"additionalProperties":false}`), Execute: a.startChatTool}} } func (a *Agent) startChatTool(ctx context.Context, args json.RawMessage) (string, bool, error) { @@ -41,7 +41,7 @@ func (a *Agent) startChatTool(ctx context.Context, args json.RawMessage) (string if brief == "" { return rejectToolArguments(ctx, invalidArgumentsPrefix+"brief is empty"), true, nil } - model := a.Model() + model := a.ConfiguredModel() if word := strings.TrimSpace(parsed.Model); word != "" { choice := a.resolveTaskModel(word) if choice.problem != "" { diff --git a/internal/tui3/app.go b/internal/tui3/app.go index ec75db114d..b92e5d6b90 100644 --- a/internal/tui3/app.go +++ b/internal/tui3/app.go @@ -1967,6 +1967,9 @@ type app struct { // MID-SENTENCE, and dropping the move would leave them on the service they // just replaced, so the one pending id lands at the turn boundary. deferredModelServiceModel string + deferredModelRename *modelRename + modelChange *modelChange + modelCommands []tea.Cmd // harn is the subharness registry (Options.Harnesses) and harnPanel the // list /harness opens over it (harnesspanel.go). A nil harn is a surface // that cannot show harnesses and says so; nothing about the OFFER depends on @@ -3636,7 +3639,7 @@ func (a *app) Init() tea.Cmd { // legend needs before it may name the switcher (hop.go), and it is asked off // the loop for the reason every other reading on this list is: the walk opens // every project's index, and the paint path may never pay for one. - standing := []tea.Cmd{a.resumeAtLaunch, a.probeGit(), a.watchTasks(), a.watchWakes(), a.watchDesigns(), a.watchTitles(), + standing := []tea.Cmd{a.takeModelCommands(), a.resumeAtLaunch, a.probeGit(), a.watchTasks(), a.watchWakes(), a.watchDesigns(), a.watchTitles(), a.watchRuns(), a.watchQuestions(), a.loadTasks(), a.stirLane(), a.askHeld(), a.watchDriving(), a.watchFollowing(), a.linkPingTick(), a.prefetchReplayedPictures(), a.countConversations(), tea.RequestBackgroundColor, // AND WHAT AUTOMATIONS HAVE BEEN DOING, read off their store on a beat of @@ -3831,7 +3834,7 @@ func (a *app) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if summary := a.refreshRunSummary(); summary != nil { cmd = tea.Batch(cmd, summary) } - return model, cmd + return model, tea.Batch(cmd, a.takeModelCommands()) } // route is the message switch: every message this surface handles, handled once. @@ -7494,6 +7497,9 @@ func (a *app) submitting(text string, start func() (<-chan session.Event, error) // shown, so the transcript leaves them plain (entry.plainTags). func (a *app) submittingShown(text, shown string, plain []segment, start func() (<-chan session.Event, error)) tea.Cmd { if !a.ensureAvailableModel() { + if a.modelChangePending() { + return a.takeModelCommands() + } a.note(noAvailableModelWord) return nil } diff --git a/internal/tui3/credits.go b/internal/tui3/credits.go index fdd2b9b468..0e74eba3b2 100644 --- a/internal/tui3/credits.go +++ b/internal/tui3/credits.go @@ -167,7 +167,9 @@ func (a *app) refreshCreditWarnings() { (a.model == config.DefaultModel || a.model == config.FreeChatModel) && a.freshAndEmpty() && config.ChatModelAt(a.profileDir) == "" && (!a.requireListedModel || a.isAvailableModel(want)) { - a.useModelForNow(want, 0) + a.creditSwitching = true + a.switchModel(want, 0) + a.creditSwitching = false } a.chatCreditWarning, a.homeCreditWarning = "", "" if a.readCredits == nil { diff --git a/internal/tui3/input.go b/internal/tui3/input.go index d1ff056033..fdbdb1c563 100644 --- a/internal/tui3/input.go +++ b/internal/tui3/input.go @@ -1575,6 +1575,9 @@ func (a *app) enterCheckedLine(ownershipRead bool) tea.Cmd { // The surface can open while catalogs warm, but a guessed launch model // must never receive the message in the meantime. if !strings.HasPrefix(line, "/") && (line != "" || held) && !a.ensureAvailableModel() { + if a.modelChangePending() { + return a.takeModelCommands() + } a.note(noAvailableModelWord) a.openPicker() return nil diff --git a/internal/tui3/model_change_test.go b/internal/tui3/model_change_test.go new file mode 100644 index 0000000000..2f690fd2b0 --- /dev/null +++ b/internal/tui3/model_change_test.go @@ -0,0 +1,169 @@ +package tui3 + +import ( + "errors" + "sync" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/config" + "github.com/Agent-Field/codeaf/internal/modelsource" + "github.com/Agent-Field/codeaf/internal/session" +) + +type heldModelAgent struct { + *modelForNowAgent + entered chan struct{} + release chan struct{} + once sync.Once + err error +} + +func (a *heldModelAgent) UseModelForNow(id string) error { + a.once.Do(func() { close(a.entered) }) + select { + case <-a.release: + case <-time.After(2 * time.Second): + return errors.New("model door fixture timed out") + } + if a.err != nil { + return a.err + } + return a.modelForNowAgent.UseModelForNow(id) +} + +// C1, C4, C5: catalog selection returns to the loop while its door waits; +// the draft and alternate send gates stay closed until a successful reply. +func TestRunOnlySelectionWaitsForAcknowledgmentOffTheLoop(t *testing.T) { + for _, refused := range []bool{false, true} { + t.Run(map[bool]string{false: "accepted", true: "refused"}[refused], func(t *testing.T) { + agent := &heldModelAgent{ + modelForNowAgent: &modelForNowAgent{fakeAgent: &fakeAgent{model: "vendor/configured"}, configured: "vendor/configured"}, + entered: make(chan struct{}), release: make(chan struct{}), + } + if refused { + agent.err = errors.New("connection unavailable") + } + a := newApp(t.Context(), Options{Agent: agent, ProfileDir: t.TempDir(), Sources: modelsource.NewSet(testDefaultService("key")), + Models: func() []Model { return []Model{{ID: "vendor/replacement"}} }, + }) + a.requireListedModel = true + a.input.insert("keep this draft") + if a.ensureAvailableModel() || a.model != "vendor/configured" { + t.Fatal("unacknowledged selection opened the send gate") + } + select { + case <-agent.entered: + case <-time.After(time.Second): + close(agent.release) + t.Fatal("model door never started") + } + called := false + cmd := a.submitting("blocked", func() (<-chan session.Event, error) { called = true; return nil, nil }) + if called || a.input.String() != "keep this draft" || a.state == stateWorking { + t.Fatal("pending selection submitted or consumed the draft") + } + close(agent.release) + spend(t, a, cmd) + if refused { + if a.model != "vendor/configured" || agent.ConfiguredModel() != "vendor/configured" { + t.Fatal("refused selection changed the model") + } + if got := lastNote(t, a); got != "could not change model · connection unavailable" { + t.Fatalf("refusal=%q", got) + } + if a.ensureAvailableModel() { + t.Fatal("failed model selection opened the send gate") + } + acknowledgeModelChanges(t, a) + return + } + if !a.ensureAvailableModel() || a.model != "vendor/replacement" || agent.ConfiguredModel() != "vendor/configured" { + t.Fatal("successful selection did not resume reconciliation") + } + spend(t, a, a.enterLine()) + if len(agent.submitted) != 1 || agent.submitted[0] != "vendor/replacement" { + t.Fatalf("submitted=%v", agent.submitted) + } + }) + } +} + +// C5: an explicit pick queued behind a temporary selection wins in wire order +// and keeps its ordinary profile write and model receipt. +func TestExplicitPickSupersedesAnUnacknowledgedRunOnlySelection(t *testing.T) { + agent := &heldModelAgent{ + modelForNowAgent: &modelForNowAgent{fakeAgent: &fakeAgent{model: "vendor/configured"}, configured: "vendor/configured"}, + entered: make(chan struct{}), release: make(chan struct{}), + } + dir := t.TempDir() + a := newApp(t.Context(), Options{Agent: agent, ProfileDir: dir, SaveModel: func(id string) error { return config.WriteChatModel(dir, id) }}) + a.useModelForNow("vendor/replacement", 0) + a.switchModel("vendor/configured", 0) + close(agent.release) + acknowledgeModelChanges(t, a) + if agent.Model() != "vendor/configured" || agent.ConfiguredModel() != "vendor/configured" || config.ChatModelAt(dir) != "vendor/configured" { + t.Fatal("the temporary selection displaced the explicit pick") + } + if got := noteSaying(t, a, "model ·"); got != "model · vendor/configured" { + t.Fatalf("explicit receipt=%q", got) + } +} + +type heldLegacyModelAgent struct { + *fakeAgent + release chan struct{} +} + +// C5: leaving a conversation before its queued explicit pick is acknowledged +// does not lose its saved preference or apply its display to the next chat. +func TestPendingExplicitPickSavesAfterLeavingTheConversation(t *testing.T) { + for _, chooseAgain := range []bool{false, true} { + t.Run(map[bool]string{false: "leave", true: "choose in next chat"}[chooseAgain], func(t *testing.T) { + agent := &heldModelAgent{ + modelForNowAgent: &modelForNowAgent{fakeAgent: &fakeAgent{model: "vendor/configured"}, configured: "vendor/configured"}, + entered: make(chan struct{}), release: make(chan struct{}), + } + dir := t.TempDir() + a := newApp(t.Context(), Options{Agent: agent, ProfileDir: dir, SaveModel: func(id string) error { return config.WriteChatModel(dir, id) }}) + a.useModelForNow("vendor/replacement", 0) + a.switchModel("vendor/chosen", 0) + a.frontGen++ + a.agent, a.model = &fakeAgent{model: "other/model"}, "other/model" + wantSaved, wantLive := "vendor/chosen", "other/model" + if chooseAgain { + a.switchModel("other/chosen", 0) + wantSaved, wantLive = "other/chosen", "other/chosen" + } + close(agent.release) + acknowledgeModelChanges(t, a) + if agent.ConfiguredModel() != "vendor/chosen" || config.ChatModelAt(dir) != wantSaved || a.model != wantLive { + t.Fatal("leaving the conversation lost a choice or applied choices out of order") + } + }) + } +} + +func (a *heldLegacyModelAgent) SetModel(id string) { + select { + case <-a.release: + case <-time.After(2 * time.Second): + return + } + a.fakeAgent.SetModel(id) +} + +// C6: an older engine's SetModel fallback also waits off the update loop. +func TestLegacyRunOnlySelectionUsesTheOffLoopDoor(t *testing.T) { + agent := &heldLegacyModelAgent{fakeAgent: &fakeAgent{model: "vendor/configured"}, release: make(chan struct{})} + a := newApp(t.Context(), Options{Agent: agent, ProfileDir: t.TempDir()}) + a.useModelForNow("vendor/replacement", 0) + if !a.modelChangePending() || a.model != "vendor/configured" { + t.Fatal("the legacy model change did not wait for its acknowledgment") + } + close(agent.release) + acknowledgeModelChanges(t, a) + if a.model != "vendor/replacement" || agent.Model() != a.model { + t.Fatal("legacy model acknowledgment was lost") + } +} diff --git a/internal/tui3/model_for_now.go b/internal/tui3/model_for_now.go index ae538d71c7..2a1c09959f 100644 --- a/internal/tui3/model_for_now.go +++ b/internal/tui3/model_for_now.go @@ -1,5 +1,19 @@ package tui3 +import ( + "strings" + + tea "charm.land/bubbletea/v2" + "github.com/Agent-Field/codeaf/internal/session" +) + +// modelChange belongs to the conversation that queued it. Its acknowledgment +// may arrive after a later pick or after the surface has switched chats. +type modelChange struct { + front int + retry func() +} + // configuredModel keeps reconciliation anchored to the person's saved choice. // Agents predating the run-only door keep the existing live-model behavior. func (a *app) configuredModel() string { @@ -13,8 +27,86 @@ func (a *app) configuredModel() string { } func (a *app) useModelForNow(id string, window int) { - wasSwitching := a.creditSwitching - a.creditSwitching = true - a.switchModel(id, window) - a.creditSwitching = wasSwitching + a.queueModelChange(id, window, true, true, nil) +} + +func (a *app) modelChangePending() bool { + return a.modelChange != nil && a.modelChange.front == a.frontGen +} + +func (a *app) takeModelCommands() tea.Cmd { + cmds := a.modelCommands + a.modelCommands = nil + return tea.Batch(cmds...) +} + +// Temporary picks and any explicit picks behind them share the ordered door +// line. The surface only adopts the last acknowledged selection, and sends +// cannot pass its gate while that selection is pending. +func (a *app) queueModelChange(id string, window int, temporary, quiet bool, rename *modelRename) { + agent := a.agent + save := a.saveModel + remember := "" + if !quiet { + remember = strings.TrimSpace(id) + } + if rename != nil { + remember = rename.save + } + change := &modelChange{front: a.frontGen} + a.modelChange = change + cmd := a.offLoop(func() func(bool) tea.Cmd { + var err error + if rename != nil { + if door, ok := agent.(interface { + SetConversationModels(session.ConversationModels) ([]session.ModelSwitch, error) + }); rename.pair != nil && ok { + _, err = door.SetConversationModels(*rename.pair) + } else if rename.save != "" { + agent.SetModel(rename.save) + } + } + if err == nil { + if door, ok := agent.(interface{ UseModelForNow(string) error }); temporary && ok { + err = door.UseModelForNow(id) + } else if rename == nil { + agent.SetModel(id) + } + } + // Durable choices keep their order even if the window leaves before + // the reply. Only display work depends on still being in front. + if err == nil && save != nil && remember != "" { + _ = save(remember) + } + return func(here bool) tea.Cmd { + if a.modelChange != change { + return nil + } + if !here { + a.modelChange = nil + return nil + } + if err != nil { + change.retry = func() { a.queueModelChange(id, window, temporary, quiet, rename) } + a.note("could not change model · " + err.Error()) + return nil + } + a.modelChange = nil + wasSwitching := a.creditSwitching + a.creditSwitching = quiet + a.modelSwitched(id, window, false) + a.creditSwitching = wasSwitching + a.ensureAvailableModel() + if a.connPanel.open { + a.connPanel.adopt(a.connectionRows()) + } + if a.at(pageSettings) { + a.sheet.build() + } + a.hudStale = true + a.touch() + return a.takeModelCommands() + } + }) + a.modelCommands = append(a.modelCommands, cmd) } diff --git a/internal/tui3/model_for_now_test.go b/internal/tui3/model_for_now_test.go index e70155480b..d8fa63e757 100644 --- a/internal/tui3/model_for_now_test.go +++ b/internal/tui3/model_for_now_test.go @@ -17,6 +17,7 @@ type modelForNowAgent struct { *fakeAgent configured string temporary []string + submitted []string } func (a *modelForNowAgent) ConfiguredModel() string { return a.configured } @@ -30,6 +31,16 @@ func (a *modelForNowAgent) SetModel(model string) { a.fakeAgent.SetModel(model) } +func (a *modelForNowAgent) Submit(ctx context.Context, text string) (<-chan session.Event, error) { + a.submitted = append(a.submitted, a.Model()) + return a.fakeAgent.Submit(ctx, text) +} + +func acknowledgeModelChanges(t *testing.T, a *app) { + t.Helper() + spend(t, a, a.takeModelCommands()) +} + // C2, C4, C5: availability reconciles to the saved model without profile writes. func TestListedModelReplacementKeepsProfileAndReturnsToConfiguredModel(t *testing.T) { t.Setenv("CODEAF_HOME", t.TempDir()) @@ -53,6 +64,7 @@ func TestListedModelReplacementKeepsProfileAndReturnsToConfiguredModel(t *testin a := newApp(t.Context(), Options{Agent: agent, ProfileDir: dir, Sources: modelsource.NewSet(testDefaultService("key")), RequireListedModel: true, Models: func() []Model { return rows }, SaveModel: func(id string) error { return config.WriteChatModel(dir, id) }, }) + acknowledgeModelChanges(t, a) if a.model != "vendor/replacement" || agent.Model() != a.model || agent.ConfiguredModel() != "vendor/configured" || agent.window != 32768 { t.Fatalf("replacement=%q/%q configured=%q window=%d", a.model, agent.Model(), agent.ConfiguredModel(), agent.window) } @@ -63,6 +75,7 @@ func TestListedModelReplacementKeepsProfileAndReturnsToConfiguredModel(t *testin t.Fatal("catalog recovery changed an answering request") } a.settle() + acknowledgeModelChanges(t, a) if a.model != "vendor/configured" || agent.Model() != a.model || agent.window != 65536 { t.Fatalf("recovered=%q/%q window=%d", a.model, agent.Model(), agent.window) } @@ -73,6 +86,7 @@ func TestListedModelReplacementKeepsProfileAndReturnsToConfiguredModel(t *testin rows = rows[:1] a.serviceModelsLanded(modelsource.DefaultID, config.DefaultBaseURL) a.slash("/model vendor/replacement") + acknowledgeModelChanges(t, a) if agent.ConfiguredModel() != "vendor/replacement" || config.ChatModelAt(dir) != "vendor/replacement" || a.modelWord() != "vendor/replacement" { t.Fatal("confirming the fallback did not save the explicit choice and HUD") } @@ -97,9 +111,11 @@ func TestCatalogRecoveryUsesConfiguredModelOnTheNextRequest(t *testing.T) { a := newApp(t.Context(), Options{Agent: agent, ProfileDir: t.TempDir(), Sources: sources, RequireListedModel: true, ModelsForService: func(modelsource.Connected) []Model { return rows }, }) + acknowledgeModelChanges(t, a) drainModelServiceTurn(t, agent, "use the available model") rows = append(rows, Model{ID: "configured"}) a.serviceModelsLanded(service.Source.ID, service.Address) + acknowledgeModelChanges(t, a) drainModelServiceTurn(t, agent, "use the recovered model") requests := completionRequests(server) if len(requests) != 2 || completionModel(t, requests[0]) != "replacement" || completionModel(t, requests[1]) != "configured" { @@ -113,6 +129,7 @@ func TestSetupCanExplicitlyChooseTheRunOnlyModel(t *testing.T) { agent := &modelForNowAgent{fakeAgent: &fakeAgent{model: "vendor/configured"}, configured: "vendor/configured"} a := newApp(t.Context(), Options{Agent: agent, ProfileDir: dir, SaveModel: func(id string) error { return config.WriteChatModel(dir, id) }}) a.useModelForNow("vendor/replacement", 0) + acknowledgeModelChanges(t, a) if config.ChatModelAt(dir) != "" { t.Fatal("an automatic setup model was saved") } @@ -122,19 +139,37 @@ func TestSetupCanExplicitlyChooseTheRunOnlyModel(t *testing.T) { } } -// C2: automatic credit defaults change this run, without changing its choice. -func TestCreditDefaultUsesTheRunOnlyDoor(t *testing.T) { - t.Setenv("CODEAF_HOME", t.TempDir()) - agent := &modelForNowAgent{fakeAgent: &fakeAgent{model: config.DefaultModel}, configured: config.DefaultModel} - a := newApp(t.Context(), Options{Agent: agent, ProfileDir: t.TempDir(), ImplicitTalk: true}) - if err := config.SetCrewFreeRoutes(a.profileDir, true); err != nil { - t.Fatal(err) - } - seedLowCredits(t, a) - a.readCredits = func(context.Context) (credits.Reading, error) { return credits.Reading{Known: true, Low: true}, nil } - a.refreshCreditWarnings() - if agent.Model() != config.FreeChatModel || agent.ConfiguredModel() != config.DefaultModel || config.ChatModelAt(a.profileDir) != "" || len(agent.temporary) == 0 { - t.Fatalf("credit default=%q configured=%q", agent.Model(), agent.ConfiguredModel()) +// C6: an untouched implicit credit default follows the ordinary saved model +// road, and availability reconciliation preserves it at the submission gate. +func TestCreditDefaultSurvivesListedModelSubmission(t *testing.T) { + for _, low := range []bool{true, false} { + t.Run(map[bool]string{true: "low", false: "recovered"}[low], func(t *testing.T) { + t.Setenv("CODEAF_HOME", t.TempDir()) + initial, want := config.DefaultModel, config.FreeChatModel + if !low { + initial, want = want, initial + } + agent := &modelForNowAgent{fakeAgent: &fakeAgent{model: initial}, configured: initial} + a := newApp(t.Context(), Options{Agent: agent, ProfileDir: t.TempDir(), ImplicitTalk: true, RequireListedModel: true, + Sources: modelsource.NewSet(testDefaultService("credit-test-key")), + Models: func() []Model { return []Model{{ID: config.DefaultModel}, {ID: config.FreeChatModel}} }, + }) + if err := config.SetCrewFreeRoutes(a.profileDir, true); err != nil { + t.Fatal(err) + } + seedLowCredits(t, a) + reading := credits.Reading{Known: true, Low: low} + if err := config.WriteCreditsReading(a.profileDir, "credit-test-key", reading); err != nil { + t.Fatal(err) + } + a.readCredits = func(context.Context) (credits.Reading, error) { return reading, nil } + a.refreshCreditWarnings() + a.input.insert("answer on the credit default") + spend(t, a, a.enterLine()) + if len(agent.submitted) != 1 || agent.submitted[0] != want || agent.ConfiguredModel() != want || config.ChatModelAt(a.profileDir) != "" || len(agent.temporary) != 0 { + t.Fatalf("submitted=%v configured=%q temporary=%v", agent.submitted, agent.ConfiguredModel(), agent.temporary) + } + }) } } @@ -178,7 +213,10 @@ func TestServiceConnectionPersistsIdleAndDeferredMoves(t *testing.T) { t.Fatal(err) } if testCase.running { + a.requireListedModel = false a.useModelForNow(want, 0) + acknowledgeModelChanges(t, a) + a.requireListedModel = true } temporary := len(agent.temporary) opening := a.model @@ -226,6 +264,7 @@ func TestDisconnectMoveStaysTemporaryUntilExplicitlyChosen(t *testing.T) { a.agent = agent a.requireListedModel = true a.disconnectModelService("z-ai") + acknowledgeModelChanges(t, a) if a.model != fallback || agent.Model() != fallback || agent.ConfiguredModel() != configured || config.ChatModelAt(dir) != configured || len(agent.temporary) == 0 { t.Fatal("disconnect did not keep its replacement temporary") } diff --git a/internal/tui3/model_rename.go b/internal/tui3/model_rename.go new file mode 100644 index 0000000000..5d873bdba7 --- /dev/null +++ b/internal/tui3/model_rename.go @@ -0,0 +1,71 @@ +package tui3 + +import ( + "github.com/Agent-Field/codeaf/internal/config" + "github.com/Agent-Field/codeaf/internal/session" +) + +// A label change carries saved and temporary selections independently. A +// working turn keeps both until settlement, alongside any explicit next pick. +type modelRename struct { + save string + live string + pair *session.ConversationModels +} + +func (a *app) reprefixTemporaryModel(oldWritten, newWritten string) bool { + configured, live := a.configuredModel(), a.model + var saved string + var changedPair *session.ConversationModels + var pair *session.ConversationModels + if door, ok := a.agent.(interface{ ModelIdentity() session.ModelIdentity }); ok { + if saved := door.ModelIdentity().Models; saved != nil { + copy := *saved + pair = © + } + } + if deferred := a.deferredModelRename; deferred != nil { + saved, changedPair = deferred.save, deferred.pair + if deferred.save != "" { + configured = deferred.save + } + live = deferred.live + if deferred.pair != nil { + pair = deferred.pair + } + } + if configured == live && pair == nil && a.deferredModelRename == nil { + return false + } + next := config.ReprefixModelID(configured, oldWritten, newWritten) + rename := &modelRename{save: saved, pair: changedPair, live: config.ReprefixModelID(live, oldWritten, newWritten)} + if next != configured { + rename.save = next + } + if pair != nil { + updated := session.ConversationModels{ + Main: config.ReprefixModelID(pair.Main, oldWritten, newWritten), + Fallback: config.ReprefixModelID(pair.Fallback, oldWritten, newWritten), + } + if updated != *pair { + rename.pair = &updated + } + } + if rename.save == "" && rename.pair == nil && rename.live == live { + return true + } + if a.state == stateWorking { + a.deferredModelRename = rename + } else { + a.applyModelRename(rename) + } + return true +} + +func (a *app) applyModelRename(rename *modelRename) { + configured := a.configuredModel() + if rename.save != "" { + configured = rename.save + } + a.queueModelChange(rename.live, 0, rename.live != configured, true, rename) +} diff --git a/internal/tui3/model_rename_test.go b/internal/tui3/model_rename_test.go new file mode 100644 index 0000000000..cf12057de7 --- /dev/null +++ b/internal/tui3/model_rename_test.go @@ -0,0 +1,95 @@ +package tui3 + +import ( + "testing" + + "github.com/Agent-Field/codeaf/internal/config" + "github.com/Agent-Field/codeaf/internal/modelsource" + "github.com/Agent-Field/codeaf/internal/session" +) + +type pairedModelForNowAgent struct { + *modelForNowAgent + pair session.ConversationModels +} + +func (a *pairedModelForNowAgent) ModelIdentity() session.ModelIdentity { + pair := a.pair + return session.ModelIdentity{Selected: a.configured, Models: &pair} +} + +func (a *pairedModelForNowAgent) SetConversationModels(pair session.ConversationModels) ([]session.ModelSwitch, error) { + a.pair = pair + a.SetModel(pair.Main) + return nil, nil +} + +// C1, C2, C4: idle and deferred service renames carry the configured and +// temporary models separately. The configured model returns when listed. +func TestServiceRenamePreservesConfiguredAndRunOnlySelections(t *testing.T) { + for _, working := range []bool{false, true} { + for _, configuredService := range []bool{false, true} { + for _, paired := range []bool{false, true} { + name := map[bool]string{false: "idle", true: "deferred"}[working] + "/" + map[bool]string{false: "replacement service", true: "configured service"}[configuredService] + "/" + map[bool]string{false: "single", true: "pair"}[paired] + t.Run(name, func(t *testing.T) { + dir := t.TempDir() + customs := connectionWriteSources(t, dir, "mybox") + configured, live := "vendor/configured", "mybox/replacement" + wantConfigured, wantLive := configured, "lab/replacement" + var cloud []Model + if configuredService { + configured, live = "mybox/chosen", "vendor/replacement" + wantConfigured, wantLive = "lab/chosen", live + cloud = []Model{{ID: live}} + } + base := &modelForNowAgent{fakeAgent: &fakeAgent{model: configured}, configured: configured} + var agent Agent = base + var pairAgent *pairedModelForNowAgent + if paired { + pairAgent = &pairedModelForNowAgent{modelForNowAgent: base, pair: session.ConversationModels{Main: configured, Fallback: "mybox/backup"}} + agent = pairAgent + } + a := modelServiceTestAppWithAgent(t, dir, configured, modelsource.NewSet(append([]modelsource.Connected{connectionDefaultService()}, customs...)...), cloud, agent) + a.modelsForService = func(modelsource.Connected) []Model { return []Model{{ID: "replacement"}} } + if err := config.WriteChatModel(dir, configured); err != nil { + t.Fatal(err) + } + a.useModelForNow(live, 0) + acknowledgeModelChanges(t, a) + a.requireListedModel = true + if working { + a.state = stateWorking + } + rows := config.PersistedSources(dir) + rows[0].Written = "lab" + if err := config.WriteSources(dir, rows); err != nil { + t.Fatal(err) + } + a.adoptModelConnectResult(modelConnectResultMsg{service: customs[0].Source.ID, written: "lab", renamedFrom: "mybox", outcome: modelsource.Outcome{Kind: modelsource.OutcomeConnected}, models: []Model{{ID: "replacement"}}}) + if working { + if a.model != live || base.ConfiguredModel() != configured { + t.Fatal("rename changed the answering turn") + } + a.settle() + } + acknowledgeModelChanges(t, a) + if a.model != wantLive || base.Model() != wantLive || base.ConfiguredModel() != wantConfigured || config.ChatModelAt(dir) != wantConfigured { + t.Fatalf("live=%q/%q configured=%q saved=%q", a.model, base.Model(), base.ConfiguredModel(), config.ChatModelAt(dir)) + } + if paired && (pairAgent.pair.Main != wantConfigured || pairAgent.pair.Fallback != "lab/backup") { + t.Fatalf("renamed saved pair=%+v", pairAgent.pair) + } + if configuredService { + a.modelsForService = func(modelsource.Connected) []Model { return []Model{{ID: "replacement"}, {ID: "chosen"}} } + delete(a.sourceModels, customs[0].Source.ID) + a.serviceModelsLanded(customs[0].Source.ID, customs[0].Address) + acknowledgeModelChanges(t, a) + if a.model != wantConfigured || base.Model() != wantConfigured || config.ChatModelAt(dir) != wantConfigured { + t.Fatal("the renamed configured model did not return when listed") + } + } + }) + } + } + } +} diff --git a/internal/tui3/modeldefault.go b/internal/tui3/modeldefault.go index 3e3ccd2088..9020b97192 100644 --- a/internal/tui3/modeldefault.go +++ b/internal/tui3/modeldefault.go @@ -37,10 +37,16 @@ func (a *app) isAvailableModel(id string) bool { // ensureAvailableModel reconciles a local conversation with its picker catalog. // A launch preference is not evidence of availability. An automatic choice uses -// the ordinary model-change road without saving a preference the person did not +// the off-loop run-only door without saving a preference the person did not // choose. With no rows, only the display is cleared: the engine has no unset-model // door, and the send gates below keep that old launch preference from being used. func (a *app) ensureAvailableModel() bool { + if a.modelChangePending() { + if retry := a.modelChange.retry; retry != nil { + retry() + } + return false + } if !a.requireListedModel || a.agent == nil { return true } @@ -59,6 +65,7 @@ func (a *app) ensureAvailableModel() bool { if model.ID != a.model { a.useModelForNow(model.ID, model.ContextLength) a.hudStale = true + return false } return a.model == model.ID } diff --git a/internal/tui3/modeldefault_test.go b/internal/tui3/modeldefault_test.go index 22b9fbf800..14d067dcf5 100644 --- a/internal/tui3/modeldefault_test.go +++ b/internal/tui3/modeldefault_test.go @@ -43,6 +43,7 @@ func TestTheOpeningModelAlwaysBelongsToItsAvailablePicker(t *testing.T) { ModelsForService: func(modelsource.Connected) []Model { return test.installed }, SaveModel: func(id string) error { saved = id; return nil }, }) + acknowledgeModelChanges(t, a) if a.model != test.want { t.Fatalf("opening model = %q, want %q", a.model, test.want) } @@ -67,6 +68,7 @@ func TestAColdCatalogHoldsTheDraftThenSuppliesTheEngineDefault(t *testing.T) { } rows = []Model{{ID: "vendor/real", ContextLength: 32768}} a.serviceModelsLanded(modelsource.DefaultID, config.DefaultBaseURL) + acknowledgeModelChanges(t, a) if a.model != "vendor/real" || agent.Model() != a.model || agent.window != 32768 || a.input.String() != "keep this draft" { t.Fatalf("landed default = %q/%q window=%d draft=%q", a.model, agent.Model(), agent.window, a.input.String()) } @@ -88,6 +90,7 @@ func TestRemovingTheDefaultModelChoosesAnotherListedModel(t *testing.T) { a := newApp(t.Context(), Options{Agent: agent, ProfileDir: t.TempDir(), Sources: modelsource.NewSet(testDefaultService("key")), RequireListedModel: true, Models: func() []Model { return rows }}) rows = []Model{{ID: "vendor/replacement"}} a.serviceModelsLanded(modelsource.DefaultID, config.DefaultBaseURL) + acknowledgeModelChanges(t, a) if a.model != "vendor/replacement" || agent.Model() != a.model { t.Fatalf("replacement = %q/%q", a.model, agent.Model()) } @@ -141,6 +144,7 @@ func TestAnAvailableOpeningModelIsTheModelSentToItsRealProvider(t *testing.T) { Models: func() []Model { return []Model{{ID: config.DefaultModel}} }, ModelsForService: func(modelsource.Connected) []Model { return []Model{{ID: "actually-listed"}} }, }) + acknowledgeModelChanges(t, a) if a.model != "deepseek-direct/actually-listed" { t.Fatalf("default = %q", a.model) } @@ -163,6 +167,7 @@ func TestARefreshDefersDefaultReplacementUntilTheAnswerSettles(t *testing.T) { t.Fatal("the catalog change interrupted an answering request") } a.settle() + acknowledgeModelChanges(t, a) if a.model != "vendor/replacement" || agent.Model() != a.model { t.Fatalf("settled default = %q/%q", a.model, agent.Model()) } diff --git a/internal/tui3/modelservices.go b/internal/tui3/modelservices.go index efcbb948de..e6c3318392 100644 --- a/internal/tui3/modelservices.go +++ b/internal/tui3/modelservices.go @@ -1012,6 +1012,9 @@ func (a *app) reprefixRenamedModel(oldWritten, newWritten string) string { if modelUsesService(a.deferredModelServiceModel, oldWritten) { a.deferredModelServiceModel = config.ReprefixModelID(a.deferredModelServiceModel, oldWritten, newWritten) } + if a.reprefixTemporaryModel(oldWritten, newWritten) { + return "" + } if !modelUsesService(a.model, oldWritten) { return "" } @@ -1127,6 +1130,10 @@ func (a *app) moveConversationToConnectedModel(next string) { // answering turn has settled. Clearing it first makes the second settle event // free and prevents a failed later path from replaying an old connection. func (a *app) applyDeferredModelServiceMove() { + if rename := a.deferredModelRename; rename != nil { + a.deferredModelRename = nil + a.applyModelRename(rename) + } next := a.deferredModelServiceModel a.deferredModelServiceModel = "" was := a.model diff --git a/internal/tui3/modelservices_test.go b/internal/tui3/modelservices_test.go index cd63ecc0b2..2bcec3bc9f 100644 --- a/internal/tui3/modelservices_test.go +++ b/internal/tui3/modelservices_test.go @@ -1365,6 +1365,7 @@ func TestDisconnectingAServiceLeavesTheConversationOnSomethingItCanReach(t *test a := modelServiceTestAppWithAgent(t, dir, agent.Model(), sources, []Model{{ID: config.DefaultModel}}, agent) a.disconnectModelService("deepseek") + acknowledgeModelChanges(t, a) if got := noteSaying(t, a, "is disconnected"); got != "deepseek-direct is disconnected · its models are gone from the picker" { t.Fatalf("disconnect acknowledgement = %q", got) } @@ -1673,6 +1674,7 @@ func TestDisconnectingReplacesTheLiveClientBeforeTheNextRequest(t *testing.T) { drainModelServiceTurn(t, agent, "before disconnect") a := modelServiceTestAppWithAgent(t, dir, agent.Model(), sources, []Model{{ID: config.DefaultModel}}, agent) a.disconnectModelService("deepseek") + acknowledgeModelChanges(t, a) drainModelServiceTurn(t, agent, "after disconnect") if got := completionRequests(directServer); len(got) != 1 || got[0].Bearer != "Bearer sk-direct-1234567890" { t.Fatalf("removed service received another request or wrong bearer: %+v", got) diff --git a/internal/tui3/palette.go b/internal/tui3/palette.go index f21ca987a6..c3feb377cc 100644 --- a/internal/tui3/palette.go +++ b/internal/tui3/palette.go @@ -3106,13 +3106,15 @@ func (a *app) windowFor(id string) int { // fires at a fraction of it, so a session that switched to a 1M model without // saying so would keep compacting as if it were on the 128k one it started on. func (a *app) switchModel(id string, window int) { - if door, ok := a.agent.(interface{ UseModelForNow(string) error }); a.creditSwitching && ok { - if err := door.UseModelForNow(id); err != nil { - return - } - } else { - a.agent.SetModel(id) + if a.modelChange != nil { + a.queueModelChange(id, window, false, a.creditSwitching, nil) + return } + a.agent.SetModel(id) + a.modelSwitched(id, window, true) +} + +func (a *app) modelSwitched(id string, window int, remember bool) { a.model = a.agent.Model() if a.model == "" { a.model = id @@ -3131,7 +3133,9 @@ func (a *app) switchModel(id string, window int) { // this number (see [app.ctxPercent]). a.ctxWindow = window } - a.rememberModel(a.model) + if remember { + a.rememberModel(a.model) + } a.refreshCreditWarnings() if a.chatCreditWarning != "" { a.askCredits(credits.PaidSwitch) diff --git a/internal/tui3/provideraccess_test.go b/internal/tui3/provideraccess_test.go index 708e5af047..d6d5533f90 100644 --- a/internal/tui3/provideraccess_test.go +++ b/internal/tui3/provideraccess_test.go @@ -135,6 +135,7 @@ func TestAddingOpenRouterFromTheModelMenuPreservesOllamaAndHandsOverTheKey(t *te a.requireListedModel = true a.model = "vendor/cloud" a.connectAct(a.connPanel.cursor) + acknowledgeModelChanges(t, a) if a.model != "ollama/installed" || a.agent.Model() != "ollama/installed" { t.Fatal("removing OpenRouter did not move the next request to available Ollama") } diff --git a/internal/tui3/startchat.go b/internal/tui3/startchat.go index e1af62c518..99b11aa70d 100644 --- a/internal/tui3/startchat.go +++ b/internal/tui3/startchat.go @@ -26,7 +26,12 @@ func (a *app) startRequestedChat(request *session.ChatStart, owner Agent) tea.Cm return nil } start := a.start - return a.besideLine(func() func(bool) tea.Cmd { + listed := a.requireListedModel + var models []Model + if listed { + models = a.modelList() + } + return a.offLoop(func() func(bool) tea.Cmd { claimer, ok := owner.(interface{ ClaimChatStart(string) (bool, error) }) if !ok { return func(bool) tea.Cmd { a.note("could not start chat · launch ownership unavailable"); return nil } @@ -44,6 +49,9 @@ func (a *app) startRequestedChat(request *session.ChatStart, owner Agent) tea.Cm err = fmt.Errorf("the new conversation has no agent") } if err == nil { + // The request carries the parent's saved choice. Availability is + // reconciled for this child before its first request, without saving + // a temporary replacement inherited from the parent. conv.Agent.SetModel(request.Model) if request.Approval != "" { if setter, ok := conv.Agent.(interface{ SetApprovalPosture(string) error }); ok { @@ -53,6 +61,21 @@ func (a *app) startRequestedChat(request *session.ChatStart, owner Agent) tea.Cm } } } + if err == nil && listed { + model, available := availableConversationModel(request.Model, models) + if !available { + err = fmt.Errorf("%s", noAvailableModelWord) + } else if model.ID != request.Model { + if door, ok := conv.Agent.(interface{ UseModelForNow(string) error }); ok { + err = door.UseModelForNow(model.ID) + } else { + conv.Agent.SetModel(model.ID) + } + } + if err == nil && model.ContextLength > 0 { + conv.Agent.SetContextWindow(model.ContextLength) + } + } if err == nil { events, err = conv.Agent.Submit(context.Background(), request.Brief) } diff --git a/internal/tui3/startchat_test.go b/internal/tui3/startchat_test.go index 73e7dfd9f9..470609a1bd 100644 --- a/internal/tui3/startchat_test.go +++ b/internal/tui3/startchat_test.go @@ -1,14 +1,110 @@ package tui3 import ( + "context" "path/filepath" "sync" "testing" + "time" + "github.com/Agent-Field/codeaf/internal/modelsource" + "github.com/Agent-Field/codeaf/internal/modelsource/sourcestub" "github.com/Agent-Field/codeaf/internal/session" teamstore "github.com/Agent-Field/codeaf/internal/teams" ) +type observedRequestedChat struct { + *session.Agent + done chan struct{} +} + +func (a *observedRequestedChat) Submit(ctx context.Context, text string) (<-chan session.Event, error) { + ctx, cancel := context.WithTimeout(ctx, 5*time.Second) + events, err := a.Agent.Submit(ctx, text) + if err != nil { + cancel() + return nil, err + } + forward := make(chan session.Event, 128) + go func() { + defer cancel() + defer close(a.done) + defer close(forward) + for event := range events { + select { + case forward <- event: + case <-ctx.Done(): + return + } + } + }() + return forward, nil +} + +// C1, C4, C5: a temporary parent's configured model is saved in an inherited +// chat. That child reconciles availability before sending and reopens on its +// saved choice; an explicit-model request retains its persistent behavior. +func TestRequestedChatSavesConfiguredModelBeforeItsOwnAvailabilityReconcile(t *testing.T) { + for _, explicit := range []bool{false, true} { + t.Run(map[bool]string{false: "inherited", true: "explicit"}[explicit], func(t *testing.T) { + server := sourcestub.New("replacement", "explicit") + defer server.Close() + service := testDirectService(server.URL()) + sources := modelsource.NewSet(testDefaultService(""), service) + a, _, _ := tabApp(t) + parent := &modelForNowAgent{fakeAgent: &fakeAgent{model: "deepseek-direct/configured"}, configured: "deepseek-direct/configured"} + if err := parent.UseModelForNow("deepseek-direct/replacement"); err != nil { + t.Fatal(err) + } + a.agent = parent + owner := &claimingChatAgent{Agent: parent, claimed: &sync.Map{}} + a.sources, a.requireListedModel = sources, true + a.modelsForService = func(modelsource.Connected) []Model { return []Model{{ID: "replacement"}, {ID: "explicit"}} } + childDir := t.TempDir() + child, err := session.New(session.Config{Workspace: childDir, SessionFile: filepath.Join(childDir, "transcript.jsonl"), Place: session.Place{Dir: childDir}, Model: "deepseek-direct/configured", Sources: sources, DeterministicNames: true}) + if err != nil { + t.Fatal(err) + } + defer child.Close() + observed := &observedRequestedChat{Agent: child, done: make(chan struct{})} + a.start = func(where string) (Conversation, error) { + return Conversation{Agent: observed, SessionFile: filepath.Join(childDir, "transcript.jsonl"), Workspace: where}, nil + } + model, wantRequest := parent.ConfiguredModel(), "replacement" + if explicit { + model, wantRequest = "deepseek-direct/explicit", "explicit" + } + request := &session.ChatStart{ID: "new-chat", Workspace: childDir, Model: model, Brief: "answer briefly"} + spend(t, a, a.startRequestedChat(request, owner)) + select { + case <-observed.done: + case <-time.After(5 * time.Second): + t.Fatal("requested chat did not finish") + } + if err := child.Close(); err != nil { + t.Fatal(err) + } + child.SettleWrites() + meta, err := session.LoadMeta(childDir) + if err != nil || meta.Model != model || meta.ModelIdentity == nil || meta.ModelIdentity.Selected != model { + t.Fatalf("child metadata=%+v, %v", meta, err) + } + requests := completionRequests(server) + if len(requests) != 1 || completionModel(t, requests[0]) != wantRequest { + t.Fatalf("child requests=%+v", requests) + } + reopened, err := session.New(session.Config{Workspace: childDir, SessionFile: filepath.Join(childDir, "transcript.jsonl"), Place: session.Place{Dir: childDir}, Model: meta.Model, Sources: sources, DeterministicNames: true}) + if err != nil { + t.Fatal(err) + } + defer reopened.Close() + if reopened.ConfiguredModel() != model || reopened.Model() != model || parent.ConfiguredModel() != "deepseek-direct/configured" { + t.Fatal("child reopen or parent choice inherited the temporary model") + } + }) + } +} + func TestStartedTeammateUsesNamedModelBeforeWaking(t *testing.T) { a, harbor, _, _ := trafficApp(t) var fresh *postureTestAgent From ed52c29eab7105507f7e2b49801c4a9f3a028e30 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 6 Nov 2026 10:13:42 -0500 Subject: [PATCH 4/6] Models: parked sends and deferred renames wait for their own chat's model switch Review round 2: - A parked message (text and attachments) stays queued while its chat's run-only switch is pending or refused, and is sent once on the acknowledged model; pending switches travel with the chat when the window moves to another. - A deferred service rename is bound to the chat it was made for and is retired by a later explicit pick, so it never saves into another chat or over a newer choice. - Two new test fixtures gain the provider base URL their sessions require. Co-Authored-By: Claude Opus 5.5 --- cmd/codeaf/chatv3_model_for_now_test.go | 2 +- internal/manual/chat/models-and-cost.md | 2 + internal/manual/chat/services.md | 3 +- internal/remote/model_for_now_test.go | 2 +- internal/tui3/attach.go | 7 + internal/tui3/detach.go | 41 ++-- internal/tui3/hostreplay.go | 3 +- internal/tui3/keeper.go | 3 + internal/tui3/model_for_now.go | 24 ++- internal/tui3/model_park_test.go | 226 +++++++++++++++++++++++ internal/tui3/model_rename.go | 38 +++- internal/tui3/model_rename_owner_test.go | 121 ++++++++++++ internal/tui3/modeldefault.go | 9 + internal/tui3/palette.go | 1 + internal/tui3/park.go | 7 +- 15 files changed, 463 insertions(+), 26 deletions(-) create mode 100644 internal/tui3/model_park_test.go create mode 100644 internal/tui3/model_rename_owner_test.go diff --git a/cmd/codeaf/chatv3_model_for_now_test.go b/cmd/codeaf/chatv3_model_for_now_test.go index 118794c586..2a0fa70262 100644 --- a/cmd/codeaf/chatv3_model_for_now_test.go +++ b/cmd/codeaf/chatv3_model_for_now_test.go @@ -14,7 +14,7 @@ import ( // temporary model as a saved choice before catalog reconciliation can restore it. func TestLinkedLocalSourceRefreshKeepsRunOnlyModelTemporary(t *testing.T) { dir, profile := t.TempDir(), t.TempDir() - agent, err := session.New(session.Config{Workspace: t.TempDir(), Place: session.Place{Dir: dir}, Model: "vendor/configured", APIKey: "fixture"}) + agent, err := session.New(session.Config{Workspace: t.TempDir(), Place: session.Place{Dir: dir}, Model: "vendor/configured", APIKey: "fixture", BaseURL: "http://127.0.0.1:1/v1"}) if err != nil { t.Fatal(err) } diff --git a/internal/manual/chat/models-and-cost.md b/internal/manual/chat/models-and-cost.md index 8220979937..5a931e3a91 100644 --- a/internal/manual/chat/models-and-cost.md +++ b/internal/manual/chat/models-and-cost.md @@ -133,6 +133,8 @@ stays in the draft. The picker opens with: When a provider's list arrives, codeaf selects an available model. Press Enter again to send the draft after the model selection succeeds. A failed selection reports `could not change model · ` and leaves the draft unsent. +Waiting messages keep their text and attachments until selection succeeds, then +send once in their own conversation. Refreshing a list or removing a connection also replaces a model that is no longer listed; existing requests finish on the model they started with. diff --git a/internal/manual/chat/services.md b/internal/manual/chat/services.md index 18a0892159..c5242ac1fe 100644 --- a/internal/manual/chat/services.md +++ b/internal/manual/chat/services.md @@ -310,7 +310,8 @@ name is re-spelled with the new one, the conversation's own pick first (a turn s answering is waited out), and with it the stored ones: reasoning levels, the worker, checker and planner pins, role pins, the fallback chain and the capability slots. A rename changes a label and nothing else; it carries saved and temporary selections separately, keeping a temporary -replacement unsaved. `ctrl+r` on the row +replacement unsaved. A deferred rename stays with its conversation; a later +explicit model or main/fallback choice supersedes it. `ctrl+r` on the row reconnects with the saved details. The `+ add a provider` row checks the address and asks for a name, requesting a key only when the server requires authentication, so the tab never sends you to `/connect` to add one. The `active provider` row reads diff --git a/internal/remote/model_for_now_test.go b/internal/remote/model_for_now_test.go index 9953ea4c2c..eef76957e3 100644 --- a/internal/remote/model_for_now_test.go +++ b/internal/remote/model_for_now_test.go @@ -10,7 +10,7 @@ import ( // C1, C4, C5: the linked-local wire distinguishes temporary and explicit picks. func TestRunOnlyModelCrossesTheWireWithoutReplacingTheSavedPair(t *testing.T) { dir := t.TempDir() - a, err := session.New(session.Config{Workspace: t.TempDir(), Place: session.Place{Dir: dir}, Model: "vendor/configured", APIKey: "fixture", + a, err := session.New(session.Config{Workspace: t.TempDir(), Place: session.Place{Dir: dir}, Model: "vendor/configured", APIKey: "fixture", BaseURL: "http://127.0.0.1:1/v1", ConversationModels: &session.ConversationModels{Main: "vendor/configured", Fallback: "vendor/backup"}, }) if err != nil { diff --git a/internal/tui3/attach.go b/internal/tui3/attach.go index d1b9f44c49..052d845639 100644 --- a/internal/tui3/attach.go +++ b/internal/tui3/attach.go @@ -916,6 +916,13 @@ func (a *app) submitImages(text string) tea.Cmd { } func (a *app) submitImagesShown(text, shown string, plain []segment) tea.Cmd { + if !a.ensureAvailableModel() { + if a.modelChangePending() { + return a.takeModelCommands() + } + a.note(noAvailableModelWord) + return nil + } agent, ctx := a.agent, a.ctx chips := append([]chip(nil), a.chips...) a.chips, a.sent = nil, chips diff --git a/internal/tui3/detach.go b/internal/tui3/detach.go index 12d6aeaa7b..8fa81a5946 100644 --- a/internal/tui3/detach.go +++ b/internal/tui3/detach.go @@ -63,6 +63,10 @@ type aside struct { // surface yet. It follows the sidecar so returning mid-send cannot send the // same message again when the stream closes. parkSending bool + // Model acknowledgments and deferred relabels belong to this conversation; + // its waiting messages cannot pass them while another chat is in front. + modelChange *modelChange + deferredModelRename *modelRename // parkNotes are receipts from held sends and memory commands. They land in // this conversation when it comes forward rather than in whichever unrelated // conversation happened to be on screen at the time. @@ -276,17 +280,19 @@ func (a *app) detachConversation() *aside { // A conversation can be put down while a task's page is in front, and the // box then holds that page's steering line — which is not this // conversation's unsent message and must not come back as one. - draft: a.mainDraftText(), - parks: a.parks, - parkSending: a.parkSending, - chips: main.chips, - pastes: main.pastes, - sends: main.sends, - offset: a.offset, - stick: a.stick, - since: a.now(), - title: a.title, - openingPrompt: a.openingPrompt, + draft: a.mainDraftText(), + parks: a.parks, + parkSending: a.parkSending, + modelChange: a.modelChange, + deferredModelRename: a.deferredModelRename, + chips: main.chips, + pastes: main.pastes, + sends: main.sends, + offset: a.offset, + stick: a.stick, + since: a.now(), + title: a.title, + openingPrompt: a.openingPrompt, } // A SHARED HANDLE CANNOT KEEP A WAITING TURN. Its engine interrupts and // closes the old conversation during the swap, so the only honest fallback @@ -463,6 +469,8 @@ func (a *app) clearConversation() { a.forgetComposers() a.parks = nil a.parkSending = false + a.modelChange = nil + a.deferredModelRename = nil a.touch() } @@ -532,6 +540,15 @@ func (a *app) attachConversation(conv Conversation, side *aside) tea.Cmd { } was := a.agent a.takeUp(conv, true) + a.modelChange = nil + a.deferredModelRename = nil + if side != nil { + a.modelChange, side.modelChange = side.modelChange, nil + a.deferredModelRename, side.deferredModelRename = side.deferredModelRename, nil + if a.modelChange != nil { + a.modelChange.front = a.frontGen + } + } // ANOTHER CONVERSATION'S QUESTIONS DO NOT COME ALONG ([app.forgetQuestions]); // the new lane below replays its own. if a.agent != was { @@ -550,7 +567,6 @@ func (a *app) attachConversation(conv Conversation, side *aside) tea.Cmd { a.resetMeters() if agent != nil { a.model = agent.Model() - a.ensureAvailableModel() a.title = strings.TrimSpace(agent.Title()) // A cached earned name survives an agent whose snapshot is still arriving. if a.title == "" && side != nil { @@ -671,6 +687,7 @@ func (a *app) attachConversation(conv Conversation, side *aside) tea.Cmd { if side != nil { cmds = append(cmds, a.restoreAside(side)) } + a.ensureAvailableModel() // THE IN-FLIGHT TURN'S STREAM IS PUMPED LAST, so the replay above has // already put the person's own message on the screen: the journal holds it // from the moment it was submitted, and the events this stream carries diff --git a/internal/tui3/hostreplay.go b/internal/tui3/hostreplay.go index 5db80bec5e..81729de0b1 100644 --- a/internal/tui3/hostreplay.go +++ b/internal/tui3/hostreplay.go @@ -79,12 +79,13 @@ func (a *app) finishHostedReplay() tea.Cmd { } } if !a.hostReplayLoading { + a.ensureAvailableModel() deferred := a.hostDeferred a.hostDeferred = nil for _, start := range deferred { cmds = append(cmds, start()) } - cmds = append(cmds, a.startFollow()) + cmds = append(cmds, a.startFollow(), a.sendParked()) } return tea.Batch(cmds...) } diff --git a/internal/tui3/keeper.go b/internal/tui3/keeper.go index a6d372b04c..d069f9b399 100644 --- a/internal/tui3/keeper.go +++ b/internal/tui3/keeper.go @@ -826,6 +826,9 @@ func (a *app) sendBehindParked(key string, held *kept) tea.Cmd { if held == nil || held.side == nil || held.side.parkSending || len(held.side.parks) == 0 { return nil } + if held.side.modelChange != nil || held.side.deferredModelRename != nil { + return nil + } held.side.parkSending = true held.side.parks[0].sending = true p := held.side.parks[0] diff --git a/internal/tui3/model_for_now.go b/internal/tui3/model_for_now.go index 2a1c09959f..fb494c8029 100644 --- a/internal/tui3/model_for_now.go +++ b/internal/tui3/model_for_now.go @@ -7,6 +7,8 @@ import ( "github.com/Agent-Field/codeaf/internal/session" ) +const modelChangeRefusedWord = "could not change model · " + // modelChange belongs to the conversation that queued it. Its acknowledgment // may arrive after a later pick or after the surface has switched chats. type modelChange struct { @@ -78,17 +80,31 @@ func (a *app) queueModelChange(id string, window int, temporary, quiet bool, ren if err == nil && save != nil && remember != "" { _ = save(remember) } - return func(here bool) tea.Cmd { + return func(bool) tea.Cmd { if a.modelChange != change { + // A switched conversation keeps its gate in the sidecar. Its + // reply may release only that conversation's waiting message. + for key, held := range a.behind { + if held.side == nil || held.side.modelChange != change { + continue + } + if err != nil { + change.retry = func() { a.queueModelChange(id, window, temporary, quiet, rename) } + held.side.parkNotes = append(held.side.parkNotes, modelChangeRefusedWord+err.Error()) + return nil + } + held.side.modelChange = nil + return a.checkBehindParked(key, agent) + } return nil } - if !here { + if a.agent != agent { a.modelChange = nil return nil } if err != nil { change.retry = func() { a.queueModelChange(id, window, temporary, quiet, rename) } - a.note("could not change model · " + err.Error()) + a.note(modelChangeRefusedWord + err.Error()) return nil } a.modelChange = nil @@ -105,7 +121,7 @@ func (a *app) queueModelChange(id string, window int, temporary, quiet bool, ren } a.hudStale = true a.touch() - return a.takeModelCommands() + return tea.Batch(a.takeModelCommands(), a.sendParked()) } }) a.modelCommands = append(a.modelCommands, cmd) diff --git a/internal/tui3/model_park_test.go b/internal/tui3/model_park_test.go new file mode 100644 index 0000000000..6494e4e4c4 --- /dev/null +++ b/internal/tui3/model_park_test.go @@ -0,0 +1,226 @@ +package tui3 + +import ( + "context" + "errors" + "os" + "path/filepath" + "reflect" + "strings" + "testing" + "time" + + tea "charm.land/bubbletea/v2" + + "github.com/Agent-Field/codeaf/internal/config" + "github.com/Agent-Field/codeaf/internal/modelsource" + "github.com/Agent-Field/codeaf/internal/session" +) + +type parkedModelAgent struct{ *heldModelAgent } + +func (a *parkedModelAgent) SubmitImage(ctx context.Context, text string, images []session.Image) (<-chan session.Event, error) { + a.submitted = append(a.submitted, a.Model()) + return a.fakeAgent.SubmitImage(ctx, text, images) +} + +// C1, C2, C4: stream closure holds parked words, paste bodies and attachments +// through catalog recovery/removal and a pending/refused model acknowledgment. +func TestParkedMessageWaitsForTheAcknowledgedModelAtStreamClose(t *testing.T) { + for _, recovery := range []bool{true, false} { + for _, attachment := range []string{"text", "file", "picture"} { + for _, refused := range []bool{false, true} { + name := map[bool]string{true: "recovery", false: "removal"}[recovery] + "/" + attachment + "/" + map[bool]string{false: "accepted", true: "refused"}[refused] + t.Run(name, func(t *testing.T) { + live, want := "vendor/configured", "vendor/replacement" + if recovery { + live, want = want, live + } + agent := &parkedModelAgent{&heldModelAgent{ + modelForNowAgent: &modelForNowAgent{fakeAgent: &fakeAgent{model: live}, configured: "vendor/configured"}, + entered: make(chan struct{}), release: make(chan struct{}), + }} + defer agent.finish() + if refused { + agent.err = errors.New("connection unavailable") + } + rows := []Model{{ID: live}} + dir := t.TempDir() + if err := config.WriteChatModel(dir, "vendor/configured"); err != nil { + t.Fatal(err) + } + a := newApp(t.Context(), Options{Agent: agent, ProfileDir: dir, RequireListedModel: true, + Sources: modelsource.NewSet(testDefaultService("key")), Models: func() []Model { return rows }, + SaveModel: func(id string) error { return config.WriteChatModel(dir, id) }, + }) + spend(t, a, a.submit("first message")) + a.pastes = []pasteChip{{n: 1, text: "the complete pasted document"}} + if attachment != "text" { + path := filepath.Join(t.TempDir(), "attachment") + if err := os.WriteFile(path, []byte("attachment bytes"), 0600); err != nil { + t.Fatal(err) + } + a.chips = []chip{{path: path, file: attachment == "file"}} + } + a.park("waiting "+pasteToken(1, 1), []segment{{from: 0, to: 7}}) + waiting := a.parks[0] + // A newer draft's tray must survive the older parked send too. + a.chips = []chip{{path: "draft attachment", file: true}} + draftChips := append([]chip(nil), a.chips...) + rows = []Model{{ID: want}} + a.serviceModelsLanded(modelsource.DefaultID, config.DefaultBaseURL) + agent.finish() + _, cmd := a.Update(streamClosedMsg{gen: a.gen}) + select { + case <-agent.entered: + case <-time.After(time.Second): + close(agent.release) + t.Fatal("the turn-end model door never started") + } + // The direct attachment door must preserve the newer tray at + // the same gate, even when called outside the parked road. + if attachment != "text" { + cmd = tea.Batch(cmd, a.submitImagesShown("draft", "draft", nil)) + } + if len(agent.sent) != 1 || len(agent.imageText) != 0 || !reflect.DeepEqual(a.parks, []parked{waiting}) || !reflect.DeepEqual(a.chips, draftChips) { + close(agent.release) + t.Fatalf("pending selection spent a parked message: sent=%q parks=%+v tray=%+v", agent.sent, a.parks, a.chips) + } + close(agent.release) + spend(t, a, cmd) + if refused { + if len(agent.sent) != 1 || len(agent.imageText) != 0 || !reflect.DeepEqual(a.parks, []parked{waiting}) || !a.modelChangePending() { + t.Fatal("refused selection spent the parked words or attachments") + } + if got := lastNote(t, a); got != "could not change model · connection unavailable" { + t.Fatalf("refusal=%q", got) + } + agent.err = nil + if a.ensureAvailableModel() { + t.Fatal("retry opened the gate before acknowledgment") + } + acknowledgeModelChanges(t, a) + } + if len(agent.sent) != 2 || !strings.Contains(agent.sent[1], waiting.spoken()) || len(agent.submitted) != 2 || agent.submitted[1] != want || len(a.parks) != 0 { + t.Fatalf("acknowledged send=%q models=%q parks=%+v", agent.sent, agent.submitted, a.parks) + } + if attachment == "picture" && (len(agent.images) != 1 || len(agent.images[0]) != 1 || string(agent.images[0][0].Bytes) != "attachment bytes") { + t.Fatalf("parked picture=%+v", agent.images) + } + if attachment == "file" && !strings.Contains(agent.sent[1], waiting.chips[0].path) { + t.Fatal("parked file did not travel with its words") + } + if !reflect.DeepEqual(a.chips, draftChips) || agent.ConfiguredModel() != "vendor/configured" || config.ChatModelAt(dir) != "vendor/configured" { + t.Fatal("parked send spent the draft tray or saved the replacement") + } + acknowledgeModelChanges(t, a) + if len(agent.sent) != 2 { + t.Fatal("the acknowledgment sent the parked message twice") + } + }) + } + } + } +} + +type parkedSwitchModelAgent struct{ *parkedModelAgent } + +func (a *parkedSwitchModelAgent) Attach() (<-chan session.Event, bool, func()) { + return nil, false, func() {} +} + +// C1, C4, C5: a model acknowledgment after a real detach/attach releases only +// its owner's parked send; a refusal keeps that owner's complete queue held. +func TestParkedModelAcknowledgmentBelongsToTheConversationThatQueuedIt(t *testing.T) { + for _, refused := range []bool{false, true} { + t.Run(map[bool]string{false: "accepted", true: "refused"}[refused], func(t *testing.T) { + agent := &parkedSwitchModelAgent{&parkedModelAgent{&heldModelAgent{ + modelForNowAgent: &modelForNowAgent{fakeAgent: &fakeAgent{model: "vendor/replacement"}, configured: "vendor/configured"}, + entered: make(chan struct{}), release: make(chan struct{}), + }}} + defer agent.finish() + if refused { + agent.err = errors.New("connection unavailable") + } + rows := []Model{{ID: "vendor/replacement"}, {ID: "vendor/other"}} + a := newApp(t.Context(), Options{Agent: agent, ProfileDir: t.TempDir(), RequireListedModel: true, + Sources: modelsource.NewSet(testDefaultService("key")), Models: func() []Model { return rows }, + }) + a.file = filepath.Join(t.TempDir(), "held.jsonl") + spend(t, a, a.submit("first message")) + a.park("the owning conversation's message", nil) + rows = append(rows, Model{ID: "vendor/configured"}) + agent.finish() + _, pending := a.Update(streamClosedMsg{gen: a.gen}) + select { + case <-agent.entered: + case <-time.After(time.Second): + close(agent.release) + t.Fatal("the model door never started") + } + conv, side := a.front(), a.detachConversation() + stowed := a.stow(conv, side) + key := a.convKey(conv.SessionFile) + held := a.behind[key] + t.Cleanup(held.watch.stop) + front := &modelForNowAgent{fakeAgent: &fakeAgent{model: "vendor/other"}, configured: "vendor/other"} + attached := a.attachConversation(Conversation{Agent: front, SessionFile: filepath.Join(t.TempDir(), "front.jsonl")}, nil) + a.parks = []parked{{text: "the front conversation's message"}} + if a.sendBehindParked(key, held) != nil || len(side.parks) != 1 || len(agent.sent) != 1 { + close(agent.release) + t.Fatal("switching chats bypassed the pending model acknowledgment") + } + close(agent.release) + spend(t, a, tea.Batch(pending, stowed, attached)) + if len(front.sent) != 0 || len(a.parks) != 1 || front.ConfiguredModel() != "vendor/other" { + t.Fatal("the old acknowledgment released or changed the front conversation") + } + if refused { + if len(agent.sent) != 1 || len(side.parks) != 1 || a.sendBehindParked(key, held) != nil { + t.Fatal("refusal released the hidden conversation's queue") + } + agent.err = nil + held.watch.stop() + delete(a.behind, key) + a.detachConversation() + spend(t, a, a.attachConversation(conv, side)) + acknowledgeModelChanges(t, a) + } + if len(agent.sent) != 2 || agent.sent[1] != "the owning conversation's message" || len(agent.submitted) != 2 || agent.submitted[1] != "vendor/configured" { + t.Fatalf("owning send=%q models=%q", agent.sent, agent.submitted) + } + }) + } +} + +// C1, C4: coming back before the original reply keeps its model gate and +// resumes the parked send once, despite the intervening front generations. +func TestReturningBeforeModelAcknowledgmentResumesTheParkedSend(t *testing.T) { + agent := &parkedModelAgent{&heldModelAgent{ + modelForNowAgent: &modelForNowAgent{fakeAgent: &fakeAgent{model: "vendor/replacement"}, configured: "vendor/configured"}, + entered: make(chan struct{}), release: make(chan struct{}), + }} + defer agent.finish() + rows := []Model{{ID: "vendor/replacement"}, {ID: "vendor/other"}} + a := newApp(t.Context(), Options{Agent: agent, ProfileDir: t.TempDir(), RequireListedModel: true, + Sources: modelsource.NewSet(testDefaultService("key")), Models: func() []Model { return rows }, + }) + spend(t, a, a.submit("first message")) + a.park("the returning conversation's message", nil) + rows = append(rows, Model{ID: "vendor/configured"}) + agent.finish() + _, pending := a.Update(streamClosedMsg{gen: a.gen}) + conv, side := a.front(), a.detachConversation() + away := a.attachConversation(Conversation{Agent: &fakeAgent{model: "vendor/other"}}, nil) + a.detachConversation() + back := a.attachConversation(conv, side) + if !a.modelChangePending() || len(a.parks) != 1 || len(agent.sent) != 1 { + close(agent.release) + t.Fatal("returning lost the pending model gate or its waiting message") + } + close(agent.release) + spend(t, a, tea.Batch(pending, away, back)) + if a.modelChangePending() || len(agent.sent) != 2 || agent.sent[1] != "the returning conversation's message" || len(agent.submitted) != 2 || agent.submitted[1] != "vendor/configured" || len(a.parks) != 0 { + t.Fatalf("returning acknowledgment: sent=%q models=%q parks=%+v", agent.sent, agent.submitted, a.parks) + } +} diff --git a/internal/tui3/model_rename.go b/internal/tui3/model_rename.go index 5d873bdba7..be44f4c484 100644 --- a/internal/tui3/model_rename.go +++ b/internal/tui3/model_rename.go @@ -6,11 +6,15 @@ import ( ) // A label change carries saved and temporary selections independently. A -// working turn keeps both until settlement, alongside any explicit next pick. +// working turn keeps both until settlement unless a later choice supersedes it. type modelRename struct { - save string - live string - pair *session.ConversationModels + owner Agent + configured string + running string + models *session.ConversationModels + save string + live string + pair *session.ConversationModels } func (a *app) reprefixTemporaryModel(oldWritten, newWritten string) bool { @@ -24,7 +28,12 @@ func (a *app) reprefixTemporaryModel(oldWritten, newWritten string) bool { pair = © } } + if deferred := a.deferredModelRename; deferred != nil && !a.modelRenameCurrent(deferred) { + a.deferredModelRename = nil + } + owner, original, running, models := a.agent, configured, a.agent.Model(), pair if deferred := a.deferredModelRename; deferred != nil { + owner, original, running, models = deferred.owner, deferred.configured, deferred.running, deferred.models saved, changedPair = deferred.save, deferred.pair if deferred.save != "" { configured = deferred.save @@ -38,7 +47,7 @@ func (a *app) reprefixTemporaryModel(oldWritten, newWritten string) bool { return false } next := config.ReprefixModelID(configured, oldWritten, newWritten) - rename := &modelRename{save: saved, pair: changedPair, live: config.ReprefixModelID(live, oldWritten, newWritten)} + rename := &modelRename{owner: owner, configured: original, running: running, models: models, save: saved, pair: changedPair, live: config.ReprefixModelID(live, oldWritten, newWritten)} if next != configured { rename.save = next } @@ -63,9 +72,28 @@ func (a *app) reprefixTemporaryModel(oldWritten, newWritten string) bool { } func (a *app) applyModelRename(rename *modelRename) { + if !a.modelRenameCurrent(rename) { + return + } configured := a.configuredModel() if rename.save != "" { configured = rename.save } a.queueModelChange(rename.live, 0, rename.live != configured, true, rename) } + +// An explicit choice made after the label change owns the saved selection, +// including a pair whose main model stayed the same but whose fallback changed. +func (a *app) modelRenameCurrent(rename *modelRename) bool { + if rename.owner != a.agent || rename.configured != a.configuredModel() || rename.running != a.agent.Model() { + return false + } + var pair *session.ConversationModels + if door, ok := a.agent.(interface{ ModelIdentity() session.ModelIdentity }); ok { + pair = door.ModelIdentity().Models + } + if pair == nil || rename.models == nil { + return pair == nil && rename.models == nil + } + return *pair == *rename.models +} diff --git a/internal/tui3/model_rename_owner_test.go b/internal/tui3/model_rename_owner_test.go new file mode 100644 index 0000000000..f9239469cf --- /dev/null +++ b/internal/tui3/model_rename_owner_test.go @@ -0,0 +1,121 @@ +package tui3 + +import ( + "testing" + + "github.com/Agent-Field/codeaf/internal/config" + "github.com/Agent-Field/codeaf/internal/modelsource" + "github.com/Agent-Field/codeaf/internal/session" +) + +func deferredRenameApp(t *testing.T, paired bool) (*app, *pairedModelForNowAgent) { + t.Helper() + dir := t.TempDir() + customs := connectionWriteSources(t, dir, "mybox") + agent := &pairedModelForNowAgent{ + modelForNowAgent: &modelForNowAgent{fakeAgent: &fakeAgent{model: "mybox/chosen"}, configured: "mybox/chosen"}, + } + var door Agent = agent.modelForNowAgent + if paired { + agent.pair = session.ConversationModels{Main: "mybox/chosen", Fallback: "mybox/backup"} + door = agent + } + a := modelServiceTestAppWithAgent(t, dir, agent.configured, + modelsource.NewSet(append([]modelsource.Connected{connectionDefaultService()}, customs...)...), + []Model{{ID: "vendor/replacement"}, {ID: "vendor/other"}, {ID: "vendor/explicit"}}, door) + a.modelsForService = func(modelsource.Connected) []Model { return []Model{{ID: "replacement"}} } + if err := config.WriteChatModel(dir, agent.configured); err != nil { + t.Fatal(err) + } + a.useModelForNow("vendor/replacement", 0) + acknowledgeModelChanges(t, a) + a.requireListedModel = true + a.state = stateWorking + rows := config.PersistedSources(dir) + rows[0].Written = "lab" + if err := config.WriteSources(dir, rows); err != nil { + t.Fatal(err) + } + a.adoptModelConnectResult(modelConnectResultMsg{service: customs[0].Source.ID, written: "lab", renamedFrom: "mybox", + outcome: modelsource.Outcome{Kind: modelsource.OutcomeConnected}, models: []Model{{ID: "replacement"}}, + }) + if a.deferredModelRename == nil || agent.ConfiguredModel() != "mybox/chosen" { + t.Fatal("rename did not wait for its answering conversation") + } + return a, agent +} + +// C1, C2, C4, C5: switching chats before settlement keeps the rename with its +// owner. The unrelated chat settles without adopting either captured model. +func TestDeferredRenameStaysWithItsConversationAcrossASwitch(t *testing.T) { + for _, paired := range []bool{false, true} { + t.Run(map[bool]string{false: "single", true: "pair"}[paired], func(t *testing.T) { + a, original := deferredRenameApp(t, paired) + a.park("waiting for the renamed conversation", nil) + conv, side := a.front(), a.detachConversation() + if a.sendBehindParked("owner", &kept{conv: conv, side: side}) != nil || len(side.parks) != 1 { + t.Fatal("the hidden parked send bypassed its deferred rename") + } + other := &modelForNowAgent{fakeAgent: &fakeAgent{model: "vendor/other"}, configured: "vendor/other"} + spend(t, a, a.attachConversation(Conversation{Agent: other}, nil)) + a.switchModel("vendor/other", 0) + a.state = stateWorking + drive(t, a, streamClosedMsg{gen: a.gen}) + if other.ConfiguredModel() != "vendor/other" || other.Model() != "vendor/other" || config.ChatModelAt(a.profileDir) != "vendor/other" || len(other.temporary) != 0 { + t.Fatal("another conversation's settlement adopted the deferred rename") + } + if original.ConfiguredModel() != "mybox/chosen" { + t.Fatal("the original conversation changed before returning to it") + } + a.detachConversation() + spend(t, a, a.attachConversation(conv, side)) + acknowledgeModelChanges(t, a) + if original.ConfiguredModel() != "lab/chosen" || original.Model() != "vendor/replacement" || other.ConfiguredModel() != "vendor/other" || config.ChatModelAt(a.profileDir) == "vendor/replacement" { + t.Fatal("the owning conversation lost its rename or saved its replacement") + } + if paired && original.pair != (session.ConversationModels{Main: "lab/chosen", Fallback: "lab/backup"}) { + t.Fatalf("the owner's renamed pair=%+v", original.pair) + } + if len(original.sent) != 1 || original.sent[0] != "waiting for the renamed conversation" || len(a.parks) != 0 { + t.Fatal("the owner's parked send did not resume after the rename acknowledgment") + } + original.finish() + }) + } +} + +// C2, C5: a later explicit single or pair choice retires the captured rename, +// including a pair whose main model is unchanged and only its fallback changes. +func TestLaterExplicitChoiceRetiresTheDeferredRename(t *testing.T) { + for _, pick := range []string{"single", "pair", "pair fallback", "same pair"} { + t.Run(pick, func(t *testing.T) { + a, agent := deferredRenameApp(t, pick != "single") + want := "vendor/explicit" + var pair session.ConversationModels + if pick == "single" { + a.slash("/model " + want) + } else { + pair = session.ConversationModels{Main: want, Fallback: "vendor/backup"} + if pick == "pair fallback" { + want, pair.Main = "mybox/chosen", "mybox/chosen" + } + if pick == "same pair" { + pair = agent.pair + want = pair.Main + } + if _, err := agent.SetConversationModels(pair); err != nil { + t.Fatal(err) + } + a.modelSwitched(want, 0, true) + } + acknowledgeModelChanges(t, a) + drive(t, a, streamClosedMsg{gen: a.gen}) + if agent.ConfiguredModel() != want || config.ChatModelAt(a.profileDir) != want || a.deferredModelRename != nil { + t.Fatalf("settlement replaced the explicit choice: configured=%q saved=%q", agent.ConfiguredModel(), config.ChatModelAt(a.profileDir)) + } + if pick != "single" && agent.pair != pair { + t.Fatalf("settlement replaced the explicit pair: %+v", agent.pair) + } + }) + } +} diff --git a/internal/tui3/modeldefault.go b/internal/tui3/modeldefault.go index 9020b97192..50679ee22a 100644 --- a/internal/tui3/modeldefault.go +++ b/internal/tui3/modeldefault.go @@ -41,6 +41,15 @@ func (a *app) isAvailableModel(id string) bool { // choose. With no rows, only the display is cleared: the engine has no unset-model // door, and the send gates below keep that old launch preference from being used. func (a *app) ensureAvailableModel() bool { + if rename := a.deferredModelRename; rename != nil { + // A held rename waits for its owner's turn/replay boundary before + // touching the saved selection. Hidden parked sends remain held too. + if a.state == stateWorking || a.hostReplayLoading { + return false + } + a.deferredModelRename = nil + a.applyModelRename(rename) + } if a.modelChangePending() { if retry := a.modelChange.retry; retry != nil { retry() diff --git a/internal/tui3/palette.go b/internal/tui3/palette.go index c3feb377cc..3300784c71 100644 --- a/internal/tui3/palette.go +++ b/internal/tui3/palette.go @@ -3106,6 +3106,7 @@ func (a *app) windowFor(id string) int { // fires at a fraction of it, so a session that switched to a 1M model without // saying so would keep compacting as if it were on the 128k one it started on. func (a *app) switchModel(id string, window int) { + a.deferredModelRename = nil if a.modelChange != nil { a.queueModelChange(id, window, false, a.creditSwitching, nil) return diff --git a/internal/tui3/park.go b/internal/tui3/park.go index 5e8bbf20d3..9d6c845221 100644 --- a/internal/tui3/park.go +++ b/internal/tui3/park.go @@ -117,9 +117,14 @@ func (a *app) sendParked() tea.Cmd { // A pending take-back can leave the surface between pumps while the // session's next turn is already running. Its queue settles first; parked // words must not open a second turn in that gap. - if a.stream != nil || len(a.follows) > 0 || a.parkSending || len(a.parks) == 0 { + if a.state == stateWorking || a.stream != nil || len(a.follows) > 0 || a.parkSending || len(a.parks) == 0 || a.hostReplayLoading || a.hostReplayWaiting { return nil } + // Keep the whole message until the model door acknowledges the next + // request's selection, including pictures that use a separate submit door. + if !a.ensureAvailableModel() { + return a.takeModelCommands() + } next := a.parks[0] a.parks = a.parks[1:] shown := next.text From 33498156893633d29a0870314f7f799559668df4 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 6 Nov 2026 10:35:08 -0500 Subject: [PATCH 5/6] Models: a send waits for its chat's pending model switch instead of the window holding it Review round 3: holding sends at the surface while a run-only switch was unacknowledged kept missing send roads (a send deferred behind a replay, a hidden chat's queued send). The surface no longer holds anything: it shows the replacement at once, and every engine call that starts a turn (a plain send, an attachment send, a follow-up, an errand send) waits inside its own command for that agent's switch still on the ordered door line, so the engine always sees the switch before the message. A refused switch still lets the message go; the next reconcile retries. A hidden chat reconciles when it comes back to the front, as before. Existing tests keep only the acknowledgment steps the asynchronous switch still needs. Co-Authored-By: Claude Opus 5.5 --- internal/manual/chat/models-and-cost.md | 9 +- internal/tui3/app.go | 20 +- internal/tui3/attach.go | 10 +- internal/tui3/detach.go | 11 +- internal/tui3/followup.go | 4 + internal/tui3/homeexchange.go | 3 + internal/tui3/hostreplay.go | 6 +- internal/tui3/input.go | 3 - internal/tui3/keeper.go | 3 - internal/tui3/model_change_test.go | 316 +++++++++++++++------ internal/tui3/model_for_now.go | 87 ++---- internal/tui3/model_order.go | 44 +++ internal/tui3/model_park_test.go | 334 +++++++++++------------ internal/tui3/model_rename_owner_test.go | 33 ++- internal/tui3/modeldefault.go | 35 ++- internal/tui3/modeldefault_test.go | 2 +- internal/tui3/palette.go | 21 +- internal/tui3/park.go | 7 +- internal/tui3/startchat.go | 3 + 19 files changed, 557 insertions(+), 394 deletions(-) create mode 100644 internal/tui3/model_order.go diff --git a/internal/manual/chat/models-and-cost.md b/internal/manual/chat/models-and-cost.md index 5a931e3a91..fdac87d515 100644 --- a/internal/manual/chat/models-and-cost.md +++ b/internal/manual/chat/models-and-cost.md @@ -124,6 +124,7 @@ model. With several providers, the default still belongs to their combined list. Availability replacements apply only to this run. They do not overwrite the conversation's saved model, main/fallback pair or profile model settings. When the configured model is listed again, codeaf returns to it automatically. +A hidden chat reconciles availability when it next comes to the front. Choosing a model yourself, including the replacement already running, saves that choice as usual. @@ -131,10 +132,10 @@ While every catalog is cold or empty, no default model is displayed and a messag stays in the draft. The picker opens with: `no available model · connect a provider or refresh /model`. When a provider's list arrives, codeaf selects an available model. Press Enter again -to send the draft after the model selection succeeds. A failed selection -reports `could not change model · ` and leaves the draft unsent. -Waiting messages keep their text and attachments until selection succeeds, then -send once in their own conversation. +to send the draft. The replacement is displayed immediately, and each send waits +for that chat's model switch to finish. A failed selection reports +`could not change model · `; the message still sends once on the engine's +current model, and the next availability check retries the selection. Refreshing a list or removing a connection also replaces a model that is no longer listed; existing requests finish on the model they started with. diff --git a/internal/tui3/app.go b/internal/tui3/app.go index b92e5d6b90..9bd16e29e0 100644 --- a/internal/tui3/app.go +++ b/internal/tui3/app.go @@ -1968,8 +1968,10 @@ type app struct { // just replaced, so the one pending id lands at the turn boundary. deferredModelServiceModel string deferredModelRename *modelRename - modelChange *modelChange - modelCommands []tea.Cmd + // A later explicit choice keeps its wire and preference-write order even + // across a chat switch. Sends wait by agent inside their commands instead. + pendingModelSelection <-chan struct{} + modelCommands []tea.Cmd // harn is the subharness registry (Options.Harnesses) and harnPanel the // list /harness opens over it (harnesspanel.go). A nil harn is a surface // that cannot show harnesses and says so; nothing about the OFFER depends on @@ -7475,7 +7477,12 @@ func (a *app) submitShown(text, shown string, plain []segment) tea.Cmd { // counts as an accepted turn, and keeps the update-loop door budget from growing // merely because a conversation can now send while it is behind the screen. func submitStart(agent Agent, ctx context.Context, text string) func() (<-chan session.Event, error) { - return func() (<-chan session.Event, error) { return agent.Submit(ctx, text) } + return func() (<-chan session.Event, error) { + if err := waitModelSelection(ctx, agent); err != nil { + return nil, err + } + return agent.Submit(ctx, text) + } } // submitting is that body with the CALL left to the caller: everything a @@ -7497,9 +7504,6 @@ func (a *app) submitting(text string, start func() (<-chan session.Event, error) // shown, so the transcript leaves them plain (entry.plainTags). func (a *app) submittingShown(text, shown string, plain []segment, start func() (<-chan session.Event, error)) tea.Cmd { if !a.ensureAvailableModel() { - if a.modelChangePending() { - return a.takeModelCommands() - } a.note(noAvailableModelWord) return nil } @@ -7550,7 +7554,11 @@ func (a *app) submittingShown(text, shown string, plain []segment, start func() a.startClock() a.follow() a.touch() + ctx, agent := a.ctx, a.agent return tea.Batch(func() tea.Msg { + if err := waitModelSelection(ctx, agent); err != nil { + return submittedMsg{err: err, echo: mark, call: call} + } ch, err := start() return submittedMsg{ch: ch, err: err, echo: mark, call: call} }, a.wake()) diff --git a/internal/tui3/attach.go b/internal/tui3/attach.go index 052d845639..d7124ac0c3 100644 --- a/internal/tui3/attach.go +++ b/internal/tui3/attach.go @@ -916,13 +916,6 @@ func (a *app) submitImages(text string) tea.Cmd { } func (a *app) submitImagesShown(text, shown string, plain []segment) tea.Cmd { - if !a.ensureAvailableModel() { - if a.modelChangePending() { - return a.takeModelCommands() - } - a.note(noAvailableModelWord) - return nil - } agent, ctx := a.agent, a.ctx chips := append([]chip(nil), a.chips...) a.chips, a.sent = nil, chips @@ -1005,6 +998,9 @@ func attachmentStart(agent Agent, ctx context.Context, hosted bool, text, shown } display = shown start = func() (<-chan session.Event, error) { + if err := waitModelSelection(ctx, agent); err != nil { + return nil, err + } images, err := readAttachments(pictures) if err != nil { return nil, err diff --git a/internal/tui3/detach.go b/internal/tui3/detach.go index 8fa81a5946..5674f52517 100644 --- a/internal/tui3/detach.go +++ b/internal/tui3/detach.go @@ -63,9 +63,7 @@ type aside struct { // surface yet. It follows the sidecar so returning mid-send cannot send the // same message again when the stream closes. parkSending bool - // Model acknowledgments and deferred relabels belong to this conversation; - // its waiting messages cannot pass them while another chat is in front. - modelChange *modelChange + // A deferred relabel belongs to this conversation across chat switches. deferredModelRename *modelRename // parkNotes are receipts from held sends and memory commands. They land in // this conversation when it comes forward rather than in whichever unrelated @@ -283,7 +281,6 @@ func (a *app) detachConversation() *aside { draft: a.mainDraftText(), parks: a.parks, parkSending: a.parkSending, - modelChange: a.modelChange, deferredModelRename: a.deferredModelRename, chips: main.chips, pastes: main.pastes, @@ -469,7 +466,6 @@ func (a *app) clearConversation() { a.forgetComposers() a.parks = nil a.parkSending = false - a.modelChange = nil a.deferredModelRename = nil a.touch() } @@ -540,14 +536,9 @@ func (a *app) attachConversation(conv Conversation, side *aside) tea.Cmd { } was := a.agent a.takeUp(conv, true) - a.modelChange = nil a.deferredModelRename = nil if side != nil { - a.modelChange, side.modelChange = side.modelChange, nil a.deferredModelRename, side.deferredModelRename = side.deferredModelRename, nil - if a.modelChange != nil { - a.modelChange.front = a.frontGen - } } // ANOTHER CONVERSATION'S QUESTIONS DO NOT COME ALONG ([app.forgetQuestions]); // the new lane below replays its own. diff --git a/internal/tui3/followup.go b/internal/tui3/followup.go index 9c25a6d4fd..791debdc0f 100644 --- a/internal/tui3/followup.go +++ b/internal/tui3/followup.go @@ -141,7 +141,11 @@ func (a *app) sendFollowFrom(agent Agent, spoken, line string, pastes []pasteChi return nil } call := a.hostCallStarted() + ctx := a.ctx return func() tea.Msg { + if err := waitModelSelection(ctx, agent); err != nil { + return followMsg{text: line, pastes: pastes, demoted: demoted, err: err, call: call} + } ch, err := agent.FollowUp(spoken) return followMsg{text: line, pastes: pastes, demoted: demoted, ch: ch, err: err, call: call} } diff --git a/internal/tui3/homeexchange.go b/internal/tui3/homeexchange.go index 55ed2effa0..a0696fdde1 100644 --- a/internal/tui3/homeexchange.go +++ b/internal/tui3/homeexchange.go @@ -1287,6 +1287,9 @@ func errandSend(ex *homeExchange, text string) tea.Cmd { chips := ex.chips ex.chips = nil return func() tea.Msg { + if err := waitModelSelection(errandContext(), ex.agent); err != nil { + return errandStartedMsg{ex: ex, err: err} + } if len(chips) == 0 { ch, err := ex.agent.Submit(errandContext(), text) return errandStartedMsg{ex: ex, ch: ch, err: err} diff --git a/internal/tui3/hostreplay.go b/internal/tui3/hostreplay.go index 81729de0b1..1b900958b5 100644 --- a/internal/tui3/hostreplay.go +++ b/internal/tui3/hostreplay.go @@ -79,13 +79,15 @@ func (a *app) finishHostedReplay() tea.Cmd { } } if !a.hostReplayLoading { - a.ensureAvailableModel() + if a.deferredModelRename != nil { + a.ensureAvailableModel() + } deferred := a.hostDeferred a.hostDeferred = nil for _, start := range deferred { cmds = append(cmds, start()) } - cmds = append(cmds, a.startFollow(), a.sendParked()) + cmds = append(cmds, a.startFollow()) } return tea.Batch(cmds...) } diff --git a/internal/tui3/input.go b/internal/tui3/input.go index fdbdb1c563..d1ff056033 100644 --- a/internal/tui3/input.go +++ b/internal/tui3/input.go @@ -1575,9 +1575,6 @@ func (a *app) enterCheckedLine(ownershipRead bool) tea.Cmd { // The surface can open while catalogs warm, but a guessed launch model // must never receive the message in the meantime. if !strings.HasPrefix(line, "/") && (line != "" || held) && !a.ensureAvailableModel() { - if a.modelChangePending() { - return a.takeModelCommands() - } a.note(noAvailableModelWord) a.openPicker() return nil diff --git a/internal/tui3/keeper.go b/internal/tui3/keeper.go index d069f9b399..a6d372b04c 100644 --- a/internal/tui3/keeper.go +++ b/internal/tui3/keeper.go @@ -826,9 +826,6 @@ func (a *app) sendBehindParked(key string, held *kept) tea.Cmd { if held == nil || held.side == nil || held.side.parkSending || len(held.side.parks) == 0 { return nil } - if held.side.modelChange != nil || held.side.deferredModelRename != nil { - return nil - } held.side.parkSending = true held.side.parks[0].sending = true p := held.side.parks[0] diff --git a/internal/tui3/model_change_test.go b/internal/tui3/model_change_test.go index 2f690fd2b0..49fdf2a602 100644 --- a/internal/tui3/model_change_test.go +++ b/internal/tui3/model_change_test.go @@ -1,11 +1,13 @@ package tui3 import ( + "context" "errors" "sync" "testing" "time" + tea "charm.land/bubbletea/v2" "github.com/Agent-Field/codeaf/internal/config" "github.com/Agent-Field/codeaf/internal/modelsource" "github.com/Agent-Field/codeaf/internal/session" @@ -13,10 +15,11 @@ import ( type heldModelAgent struct { *modelForNowAgent - entered chan struct{} - release chan struct{} - once sync.Once - err error + entered chan struct{} + release chan struct{} + once sync.Once + err error + atSubmit chan string } func (a *heldModelAgent) UseModelForNow(id string) error { @@ -32,100 +35,214 @@ func (a *heldModelAgent) UseModelForNow(id string) error { return a.modelForNowAgent.UseModelForNow(id) } -// C1, C4, C5: catalog selection returns to the loop while its door waits; -// the draft and alternate send gates stay closed until a successful reply. -func TestRunOnlySelectionWaitsForAcknowledgmentOffTheLoop(t *testing.T) { - for _, refused := range []bool{false, true} { - t.Run(map[bool]string{false: "accepted", true: "refused"}[refused], func(t *testing.T) { - agent := &heldModelAgent{ - modelForNowAgent: &modelForNowAgent{fakeAgent: &fakeAgent{model: "vendor/configured"}, configured: "vendor/configured"}, - entered: make(chan struct{}), release: make(chan struct{}), - } - if refused { - agent.err = errors.New("connection unavailable") - } - a := newApp(t.Context(), Options{Agent: agent, ProfileDir: t.TempDir(), Sources: modelsource.NewSet(testDefaultService("key")), - Models: func() []Model { return []Model{{ID: "vendor/replacement"}} }, - }) - a.requireListedModel = true - a.input.insert("keep this draft") - if a.ensureAvailableModel() || a.model != "vendor/configured" { - t.Fatal("unacknowledged selection opened the send gate") - } - select { - case <-agent.entered: - case <-time.After(time.Second): - close(agent.release) - t.Fatal("model door never started") - } - called := false - cmd := a.submitting("blocked", func() (<-chan session.Event, error) { called = true; return nil, nil }) - if called || a.input.String() != "keep this draft" || a.state == stateWorking { - t.Fatal("pending selection submitted or consumed the draft") - } - close(agent.release) - spend(t, a, cmd) - if refused { - if a.model != "vendor/configured" || agent.ConfiguredModel() != "vendor/configured" { - t.Fatal("refused selection changed the model") - } - if got := lastNote(t, a); got != "could not change model · connection unavailable" { - t.Fatalf("refusal=%q", got) - } - if a.ensureAvailableModel() { - t.Fatal("failed model selection opened the send gate") - } - acknowledgeModelChanges(t, a) - return - } - if !a.ensureAvailableModel() || a.model != "vendor/replacement" || agent.ConfiguredModel() != "vendor/configured" { - t.Fatal("successful selection did not resume reconciliation") - } - spend(t, a, a.enterLine()) - if len(agent.submitted) != 1 || agent.submitted[0] != "vendor/replacement" { - t.Fatalf("submitted=%v", agent.submitted) - } - }) - } +func (a *heldModelAgent) Submit(ctx context.Context, text string) (<-chan session.Event, error) { + a.atSubmit <- a.Model() + return a.modelForNowAgent.Submit(ctx, text) } -// C5: an explicit pick queued behind a temporary selection wins in wire order -// and keeps its ordinary profile write and model receipt. -func TestExplicitPickSupersedesAnUnacknowledgedRunOnlySelection(t *testing.T) { +func (a *heldModelAgent) FollowUp(text string) (<-chan session.Event, error) { + return a.Submit(context.Background(), text) +} + +func modelOrderingApp(t *testing.T, refused bool) (*app, *heldModelAgent, func()) { + t.Helper() agent := &heldModelAgent{ modelForNowAgent: &modelForNowAgent{fakeAgent: &fakeAgent{model: "vendor/configured"}, configured: "vendor/configured"}, - entered: make(chan struct{}), release: make(chan struct{}), + entered: make(chan struct{}), release: make(chan struct{}), atSubmit: make(chan string, 8), } + if refused { + agent.err = errors.New("connection unavailable") + } + unblock := sync.OnceFunc(func() { close(agent.release) }) + t.Cleanup(unblock) + t.Cleanup(agent.finish) dir := t.TempDir() - a := newApp(t.Context(), Options{Agent: agent, ProfileDir: dir, SaveModel: func(id string) error { return config.WriteChatModel(dir, id) }}) - a.useModelForNow("vendor/replacement", 0) - a.switchModel("vendor/configured", 0) - close(agent.release) + a := newApp(t.Context(), Options{Agent: agent, ProfileDir: dir, + Sources: modelsource.NewSet(testDefaultService("key")), + Models: func() []Model { return []Model{{ID: "vendor/replacement"}, {ID: "vendor/chosen"}} }, + SaveModel: func(id string) error { return config.WriteChatModel(dir, id) }, + }) + a.requireListedModel = true + return a, agent, unblock +} + +func beginOrderedSend(t *testing.T, agent *heldModelAgent, cmd tea.Cmd) <-chan []tea.Msg { + t.Helper() + select { + case <-agent.entered: + case <-time.After(time.Second): + t.Fatal("model door never started") + } + done := make(chan []tea.Msg, 1) + go func() { done <- runCmd(cmd) }() + select { + case model := <-agent.atSubmit: + t.Fatalf("Submit passed the unfinished model door on %q", model) + case <-done: + t.Fatal("send command finished before the model door") + case <-time.After(30 * time.Millisecond): + } + return done +} + +func finishOrderedSend(t *testing.T, a *app, done <-chan []tea.Msg) { + t.Helper() + select { + case msgs := <-done: + drive(t, a, msgs...) + case <-time.After(time.Second): + t.Fatal("send did not resume after the model door") + } +} + +// C1, C4: display changes immediately; only the command waits for selection, +// without requiring its UI acknowledgment. +func TestRunOnlySelectionOrdersTheSendOffTheLoop(t *testing.T) { + a, agent, unblock := modelOrderingApp(t, false) + if !a.ensureAvailableModel() || a.model != "vendor/replacement" { + t.Fatal("selection did not display immediately") + } + a.input.insert("send this draft") + done := beginOrderedSend(t, agent, a.enterLine()) + if a.input.String() != "" || a.state != stateWorking { + t.Fatal("surface held the send") + } + unblock() + finishOrderedSend(t, a, done) acknowledgeModelChanges(t, a) - if agent.Model() != "vendor/configured" || agent.ConfiguredModel() != "vendor/configured" || config.ChatModelAt(dir) != "vendor/configured" { - t.Fatal("the temporary selection displaced the explicit pick") + if len(agent.sent) != 1 || agent.sent[0] != "send this draft" || agent.submitted[0] != "vendor/replacement" || agent.ConfiguredModel() != "vendor/configured" { + t.Fatalf("sent=%q models=%q", agent.sent, agent.submitted) } - if got := noteSaying(t, a, "model ·"); got != "model · vendor/configured" { - t.Fatalf("explicit receipt=%q", got) +} + +// C4: a newer catalog can restore the configured model behind a pending +// replacement, and the next send waits for both asks without a UI fold. +func TestCatalogRecoverySupersedesAPendingReplacement(t *testing.T) { + a, agent, unblock := modelOrderingApp(t, false) + a.ensureAvailableModel() + a.models = func() []Model { return []Model{{ID: "vendor/configured"}} } + a.serviceModelsLanded(modelsource.DefaultID, config.DefaultBaseURL) + if a.model != "vendor/configured" { + t.Fatal("recovery did not display immediately") + } + done := beginOrderedSend(t, agent, a.submit("use the recovered model")) + unblock() + finishOrderedSend(t, a, done) + acknowledgeModelChanges(t, a) + if len(agent.sent) != 1 || agent.submitted[0] != "vendor/configured" || len(agent.temporary) != 2 { + t.Fatalf("sent=%q models=%q selections=%q", agent.sent, agent.submitted, agent.temporary) } } -type heldLegacyModelAgent struct { - *fakeAgent - release chan struct{} +// C4, C6: alternate surface turn doors share the command wait even when +// their start closure does not go through submitStart. +func TestAlternateTurnStartFollowsTheOrderedSelection(t *testing.T) { + a, agent, unblock := modelOrderingApp(t, false) + a.ensureAvailableModel() + cmd := a.submitting("alternate turn", func() (<-chan session.Event, error) { + return agent.Submit(t.Context(), "alternate turn") + }) + done := beginOrderedSend(t, agent, cmd) + unblock() + finishOrderedSend(t, a, done) + acknowledgeModelChanges(t, a) + if len(agent.sent) != 1 || agent.submitted[0] != "vendor/replacement" { + t.Fatal("alternate turn passed the model selection") + } +} + +// C4, C6: a follow-up on an idle engine starts a turn, so its separate call +// must also follow the pending switch rather than overtaking plain sends. +func TestIdleFollowUpFollowsTheOrderedSelection(t *testing.T) { + a, agent, unblock := modelOrderingApp(t, false) + a.ensureAvailableModel() + a.input.insert("start this follow-up") + done := beginOrderedSend(t, agent, a.followUp()) + unblock() + finishOrderedSend(t, a, done) + acknowledgeModelChanges(t, a) + if len(agent.sent) != 1 || agent.sent[0] != "start this follow-up" || agent.submitted[0] != "vendor/replacement" { + t.Fatal("idle follow-up bypassed the ordered model selection") + } +} + +// C6: cancellation bounds a send already waiting for a model door, without +// submitting or canceling the independently ordered model change. +func TestCancelingSendEndsTheModelWait(t *testing.T) { + a, agent, unblock := modelOrderingApp(t, false) + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + a.ctx = ctx + a.ensureAvailableModel() + done := beginOrderedSend(t, agent, a.submit("canceled while waiting")) + cancel() + var msgs []tea.Msg + select { + case msgs = <-done: + case <-time.After(time.Second): + t.Fatal("send context did not end the model wait") + } + found := false + for _, msg := range msgs { + if submitted, ok := msg.(submittedMsg); ok && errors.Is(submitted.err, context.Canceled) { + found = true + } + } + if !found || len(agent.atSubmit) != 0 { + t.Fatal("canceled wait reached Submit or lost its cancellation") + } + unblock() + drive(t, a, msgs...) + acknowledgeModelChanges(t, a) + if len(agent.sent) != 0 { + t.Fatal("model acknowledgment resurrected a canceled send") + } +} + +// C1, C4: refusal releases one message on the current model; reconciliation retries. +func TestRefusedRunOnlySelectionStillSendsOnce(t *testing.T) { + a, agent, unblock := modelOrderingApp(t, true) + a.ensureAvailableModel() + done := beginOrderedSend(t, agent, a.submit("send despite refusal")) + unblock() + finishOrderedSend(t, a, done) + if len(agent.sent) != 1 || agent.sent[0] != "send despite refusal" || agent.submitted[0] != "vendor/configured" { + t.Fatalf("sent=%q models=%q", agent.sent, agent.submitted) + } + if got := lastNote(t, a); got != "could not change model · connection unavailable" { + t.Fatalf("refusal=%q", got) + } + agent.err = nil + a.settle() + acknowledgeModelChanges(t, a) + if agent.Model() != "vendor/replacement" || len(agent.sent) != 1 { + t.Fatal("retry failed or repeated the send") + } +} + +// C5: the explicit pick after a pending temporary selection owns the next send, +// profile write and immediate receipt. +func TestExplicitPickSupersedesAnUnacknowledgedRunOnlySelection(t *testing.T) { + a, agent, unblock := modelOrderingApp(t, false) + a.useModelForNow("vendor/replacement", 0) + a.switchModel("vendor/chosen", 0) + if a.model != "vendor/chosen" || noteSaying(t, a, "model ·") != "model · vendor/chosen" { + t.Fatal("explicit pick did not display immediately") + } + done := beginOrderedSend(t, agent, a.submit("use the explicit choice")) + unblock() + finishOrderedSend(t, a, done) + acknowledgeModelChanges(t, a) + if agent.ConfiguredModel() != "vendor/chosen" || config.ChatModelAt(a.profileDir) != "vendor/chosen" || len(agent.sent) != 1 || agent.submitted[0] != "vendor/chosen" { + t.Fatal("temporary selection displaced the explicit pick or its send") + } } -// C5: leaving a conversation before its queued explicit pick is acknowledged -// does not lose its saved preference or apply its display to the next chat. +// C5: picks made after leaving a conversation keep preference writes ordered. func TestPendingExplicitPickSavesAfterLeavingTheConversation(t *testing.T) { for _, chooseAgain := range []bool{false, true} { t.Run(map[bool]string{false: "leave", true: "choose in next chat"}[chooseAgain], func(t *testing.T) { - agent := &heldModelAgent{ - modelForNowAgent: &modelForNowAgent{fakeAgent: &fakeAgent{model: "vendor/configured"}, configured: "vendor/configured"}, - entered: make(chan struct{}), release: make(chan struct{}), - } - dir := t.TempDir() - a := newApp(t.Context(), Options{Agent: agent, ProfileDir: dir, SaveModel: func(id string) error { return config.WriteChatModel(dir, id) }}) + a, agent, unblock := modelOrderingApp(t, false) a.useModelForNow("vendor/replacement", 0) a.switchModel("vendor/chosen", 0) a.frontGen++ @@ -135,15 +252,20 @@ func TestPendingExplicitPickSavesAfterLeavingTheConversation(t *testing.T) { a.switchModel("other/chosen", 0) wantSaved, wantLive = "other/chosen", "other/chosen" } - close(agent.release) + unblock() acknowledgeModelChanges(t, a) - if agent.ConfiguredModel() != "vendor/chosen" || config.ChatModelAt(dir) != wantSaved || a.model != wantLive { - t.Fatal("leaving the conversation lost a choice or applied choices out of order") + if agent.ConfiguredModel() != "vendor/chosen" || config.ChatModelAt(a.profileDir) != wantSaved || a.model != wantLive { + t.Fatal("leaving lost a choice or reordered preferences") } }) } } +type heldLegacyModelAgent struct { + *fakeAgent + release chan struct{} +} + func (a *heldLegacyModelAgent) SetModel(id string) { select { case <-a.release: @@ -153,17 +275,29 @@ func (a *heldLegacyModelAgent) SetModel(id string) { a.fakeAgent.SetModel(id) } -// C6: an older engine's SetModel fallback also waits off the update loop. +// C6: legacy selection displays immediately, precedes Submit without a UI fold, +// and a canceled send context ends the wait without submitting. func TestLegacyRunOnlySelectionUsesTheOffLoopDoor(t *testing.T) { agent := &heldLegacyModelAgent{fakeAgent: &fakeAgent{model: "vendor/configured"}, release: make(chan struct{})} + unblock := sync.OnceFunc(func() { close(agent.release) }) + t.Cleanup(unblock) a := newApp(t.Context(), Options{Agent: agent, ProfileDir: t.TempDir()}) a.useModelForNow("vendor/replacement", 0) - if !a.modelChangePending() || a.model != "vendor/configured" { - t.Fatal("the legacy model change did not wait for its acknowledgment") + if a.model != "vendor/replacement" { + t.Fatal("legacy selection did not display immediately") + } + ctx, cancel := context.WithCancel(t.Context()) + cancel() + if _, err := submitStart(agent, ctx, "canceled send")(); !errors.Is(err, context.Canceled) || len(agent.sent) != 0 { + t.Fatalf("canceled wait submitted: sent=%q err=%v", agent.sent, err) + } + unblock() + if _, err := submitStart(agent, t.Context(), "legacy send")(); err != nil { + t.Fatal(err) } - close(agent.release) acknowledgeModelChanges(t, a) - if a.model != "vendor/replacement" || agent.Model() != a.model { - t.Fatal("legacy model acknowledgment was lost") + if agent.Model() != "vendor/replacement" || len(agent.sent) != 1 { + t.Fatal("legacy selection did not precede its send") } + agent.finish() } diff --git a/internal/tui3/model_for_now.go b/internal/tui3/model_for_now.go index fb494c8029..c196f9540f 100644 --- a/internal/tui3/model_for_now.go +++ b/internal/tui3/model_for_now.go @@ -9,13 +9,6 @@ import ( const modelChangeRefusedWord = "could not change model · " -// modelChange belongs to the conversation that queued it. Its acknowledgment -// may arrive after a later pick or after the surface has switched chats. -type modelChange struct { - front int - retry func() -} - // configuredModel keeps reconciliation anchored to the person's saved choice. // Agents predating the run-only door keep the existing live-model behavior. func (a *app) configuredModel() string { @@ -32,19 +25,15 @@ func (a *app) useModelForNow(id string, window int) { a.queueModelChange(id, window, true, true, nil) } -func (a *app) modelChangePending() bool { - return a.modelChange != nil && a.modelChange.front == a.frontGen -} - func (a *app) takeModelCommands() tea.Cmd { cmds := a.modelCommands a.modelCommands = nil return tea.Batch(cmds...) } -// Temporary picks and any explicit picks behind them share the ordered door -// line. The surface only adopts the last acknowledged selection, and sends -// cannot pass its gate while that selection is pending. +// Temporary picks and explicit picks behind them share the ordered door line. +// The display changes immediately; send commands wait for the agent's token, +// which completes with the ask rather than its later UI acknowledgment. func (a *app) queueModelChange(id string, window int, temporary, quiet bool, rename *modelRename) { agent := a.agent save := a.saveModel @@ -55,9 +44,17 @@ func (a *app) queueModelChange(id string, window int, temporary, quiet bool, ren if rename != nil { remember = rename.save } - change := &modelChange{front: a.frontGen} - a.modelChange = change + if window <= 0 { + window = a.windowFor(id) + } + selected := &modelSelection{done: make(chan struct{}), model: id, window: window, temporary: temporary && rename == nil} + modelSelections.Store(agent, selected) + a.pendingModelSelection = selected.done cmd := a.offLoop(func() func(bool) tea.Cmd { + defer func() { + modelSelections.CompareAndDelete(agent, selected) + close(selected.done) + }() var err error if rename != nil { if door, ok := agent.(interface { @@ -75,54 +72,28 @@ func (a *app) queueModelChange(id string, window int, temporary, quiet bool, ren agent.SetModel(id) } } - // Durable choices keep their order even if the window leaves before - // the reply. Only display work depends on still being in front. + // Durable choices keep their order even after leaving the conversation. if err == nil && save != nil && remember != "" { _ = save(remember) } - return func(bool) tea.Cmd { - if a.modelChange != change { - // A switched conversation keeps its gate in the sidecar. Its - // reply may release only that conversation's waiting message. - for key, held := range a.behind { - if held.side == nil || held.side.modelChange != change { - continue - } - if err != nil { - change.retry = func() { a.queueModelChange(id, window, temporary, quiet, rename) } - held.side.parkNotes = append(held.side.parkNotes, modelChangeRefusedWord+err.Error()) - return nil - } - held.side.modelChange = nil - return a.checkBehindParked(key, agent) - } - return nil - } - if a.agent != agent { - a.modelChange = nil - return nil - } - if err != nil { - change.retry = func() { a.queueModelChange(id, window, temporary, quiet, rename) } + return func(here bool) tea.Cmd { + if err != nil && here && a.agent == agent { a.note(modelChangeRefusedWord + err.Error()) - return nil - } - a.modelChange = nil - wasSwitching := a.creditSwitching - a.creditSwitching = quiet - a.modelSwitched(id, window, false) - a.creditSwitching = wasSwitching - a.ensureAvailableModel() - if a.connPanel.open { - a.connPanel.adopt(a.connectionRows()) - } - if a.at(pageSettings) { - a.sheet.build() } - a.hudStale = true - a.touch() - return tea.Batch(a.takeModelCommands(), a.sendParked()) + return nil } }) a.modelCommands = append(a.modelCommands, cmd) + wasSwitching := a.creditSwitching + a.creditSwitching = quiet + a.displayModelSwitched(id, window, false) + a.creditSwitching = wasSwitching + if a.connPanel.open { + a.connPanel.adopt(a.connectionRows()) + } + if a.at(pageSettings) { + a.sheet.build() + } + a.hudStale = true + a.touch() } diff --git a/internal/tui3/model_order.go b/internal/tui3/model_order.go new file mode 100644 index 0000000000..e69624e40f --- /dev/null +++ b/internal/tui3/model_order.go @@ -0,0 +1,44 @@ +package tui3 + +import ( + "context" + "sync" +) + +// modelSelections holds only unfinished asks, keyed by their owning agent. +// Commands can start in any order, including after replay or a chat switch. +var modelSelections sync.Map + +type modelSelection struct { + done chan struct{} + model string + window int + temporary bool +} + +func pendingModelSelection(agent Agent) *modelSelection { + if selected, ok := modelSelections.Load(agent); ok { + return selected.(*modelSelection) + } + return nil +} + +// waitModelSelection runs only inside a send command, never on the update loop. +// A refusal still completes the ask, letting the engine answer on its current +// model. A later selection queued during the wait must also precede this send. +func waitModelSelection(ctx context.Context, agent Agent) error { + for { + if err := ctx.Err(); err != nil { + return err + } + selected := pendingModelSelection(agent) + if selected == nil { + return nil + } + select { + case <-selected.done: + case <-ctx.Done(): + return ctx.Err() + } + } +} diff --git a/internal/tui3/model_park_test.go b/internal/tui3/model_park_test.go index 6494e4e4c4..ed125e89db 100644 --- a/internal/tui3/model_park_test.go +++ b/internal/tui3/model_park_test.go @@ -2,16 +2,13 @@ package tui3 import ( "context" - "errors" "os" "path/filepath" "reflect" "strings" "testing" - "time" tea "charm.land/bubbletea/v2" - "github.com/Agent-Field/codeaf/internal/config" "github.com/Agent-Field/codeaf/internal/modelsource" "github.com/Agent-Field/codeaf/internal/session" @@ -20,207 +17,186 @@ import ( type parkedModelAgent struct{ *heldModelAgent } func (a *parkedModelAgent) SubmitImage(ctx context.Context, text string, images []session.Image) (<-chan session.Event, error) { + a.atSubmit <- a.Model() a.submitted = append(a.submitted, a.Model()) return a.fakeAgent.SubmitImage(ctx, text, images) } -// C1, C2, C4: stream closure holds parked words, paste bodies and attachments -// through catalog recovery/removal and a pending/refused model acknowledgment. -func TestParkedMessageWaitsForTheAcknowledgedModelAtStreamClose(t *testing.T) { - for _, recovery := range []bool{true, false} { - for _, attachment := range []string{"text", "file", "picture"} { - for _, refused := range []bool{false, true} { - name := map[bool]string{true: "recovery", false: "removal"}[recovery] + "/" + attachment + "/" + map[bool]string{false: "accepted", true: "refused"}[refused] - t.Run(name, func(t *testing.T) { - live, want := "vendor/configured", "vendor/replacement" - if recovery { - live, want = want, live - } - agent := &parkedModelAgent{&heldModelAgent{ - modelForNowAgent: &modelForNowAgent{fakeAgent: &fakeAgent{model: live}, configured: "vendor/configured"}, - entered: make(chan struct{}), release: make(chan struct{}), - }} - defer agent.finish() - if refused { - agent.err = errors.New("connection unavailable") - } - rows := []Model{{ID: live}} - dir := t.TempDir() - if err := config.WriteChatModel(dir, "vendor/configured"); err != nil { - t.Fatal(err) - } - a := newApp(t.Context(), Options{Agent: agent, ProfileDir: dir, RequireListedModel: true, - Sources: modelsource.NewSet(testDefaultService("key")), Models: func() []Model { return rows }, - SaveModel: func(id string) error { return config.WriteChatModel(dir, id) }, - }) - spend(t, a, a.submit("first message")) - a.pastes = []pasteChip{{n: 1, text: "the complete pasted document"}} - if attachment != "text" { - path := filepath.Join(t.TempDir(), "attachment") - if err := os.WriteFile(path, []byte("attachment bytes"), 0600); err != nil { - t.Fatal(err) - } - a.chips = []chip{{path: path, file: attachment == "file"}} - } - a.park("waiting "+pasteToken(1, 1), []segment{{from: 0, to: 7}}) - waiting := a.parks[0] - // A newer draft's tray must survive the older parked send too. - a.chips = []chip{{path: "draft attachment", file: true}} - draftChips := append([]chip(nil), a.chips...) - rows = []Model{{ID: want}} - a.serviceModelsLanded(modelsource.DefaultID, config.DefaultBaseURL) - agent.finish() - _, cmd := a.Update(streamClosedMsg{gen: a.gen}) - select { - case <-agent.entered: - case <-time.After(time.Second): - close(agent.release) - t.Fatal("the turn-end model door never started") - } - // The direct attachment door must preserve the newer tray at - // the same gate, even when called outside the parked road. - if attachment != "text" { - cmd = tea.Batch(cmd, a.submitImagesShown("draft", "draft", nil)) - } - if len(agent.sent) != 1 || len(agent.imageText) != 0 || !reflect.DeepEqual(a.parks, []parked{waiting}) || !reflect.DeepEqual(a.chips, draftChips) { - close(agent.release) - t.Fatalf("pending selection spent a parked message: sent=%q parks=%+v tray=%+v", agent.sent, a.parks, a.chips) - } - close(agent.release) - spend(t, a, cmd) - if refused { - if len(agent.sent) != 1 || len(agent.imageText) != 0 || !reflect.DeepEqual(a.parks, []parked{waiting}) || !a.modelChangePending() { - t.Fatal("refused selection spent the parked words or attachments") - } - if got := lastNote(t, a); got != "could not change model · connection unavailable" { - t.Fatalf("refusal=%q", got) - } - agent.err = nil - if a.ensureAvailableModel() { - t.Fatal("retry opened the gate before acknowledgment") - } - acknowledgeModelChanges(t, a) - } - if len(agent.sent) != 2 || !strings.Contains(agent.sent[1], waiting.spoken()) || len(agent.submitted) != 2 || agent.submitted[1] != want || len(a.parks) != 0 { - t.Fatalf("acknowledged send=%q models=%q parks=%+v", agent.sent, agent.submitted, a.parks) - } - if attachment == "picture" && (len(agent.images) != 1 || len(agent.images[0]) != 1 || string(agent.images[0][0].Bytes) != "attachment bytes") { - t.Fatalf("parked picture=%+v", agent.images) - } - if attachment == "file" && !strings.Contains(agent.sent[1], waiting.chips[0].path) { - t.Fatal("parked file did not travel with its words") - } - if !reflect.DeepEqual(a.chips, draftChips) || agent.ConfiguredModel() != "vendor/configured" || config.ChatModelAt(dir) != "vendor/configured" { - t.Fatal("parked send spent the draft tray or saved the replacement") - } - acknowledgeModelChanges(t, a) - if len(agent.sent) != 2 { - t.Fatal("the acknowledgment sent the parked message twice") - } - }) +// C1, C2, C4: parked text, paste bodies and attachments submit exactly once after +// the model door, without UI acknowledgment or loss of a newer draft tray. +func TestParkedMessageFollowsItsOrderedModelAtStreamClose(t *testing.T) { + for _, attachment := range []string{"text", "file", "picture"} { + t.Run(attachment, func(t *testing.T) { + a, base, unblock := modelOrderingApp(t, false) + agent := &parkedModelAgent{base} + a.agent = agent + a.state = stateWorking + a.pastes = []pasteChip{{n: 1, text: "the complete pasted document"}} + if attachment != "text" { + path := filepath.Join(t.TempDir(), "attachment") + if err := os.WriteFile(path, []byte("attachment bytes"), 0600); err != nil { + t.Fatal(err) + } + a.chips = []chip{{path: path, file: attachment == "file"}} + } + a.park("waiting "+pasteToken(1, 1), []segment{{from: 0, to: 7}}) + waiting := a.parks[0] + a.chips = []chip{{path: "draft attachment", file: true}} + draft := append([]chip(nil), a.chips...) + _, cmd := a.Update(streamClosedMsg{gen: a.gen}) + done := beginOrderedSend(t, base, cmd) + if len(a.parks) != 0 || !reflect.DeepEqual(a.chips, draft) { + t.Fatal("surface held the parked message or spent the newer tray") + } + unblock() + finishOrderedSend(t, a, done) + if len(agent.sent) != 1 || !strings.Contains(agent.sent[0], waiting.spoken()) || len(agent.submitted) != 1 || agent.submitted[0] != "vendor/replacement" { + t.Fatalf("sent=%q models=%q", agent.sent, agent.submitted) } - } + if attachment == "picture" && (len(agent.images) != 1 || len(agent.images[0]) != 1 || string(agent.images[0][0].Bytes) != "attachment bytes") { + t.Fatalf("picture=%+v", agent.images) + } + if attachment == "file" && !strings.Contains(agent.sent[0], waiting.chips[0].path) { + t.Fatal("file path was lost") + } + acknowledgeModelChanges(t, a) + if len(agent.sent) != 1 || agent.ConfiguredModel() != "vendor/configured" { + t.Fatal("acknowledgment repeated the send or saved the replacement") + } + }) } } -type parkedSwitchModelAgent struct{ *parkedModelAgent } +type replayModelAgent struct{ *heldModelAgent } -func (a *parkedSwitchModelAgent) Attach() (<-chan session.Event, bool, func()) { - return nil, false, func() {} +func (a *replayModelAgent) AttachReplay() ([]session.DisplayEntry, <-chan session.Event, func()) { + return nil, nil, func() {} } -// C1, C4, C5: a model acknowledgment after a real detach/attach releases only -// its owner's parked send; a refusal keeps that owner's complete queue held. -func TestParkedModelAcknowledgmentBelongsToTheConversationThatQueuedIt(t *testing.T) { +// C1, C4: a replay-deferred draft survives a catalog change before the replay +// fold and sends once after the switch, including a refused switch. +func TestReplayDeferredSendFollowsItsOrderedModel(t *testing.T) { for _, refused := range []bool{false, true} { t.Run(map[bool]string{false: "accepted", true: "refused"}[refused], func(t *testing.T) { - agent := &parkedSwitchModelAgent{&parkedModelAgent{&heldModelAgent{ - modelForNowAgent: &modelForNowAgent{fakeAgent: &fakeAgent{model: "vendor/replacement"}, configured: "vendor/configured"}, - entered: make(chan struct{}), release: make(chan struct{}), - }}} - defer agent.finish() - if refused { - agent.err = errors.New("connection unavailable") - } - rows := []Model{{ID: "vendor/replacement"}, {ID: "vendor/other"}} - a := newApp(t.Context(), Options{Agent: agent, ProfileDir: t.TempDir(), RequireListedModel: true, - Sources: modelsource.NewSet(testDefaultService("key")), Models: func() []Model { return rows }, - }) - a.file = filepath.Join(t.TempDir(), "held.jsonl") - spend(t, a, a.submit("first message")) - a.park("the owning conversation's message", nil) - rows = append(rows, Model{ID: "vendor/configured"}) - agent.finish() - _, pending := a.Update(streamClosedMsg{gen: a.gen}) - select { - case <-agent.entered: - case <-time.After(time.Second): - close(agent.release) - t.Fatal("the model door never started") + a, base, unblock := modelOrderingApp(t, refused) + a.agent = &replayModelAgent{base} + base.fakeAgent.model, a.model = "vendor/replacement", "vendor/replacement" + answer := a.refreshHostedReplay()() + a.input.insert("keep the replay-deferred words") + if cmd := a.enterLine(); cmd != nil || len(a.hostDeferred) != 1 || a.input.String() != "" { + t.Fatal("send was not deferred at the replay boundary") } - conv, side := a.front(), a.detachConversation() - stowed := a.stow(conv, side) - key := a.convKey(conv.SessionFile) - held := a.behind[key] - t.Cleanup(held.watch.stop) - front := &modelForNowAgent{fakeAgent: &fakeAgent{model: "vendor/other"}, configured: "vendor/other"} - attached := a.attachConversation(Conversation{Agent: front, SessionFile: filepath.Join(t.TempDir(), "front.jsonl")}, nil) - a.parks = []parked{{text: "the front conversation's message"}} - if a.sendBehindParked(key, held) != nil || len(side.parks) != 1 || len(agent.sent) != 1 { - close(agent.release) - t.Fatal("switching chats bypassed the pending model acknowledgment") - } - close(agent.release) - spend(t, a, tea.Batch(pending, stowed, attached)) - if len(front.sent) != 0 || len(a.parks) != 1 || front.ConfiguredModel() != "vendor/other" { - t.Fatal("the old acknowledgment released or changed the front conversation") + a.models = func() []Model { return []Model{{ID: "vendor/configured"}} } + a.serviceModelsLanded(modelsource.DefaultID, config.DefaultBaseURL) + _, cmd := a.Update(answer) + done := beginOrderedSend(t, base, cmd) + if len(a.hostDeferred) != 0 { + t.Fatal("replay kept a second copy of its send") } + unblock() + finishOrderedSend(t, a, done) + want := "vendor/configured" if refused { - if len(agent.sent) != 1 || len(side.parks) != 1 || a.sendBehindParked(key, held) != nil { - t.Fatal("refusal released the hidden conversation's queue") - } - agent.err = nil - held.watch.stop() - delete(a.behind, key) - a.detachConversation() - spend(t, a, a.attachConversation(conv, side)) - acknowledgeModelChanges(t, a) + want = "vendor/replacement" } - if len(agent.sent) != 2 || agent.sent[1] != "the owning conversation's message" || len(agent.submitted) != 2 || agent.submitted[1] != "vendor/configured" { - t.Fatalf("owning send=%q models=%q", agent.sent, agent.submitted) + if len(base.sent) != 1 || base.sent[0] != "keep the replay-deferred words" || base.submitted[0] != want { + t.Fatalf("sent=%q models=%q", base.sent, base.submitted) + } + acknowledgeModelChanges(t, a) + if len(base.sent) != 1 { + t.Fatal("replay acknowledgment repeated the send") } }) } } -// C1, C4: coming back before the original reply keeps its model gate and -// resumes the parked send once, despite the intervening front generations. -func TestReturningBeforeModelAcknowledgmentResumesTheParkedSend(t *testing.T) { - agent := &parkedModelAgent{&heldModelAgent{ - modelForNowAgent: &modelForNowAgent{fakeAgent: &fakeAgent{model: "vendor/replacement"}, configured: "vendor/configured"}, - entered: make(chan struct{}), release: make(chan struct{}), - }} - defer agent.finish() - rows := []Model{{ID: "vendor/replacement"}, {ID: "vendor/other"}} - a := newApp(t.Context(), Options{Agent: agent, ProfileDir: t.TempDir(), RequireListedModel: true, - Sources: modelsource.NewSet(testDefaultService("key")), Models: func() []Model { return rows }, - }) - spend(t, a, a.submit("first message")) - a.park("the returning conversation's message", nil) - rows = append(rows, Model{ID: "vendor/configured"}) - agent.finish() - _, pending := a.Update(streamClosedMsg{gen: a.gen}) +type parkedSwitchModelAgent struct{ *heldModelAgent } + +func (a *parkedSwitchModelAgent) Attach() (<-chan session.Event, bool, func()) { + return nil, false, func() {} +} + +// C1, C4, C5: a held chat's pending switch precedes its parked send and never +// changes the unrelated front chat; the keeper consumes the message once. +func TestHeldSendFollowsItsOwnOrderedModel(t *testing.T) { + a, base, unblock := modelOrderingApp(t, false) + agent := &parkedSwitchModelAgent{base} + a.agent = agent + a.file = filepath.Join(t.TempDir(), "held.jsonl") + a.parks = []parked{{text: "the held conversation's message"}} + a.ensureAvailableModel() conv, side := a.front(), a.detachConversation() - away := a.attachConversation(Conversation{Agent: &fakeAgent{model: "vendor/other"}}, nil) + stowed := a.stow(conv, side) + key := a.convKey(conv.SessionFile) + held := a.behind[key] + t.Cleanup(held.watch.stop) + front := &modelForNowAgent{fakeAgent: &fakeAgent{model: "vendor/chosen"}, configured: "vendor/chosen"} + attached := a.attachConversation(Conversation{Agent: front, SessionFile: filepath.Join(t.TempDir(), "front.jsonl")}, nil) + done := beginOrderedSend(t, base, tea.Batch(a.sendBehindParked(key, held), stowed, attached)) + unblock() + finishOrderedSend(t, a, done) + acknowledgeModelChanges(t, a) + if len(base.sent) != 1 || base.sent[0] != "the held conversation's message" || base.submitted[0] != "vendor/replacement" || len(side.parks) != 0 { + t.Fatalf("held send=%q models=%q parks=%+v", base.sent, base.submitted, side.parks) + } + if len(front.sent) != 0 || front.Model() != "vendor/chosen" || a.model != "vendor/chosen" { + t.Fatal("held selection affected the front chat") + } +} + +// C1, C4, C5: returning before the model ask completes restores its immediate +// display and sends once through the same agent token, across front generations. +func TestReturningBeforeModelSelectionKeepsTheOrderedSend(t *testing.T) { + a, base, unblock := modelOrderingApp(t, false) + a.ensureAvailableModel() + conv, side := a.front(), a.detachConversation() + away := a.attachConversation(Conversation{Agent: &fakeAgent{model: "vendor/chosen"}}, nil) a.detachConversation() back := a.attachConversation(conv, side) - if !a.modelChangePending() || len(a.parks) != 1 || len(agent.sent) != 1 { - close(agent.release) - t.Fatal("returning lost the pending model gate or its waiting message") + if a.model != "vendor/replacement" { + t.Fatal("returning lost the pending model display") + } + done := beginOrderedSend(t, base, tea.Batch(away, back, a.submit("send after returning"))) + unblock() + finishOrderedSend(t, a, done) + acknowledgeModelChanges(t, a) + if len(base.sent) != 1 || base.submitted[0] != "vendor/replacement" { + t.Fatal("returning repeated or bypassed the ordered send") } - close(agent.release) - spend(t, a, tea.Batch(pending, away, back)) - if a.modelChangePending() || len(agent.sent) != 2 || agent.sent[1] != "the returning conversation's message" || len(agent.submitted) != 2 || agent.submitted[1] != "vendor/configured" || len(a.parks) != 0 { - t.Fatalf("returning acknowledgment: sent=%q models=%q parks=%+v", agent.sent, agent.submitted, a.parks) +} + +// C4, C6: catalog recovery does not reconcile a hidden chat; its next front +// attach restores the configured model before that chat's subsequent request. +func TestHiddenChatReconcilesAvailabilityOnlyWhenItReturns(t *testing.T) { + a, base, unblock := modelOrderingApp(t, false) + agent := &parkedSwitchModelAgent{base} + a.agent = agent + a.file = filepath.Join(t.TempDir(), "held.jsonl") + a.ensureAvailableModel() + unblock() + acknowledgeModelChanges(t, a) + conv, side := a.front(), a.detachConversation() + stowed := a.stow(conv, side) + key := a.convKey(conv.SessionFile) + held := a.behind[key] + t.Cleanup(held.watch.stop) + front := &modelForNowAgent{fakeAgent: &fakeAgent{model: "vendor/chosen"}, configured: "vendor/chosen"} + spend(t, a, tea.Batch(stowed, a.attachConversation(Conversation{Agent: front}, nil))) + a.models = func() []Model { return []Model{{ID: "vendor/configured"}, {ID: "vendor/chosen"}} } + a.serviceModelsLanded(modelsource.DefaultID, config.DefaultBaseURL) + held.side.parks = []parked{{text: "send while hidden"}} + spend(t, a, a.checkBehindParked(key, agent)) + if len(base.sent) != 1 || base.submitted[0] != "vendor/replacement" || len(base.temporary) != 1 { + t.Fatal("hidden catalog arrival reconciled the held conversation") + } + base.finish() + held.watch.stop() + delete(a.behind, key) + a.detachConversation() + spend(t, a, a.attachConversation(conv, side)) + acknowledgeModelChanges(t, a) + spend(t, a, a.submit("send after returning")) + if len(base.sent) != 2 || base.submitted[1] != "vendor/configured" || base.ConfiguredModel() != "vendor/configured" { + t.Fatalf("returning send=%q models=%q", base.sent, base.submitted) } } diff --git a/internal/tui3/model_rename_owner_test.go b/internal/tui3/model_rename_owner_test.go index f9239469cf..2958af96d3 100644 --- a/internal/tui3/model_rename_owner_test.go +++ b/internal/tui3/model_rename_owner_test.go @@ -8,7 +8,13 @@ import ( "github.com/Agent-Field/codeaf/internal/session" ) -func deferredRenameApp(t *testing.T, paired bool) (*app, *pairedModelForNowAgent) { +type replayRenameAgent struct{ *pairedModelForNowAgent } + +func (a *replayRenameAgent) AttachReplay() ([]session.DisplayEntry, <-chan session.Event, func()) { + return nil, nil, func() {} +} + +func deferredRenameApp(t *testing.T, paired bool, hosted ...bool) (*app, *pairedModelForNowAgent) { t.Helper() dir := t.TempDir() customs := connectionWriteSources(t, dir, "mybox") @@ -20,6 +26,9 @@ func deferredRenameApp(t *testing.T, paired bool) (*app, *pairedModelForNowAgent agent.pair = session.ConversationModels{Main: "mybox/chosen", Fallback: "mybox/backup"} door = agent } + if len(hosted) > 0 && hosted[0] { + door = &replayRenameAgent{agent} + } a := modelServiceTestAppWithAgent(t, dir, agent.configured, modelsource.NewSet(append([]modelsource.Connected{connectionDefaultService()}, customs...)...), []Model{{ID: "vendor/replacement"}, {ID: "vendor/other"}, {ID: "vendor/explicit"}}, door) @@ -53,9 +62,6 @@ func TestDeferredRenameStaysWithItsConversationAcrossASwitch(t *testing.T) { a, original := deferredRenameApp(t, paired) a.park("waiting for the renamed conversation", nil) conv, side := a.front(), a.detachConversation() - if a.sendBehindParked("owner", &kept{conv: conv, side: side}) != nil || len(side.parks) != 1 { - t.Fatal("the hidden parked send bypassed its deferred rename") - } other := &modelForNowAgent{fakeAgent: &fakeAgent{model: "vendor/other"}, configured: "vendor/other"} spend(t, a, a.attachConversation(Conversation{Agent: other}, nil)) a.switchModel("vendor/other", 0) @@ -70,6 +76,7 @@ func TestDeferredRenameStaysWithItsConversationAcrossASwitch(t *testing.T) { a.detachConversation() spend(t, a, a.attachConversation(conv, side)) acknowledgeModelChanges(t, a) + spend(t, a, a.sendParked()) if original.ConfiguredModel() != "lab/chosen" || original.Model() != "vendor/replacement" || other.ConfiguredModel() != "vendor/other" || config.ChatModelAt(a.profileDir) == "vendor/replacement" { t.Fatal("the owning conversation lost its rename or saved its replacement") } @@ -84,6 +91,24 @@ func TestDeferredRenameStaysWithItsConversationAcrossASwitch(t *testing.T) { } } +// C1, C2, C5: the owning chat's hosted replay boundary applies its deferred +// rename even when no send is waiting; the other chat remains untouched. +func TestDeferredRenameAppliesAtItsOwningReplayBoundary(t *testing.T) { + a, original := deferredRenameApp(t, true, true) + conv, side := a.front(), a.detachConversation() + other := &modelForNowAgent{fakeAgent: &fakeAgent{model: "vendor/other"}, configured: "vendor/other"} + spend(t, a, a.attachConversation(Conversation{Agent: other}, nil)) + a.detachConversation() + spend(t, a, a.attachConversation(conv, side)) + acknowledgeModelChanges(t, a) + if original.ConfiguredModel() != "lab/chosen" || original.Model() != "vendor/replacement" || original.pair != (session.ConversationModels{Main: "lab/chosen", Fallback: "lab/backup"}) || a.deferredModelRename != nil { + t.Fatal("the owning replay did not apply its deferred rename") + } + if other.ConfiguredModel() != "vendor/other" || config.ChatModelAt(a.profileDir) == "vendor/replacement" || len(original.sent) != 0 || len(other.sent) != 0 { + t.Fatal("the replay rename changed another chat, saved the replacement or invented a send") + } +} + // C2, C5: a later explicit single or pair choice retires the captured rename, // including a pair whose main model is unchanged and only its fallback changes. func TestLaterExplicitChoiceRetiresTheDeferredRename(t *testing.T) { diff --git a/internal/tui3/modeldefault.go b/internal/tui3/modeldefault.go index 50679ee22a..37238ceaa2 100644 --- a/internal/tui3/modeldefault.go +++ b/internal/tui3/modeldefault.go @@ -39,24 +39,27 @@ func (a *app) isAvailableModel(id string) bool { // A launch preference is not evidence of availability. An automatic choice uses // the off-loop run-only door without saving a preference the person did not // choose. With no rows, only the display is cleared: the engine has no unset-model -// door, and the send gates below keep that old launch preference from being used. +// door, and the empty-catalog checks keep that old preference from being used. func (a *app) ensureAvailableModel() bool { if rename := a.deferredModelRename; rename != nil { - // A held rename waits for its owner's turn/replay boundary before - // touching the saved selection. Hidden parked sends remain held too. - if a.state == stateWorking || a.hostReplayLoading { - return false + // Relabel only at the owning conversation's turn/replay boundary. + if a.state != stateWorking && !a.hostReplayLoading { + a.deferredModelRename = nil + a.applyModelRename(rename) } - a.deferredModelRename = nil - a.applyModelRename(rename) } - if a.modelChangePending() { - if retry := a.modelChange.retry; retry != nil { - retry() + if a.agent == nil { + return true + } + pending := pendingModelSelection(a.agent) + if pending != nil { + // Returning before the ask completes still shows the chosen model. + a.model = pending.model + if pending.window > 0 { + a.ctxWindow = pending.window } - return false } - if !a.requireListedModel || a.agent == nil { + if !a.requireListedModel { return true } // An answering request keeps its model until it settles. The turn-end path @@ -64,6 +67,11 @@ func (a *app) ensureAvailableModel() bool { if a.state == stateWorking { return a.model != "" } + // A later explicit pick or relabel owns its place on the line. Automatic + // selections may follow a newer catalog while an earlier ask is unfinished. + if pending != nil && !pending.temporary { + return a.model != "" + } preferred := a.configuredModel() model, ok := availableConversationModel(preferred, a.modelList()) if !ok { @@ -71,10 +79,9 @@ func (a *app) ensureAvailableModel() bool { a.hudStale = true return false } - if model.ID != a.model { + if model.ID != a.model || (pending == nil && model.ID != a.agent.Model()) { a.useModelForNow(model.ID, model.ContextLength) a.hudStale = true - return false } return a.model == model.ID } diff --git a/internal/tui3/modeldefault_test.go b/internal/tui3/modeldefault_test.go index 14d067dcf5..06a315500d 100644 --- a/internal/tui3/modeldefault_test.go +++ b/internal/tui3/modeldefault_test.go @@ -144,10 +144,10 @@ func TestAnAvailableOpeningModelIsTheModelSentToItsRealProvider(t *testing.T) { Models: func() []Model { return []Model{{ID: config.DefaultModel}} }, ModelsForService: func(modelsource.Connected) []Model { return []Model{{ID: "actually-listed"}} }, }) - acknowledgeModelChanges(t, a) if a.model != "deepseek-direct/actually-listed" { t.Fatalf("default = %q", a.model) } + acknowledgeModelChanges(t, a) drainModelServiceTurn(t, agent, "answer on the available default") requests := completionRequests(server) if len(requests) != 1 || completionModel(t, requests[0]) != "actually-listed" { diff --git a/internal/tui3/palette.go b/internal/tui3/palette.go index 3300784c71..e8967e69b3 100644 --- a/internal/tui3/palette.go +++ b/internal/tui3/palette.go @@ -3107,19 +3107,28 @@ func (a *app) windowFor(id string) int { // saying so would keep compacting as if it were on the 128k one it started on. func (a *app) switchModel(id string, window int) { a.deferredModelRename = nil - if a.modelChange != nil { - a.queueModelChange(id, window, false, a.creditSwitching, nil) - return + if a.pendingModelSelection != nil { + select { + case <-a.pendingModelSelection: + default: + a.queueModelChange(id, window, false, a.creditSwitching, nil) + return + } } a.agent.SetModel(id) a.modelSwitched(id, window, true) } func (a *app) modelSwitched(id string, window int, remember bool) { - a.model = a.agent.Model() - if a.model == "" { - a.model = id + if model := a.agent.Model(); model != "" { + id = model } + a.displayModelSwitched(id, window, remember) +} + +// Queued selections paint on the gesture, before the engine acknowledges them. +func (a *app) displayModelSwitched(id string, window int, remember bool) { + a.model = id // The new model's dial is learned HERE rather than left to the frame clock, // so the status row names it on the very frame the switch lands on // (reasoninglevel.go states the three moments that are seeded and why). diff --git a/internal/tui3/park.go b/internal/tui3/park.go index 9d6c845221..5e8bbf20d3 100644 --- a/internal/tui3/park.go +++ b/internal/tui3/park.go @@ -117,14 +117,9 @@ func (a *app) sendParked() tea.Cmd { // A pending take-back can leave the surface between pumps while the // session's next turn is already running. Its queue settles first; parked // words must not open a second turn in that gap. - if a.state == stateWorking || a.stream != nil || len(a.follows) > 0 || a.parkSending || len(a.parks) == 0 || a.hostReplayLoading || a.hostReplayWaiting { + if a.stream != nil || len(a.follows) > 0 || a.parkSending || len(a.parks) == 0 { return nil } - // Keep the whole message until the model door acknowledges the next - // request's selection, including pictures that use a separate submit door. - if !a.ensureAvailableModel() { - return a.takeModelCommands() - } next := a.parks[0] a.parks = a.parks[1:] shown := next.text diff --git a/internal/tui3/startchat.go b/internal/tui3/startchat.go index 99b11aa70d..672579c5b4 100644 --- a/internal/tui3/startchat.go +++ b/internal/tui3/startchat.go @@ -76,6 +76,9 @@ func (a *app) startRequestedChat(request *session.ChatStart, owner Agent) tea.Cm conv.Agent.SetContextWindow(model.ContextLength) } } + if err == nil { + err = waitModelSelection(context.Background(), conv.Agent) + } if err == nil { events, err = conv.Agent.Submit(context.Background(), request.Brief) } From 162a795186647411b821450a0929ab0c5974f487 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 6 Nov 2026 10:49:41 -0500 Subject: [PATCH 6/6] Models: a takeover resume waits for its chat's pending model switch Review round 4: resuming a turn stopped by a takeover started the turn without waiting for the chat's run-only switch still on the door line. It now waits inside its command like every other turn start; a cancelled wait leaves the resume for later, a refused switch still allows one resume. Co-Authored-By: Claude Opus 5.5 --- internal/tui3/takeover.go | 5 +- internal/tui3/takeover_model_order_test.go | 220 +++++++++++++++++++++ 2 files changed, 224 insertions(+), 1 deletion(-) create mode 100644 internal/tui3/takeover_model_order_test.go diff --git a/internal/tui3/takeover.go b/internal/tui3/takeover.go index 592781042b..8c64b09817 100644 --- a/internal/tui3/takeover.go +++ b/internal/tui3/takeover.go @@ -665,8 +665,11 @@ func (a *app) resumeStoppedTurn() tea.Cmd { if !ok || a.stream != nil { return nil } - gen, ctx := a.gen, a.ctx + gen, ctx, agent := a.gen, a.ctx, a.agent return func() tea.Msg { + if err := waitModelSelection(ctx, agent); err != nil { + return nil + } events, resumed := door.ResumeStoppedTurn(ctx) if !resumed { return nil diff --git a/internal/tui3/takeover_model_order_test.go b/internal/tui3/takeover_model_order_test.go new file mode 100644 index 0000000000..43b6c0dbf6 --- /dev/null +++ b/internal/tui3/takeover_model_order_test.go @@ -0,0 +1,220 @@ +package tui3 + +import ( + "context" + "encoding/json" + "errors" + "os" + "path/filepath" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + tea "charm.land/bubbletea/v2" + "github.com/Agent-Field/codeaf/internal/modelsource" + "github.com/Agent-Field/codeaf/internal/modelsource/sourcestub" + "github.com/Agent-Field/codeaf/internal/session" +) + +// Only the arrival and selection doors are added to the ordinary surface +// interface, so unrelated standing task lanes do not participate in this test. +type heldResumeModelAgent struct { + Agent + engine *session.Agent + ctx context.Context + entered chan struct{} + release chan struct{} + refused bool + resumeCalls atomic.Int32 +} + +func (a *heldResumeModelAgent) ConfiguredModel() string { return a.engine.ConfiguredModel() } + +func (a *heldResumeModelAgent) UseModelForNow(model string) error { + close(a.entered) + select { + case <-a.release: + case <-a.ctx.Done(): + return a.ctx.Err() + } + if a.refused { + return errors.New("model selection refused") + } + return a.engine.UseModelForNow(model) +} + +func (a *heldResumeModelAgent) AttachReplay() ([]session.DisplayEntry, <-chan session.Event, func()) { + return a.engine.AttachReplay() +} + +func (a *heldResumeModelAgent) ResumeStoppedTurn(ctx context.Context) (<-chan session.Event, bool) { + a.resumeCalls.Add(1) + return a.engine.ResumeStoppedTurn(ctx) +} + +// C1, C4, C6; fix round 5: an unanswered arrival waits for its owning model +// selection before resuming once. Refusal permits the current model; canceling +// the wait preserves the journal's resume opportunity for a later attempt. +func TestTakeoverReplayResumeFollowsItsOrderedModel(t *testing.T) { + for _, outcome := range []string{"accepted", "refused", "canceled"} { + t.Run(outcome, func(t *testing.T) { + t.Setenv("CODEAF_HOME", t.TempDir()) + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + defer cancel() + server := sourcestub.New("replacement") + defer server.Close() + sources := modelsource.NewSet(testDefaultService(""), testDirectService(server.URL())) + dir := t.TempDir() + path := filepath.Join(dir, "transcript.jsonl") + const question = "answer the question already in the journal" + journal := `{"type":"session","version":1,"id":"resume-model","model":"deepseek-direct/configured"}` + "\n" + + `{"type":"message","role":"user","content":"` + question + `"}` + "\n" + + `{"type":"error","error":{"door":"taken over","message":"turn ended: taken over"}}` + "\n" + + `{"type":"title","title":"An unanswered question"}` + "\n" + if err := os.WriteFile(path, []byte(journal), 0600); err != nil { + t.Fatal(err) + } + engine, err := session.New(session.Config{Workspace: dir, SessionFile: path, + Model: "deepseek-direct/configured", Sources: sources, System: "SYSTEM", DeterministicNames: true}) + if err != nil { + t.Fatal(err) + } + defer engine.Close() + agent := &heldResumeModelAgent{Agent: engine, engine: engine, ctx: ctx, + entered: make(chan struct{}), release: make(chan struct{}), refused: outcome == "refused"} + unblock := sync.OnceFunc(func() { close(agent.release) }) + defer unblock() + a := newApp(ctx, Options{Agent: &fakeAgent{model: "deepseek-direct/replacement"}, + ProfileDir: t.TempDir(), Workspace: dir, Sources: sources, RequireListedModel: true, + ModelsForService: func(modelsource.Connected) []Model { return []Model{{ID: "replacement"}} }, + }) + defer a.doorLine.close() + a.detachConversation() + resumeCtx, cancelResume := context.WithCancel(ctx) + defer cancelResume() + a.ctx = resumeCtx + attached := a.attachConversation(Conversation{Agent: agent, SessionFile: path, Workspace: dir, Resumed: true}, nil) + select { + case <-agent.entered: + case <-ctx.Done(): + t.Fatal("arrival never asked for the listed replacement") + } + var resume tea.Cmd + for _, msg := range runCmd(attached) { + if answer, ok := msg.(doorMsg); ok { + resume = a.doorSaid(answer) + } + } + if a.hostReplayLoading || resume == nil || pendingModelSelection(agent) == nil || + a.model != "deepseek-direct/replacement" || engine.Model() != "deepseek-direct/configured" { + t.Fatal("idle replay did not leave a resume behind the pending replacement") + } + if got := strings.TrimSpace(transcriptText(a)); got != question { + t.Fatalf("arrival replay = %q, want the one journal question", got) + } + // Fold the replay without consuming the separate selection receipt: + // completing the engine ask alone must release this command. + done := make(chan []tea.Msg, 1) + go func() { done <- runCmd(resume) }() + select { + case <-done: + t.Fatal("resume command finished before model selection") + case <-time.After(30 * time.Millisecond): + } + if agent.resumeCalls.Load() != 0 || len(completionRequests(server)) != 0 { + t.Fatal("resume entered the engine before model selection completed") + } + if outcome == "canceled" { + cancelResume() + select { + case msgs := <-done: + if len(msgs) != 0 || agent.resumeCalls.Load() != 0 || pendingModelSelection(agent) == nil { + t.Fatal("canceled wait consumed the resume opportunity or the independent selection") + } + case <-ctx.Done(): + t.Fatal("canceled resume did not leave the model wait") + } + } + unblock() + var msgs []tea.Msg + if outcome == "canceled" { + acknowledgeModelChanges(t, a) + if agent.resumeCalls.Load() != 0 || len(completionRequests(server)) != 0 { + t.Fatal("selection receipt resurrected a canceled resume") + } + a.ctx = ctx + msgs = runCmd(a.resumeStoppedTurn()) + } else { + select { + case msgs = <-done: + case <-ctx.Done(): + t.Fatal("resume did not follow the completed model ask") + } + } + if len(msgs) != 1 || agent.resumeCalls.Load() != 1 { + t.Fatalf("resume calls = %d, messages = %v", agent.resumeCalls.Load(), msgs) + } + resumed, ok := msgs[0].(resumedTurnMsg) + if !ok || resumed.ch == nil { + t.Fatalf("arrival did not start its unanswered turn: %v", msgs) + } + drainResumedModelTurn(t, ctx, resumed.ch) + acknowledgeModelChanges(t, a) + if again := runCmd(a.resumeStoppedTurn()); len(again) != 0 { + t.Fatalf("arrival resumed the same question twice: %v", again) + } + want := "replacement" + if outcome == "refused" { + want = "configured" + } + requests := completionRequests(server) + if len(requests) != 1 || completionModel(t, requests[0]) != want { + t.Fatalf("resumed provider requests = %+v, want one on %q", requests, want) + } + var said []string + saved, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + for _, line := range strings.Split(strings.TrimSpace(string(saved)), "\n") { + var entry struct { + Type, Role, Content string + Note bool + } + if err := json.Unmarshal([]byte(line), &entry); err != nil { + t.Fatal(err) + } + if entry.Type == "message" && entry.Role == "user" && !entry.Note { + said = append(said, entry.Content) + } + } + if len(said) != 1 || said[0] != question || engine.ConfiguredModel() != "deepseek-direct/configured" { + t.Fatalf("resume duplicated the journal question or saved the replacement: %q", said) + } + }) + } +} + +func drainResumedModelTurn(t *testing.T, ctx context.Context, events <-chan session.Event) { + t.Helper() + finished := false + for { + select { + case event, ok := <-events: + if !ok { + if !finished { + t.Fatal("resumed turn closed without finishing") + } + return + } + if event.Kind == session.EventError { + t.Fatalf("resumed turn failed: %+v", event) + } + finished = finished || event.Kind == session.EventTurnDone + case <-ctx.Done(): + t.Fatal("resumed turn exceeded its deadline") + } + } +}