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..2a0fa70262 --- /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", BaseURL: "http://127.0.0.1:1/v1"}) + 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/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..6c05b920bd --- /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. 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 8051237d5d..fdac87d515 100644 --- a/internal/manual/chat/models-and-cost.md +++ b/internal/manual/chat/models-and-cost.md @@ -121,13 +121,22 @@ 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. +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. While every catalog is cold or empty, no default model is displayed and a message 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. 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. 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 c4780263d3..c5242ac1fe 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 @@ -306,7 +309,9 @@ 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. 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.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..eef76957e3 --- /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", BaseURL: "http://127.0.0.1:1/v1", + 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..e9ccb04f68 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,11 @@ 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 if a.clientPool != nil { a.clientPool.setSeat(model) @@ -865,6 +876,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..0c09edc900 100644 --- a/internal/session/conversation_models.go +++ b/internal/session/conversation_models.go @@ -93,12 +93,17 @@ 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 + // 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 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..695da2a676 --- /dev/null +++ b/internal/session/model_for_now_test.go @@ -0,0 +1,246 @@ +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} { + 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/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..9bd16e29e0 100644 --- a/internal/tui3/app.go +++ b/internal/tui3/app.go @@ -1967,6 +1967,11 @@ 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 + // 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 @@ -3636,7 +3641,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 +3836,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. @@ -7472,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 @@ -7544,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 d1b9f44c49..d7124ac0c3 100644 --- a/internal/tui3/attach.go +++ b/internal/tui3/attach.go @@ -998,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 12d6aeaa7b..5674f52517 100644 --- a/internal/tui3/detach.go +++ b/internal/tui3/detach.go @@ -63,6 +63,8 @@ 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 + // 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 // conversation happened to be on screen at the time. @@ -276,17 +278,18 @@ 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, + 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 +466,7 @@ func (a *app) clearConversation() { a.forgetComposers() a.parks = nil a.parkSending = false + a.deferredModelRename = nil a.touch() } @@ -532,6 +536,10 @@ func (a *app) attachConversation(conv Conversation, side *aside) tea.Cmd { } was := a.agent a.takeUp(conv, true) + a.deferredModelRename = nil + if side != nil { + a.deferredModelRename, side.deferredModelRename = side.deferredModelRename, nil + } // ANOTHER CONVERSATION'S QUESTIONS DO NOT COME ALONG ([app.forgetQuestions]); // the new lane below replays its own. if a.agent != was { @@ -550,7 +558,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 +678,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/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 5db80bec5e..1b900958b5 100644 --- a/internal/tui3/hostreplay.go +++ b/internal/tui3/hostreplay.go @@ -79,6 +79,9 @@ func (a *app) finishHostedReplay() tea.Cmd { } } if !a.hostReplayLoading { + if a.deferredModelRename != nil { + a.ensureAvailableModel() + } deferred := a.hostDeferred a.hostDeferred = nil for _, start := range deferred { diff --git a/internal/tui3/model_change_test.go b/internal/tui3/model_change_test.go new file mode 100644 index 0000000000..49fdf2a602 --- /dev/null +++ b/internal/tui3/model_change_test.go @@ -0,0 +1,303 @@ +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" +) + +type heldModelAgent struct { + *modelForNowAgent + entered chan struct{} + release chan struct{} + once sync.Once + err error + atSubmit chan string +} + +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) +} + +func (a *heldModelAgent) Submit(ctx context.Context, text string) (<-chan session.Event, error) { + a.atSubmit <- a.Model() + return a.modelForNowAgent.Submit(ctx, text) +} + +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{}), 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, + 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 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) + } +} + +// 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) + } +} + +// 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: 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) { + a, agent, unblock := modelOrderingApp(t, false) + 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" + } + unblock() + acknowledgeModelChanges(t, a) + 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: + case <-time.After(2 * time.Second): + return + } + a.fakeAgent.SetModel(id) +} + +// 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.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) + } + acknowledgeModelChanges(t, a) + 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 new file mode 100644 index 0000000000..c196f9540f --- /dev/null +++ b/internal/tui3/model_for_now.go @@ -0,0 +1,99 @@ +package tui3 + +import ( + "strings" + + tea "charm.land/bubbletea/v2" + "github.com/Agent-Field/codeaf/internal/session" +) + +const modelChangeRefusedWord = "could not change model · " + +// 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) { + a.queueModelChange(id, window, true, true, nil) +} + +func (a *app) takeModelCommands() tea.Cmd { + cmds := a.modelCommands + a.modelCommands = nil + return tea.Batch(cmds...) +} + +// 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 + remember := "" + if !quiet { + remember = strings.TrimSpace(id) + } + if rename != nil { + remember = rename.save + } + 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 { + 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 after leaving the conversation. + if err == nil && save != nil && remember != "" { + _ = save(remember) + } + return func(here bool) tea.Cmd { + if err != nil && here && a.agent == agent { + a.note(modelChangeRefusedWord + err.Error()) + } + 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_for_now_test.go b/internal/tui3/model_for_now_test.go new file mode 100644 index 0000000000..d8fa63e757 --- /dev/null +++ b/internal/tui3/model_for_now_test.go @@ -0,0 +1,286 @@ +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 + submitted []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) +} + +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()) + 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) }, + }) + 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) + } + 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() + 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) + } + 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") + 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") + } + 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 }, + }) + 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" { + 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) + acknowledgeModelChanges(t, a) + 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") + } +} + +// 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) + } + }) + } +} + +// 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.requireListedModel = false + a.useModelForNow(want, 0) + acknowledgeModelChanges(t, a) + a.requireListedModel = true + } + 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") + 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") + } + 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/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 new file mode 100644 index 0000000000..ed125e89db --- /dev/null +++ b/internal/tui3/model_park_test.go @@ -0,0 +1,202 @@ +package tui3 + +import ( + "context" + "os" + "path/filepath" + "reflect" + "strings" + "testing" + + 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.atSubmit <- a.Model() + a.submitted = append(a.submitted, a.Model()) + return a.fakeAgent.SubmitImage(ctx, text, images) +} + +// 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 replayModelAgent struct{ *heldModelAgent } + +func (a *replayModelAgent) AttachReplay() ([]session.DisplayEntry, <-chan session.Event, func()) { + return nil, nil, func() {} +} + +// 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) { + 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") + } + 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 { + want = "vendor/replacement" + } + 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") + } + }) + } +} + +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() + 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.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") + } +} + +// 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.go b/internal/tui3/model_rename.go new file mode 100644 index 0000000000..be44f4c484 --- /dev/null +++ b/internal/tui3/model_rename.go @@ -0,0 +1,99 @@ +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 unless a later choice supersedes it. +type modelRename struct { + owner Agent + configured string + running string + models *session.ConversationModels + 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 && !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 + } + 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{owner: owner, configured: original, running: running, models: models, 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) { + 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..2958af96d3 --- /dev/null +++ b/internal/tui3/model_rename_owner_test.go @@ -0,0 +1,146 @@ +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 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") + 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 + } + 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) + 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() + 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) + 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") + } + 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() + }) + } +} + +// 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) { + 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/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 7905a60ce0..37238ceaa2 100644 --- a/internal/tui3/modeldefault.go +++ b/internal/tui3/modeldefault.go @@ -37,11 +37,29 @@ 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. +// door, and the empty-catalog checks keep that old preference from being used. func (a *app) ensureAvailableModel() bool { - if !a.requireListedModel || a.agent == nil { + if rename := a.deferredModelRename; rename != nil { + // Relabel only at the owning conversation's turn/replay boundary. + if a.state != stateWorking && !a.hostReplayLoading { + a.deferredModelRename = nil + a.applyModelRename(rename) + } + } + 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 + } + } + if !a.requireListedModel { return true } // An answering request keeps its model until it settles. The turn-end path @@ -49,22 +67,21 @@ func (a *app) ensureAvailableModel() bool { if a.state == stateWorking { return a.model != "" } - preferred := a.model - if preferred == "" { - preferred = a.agent.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 { a.model, a.ctxWindow = "", 0 a.hudStale = true return false } - if model.ID != a.model { - wasSwitching := a.creditSwitching - a.creditSwitching = true - a.switchModel(model.ID, model.ContextLength) - a.creditSwitching = wasSwitching + if model.ID != a.model || (pending == nil && model.ID != a.agent.Model()) { + a.useModelForNow(model.ID, model.ContextLength) a.hudStale = true } - return true + return a.model == model.ID } diff --git a/internal/tui3/modeldefault_test.go b/internal/tui3/modeldefault_test.go index 22b9fbf800..06a315500d 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()) } @@ -144,6 +147,7 @@ func TestAnAvailableOpeningModelIsTheModelSentToItsRealProvider(t *testing.T) { 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" { @@ -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 182d5274a4..e6c3318392 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 { @@ -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 "" } @@ -1112,7 +1115,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 @@ -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 @@ -1172,7 +1179,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 +1310,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/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/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..e8967e69b3 100644 --- a/internal/tui3/palette.go +++ b/internal/tui3/palette.go @@ -3106,11 +3106,29 @@ 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.pendingModelSelection != nil { + select { + case <-a.pendingModelSelection: + default: + a.queueModelChange(id, window, false, a.creditSwitching, nil) + return + } + } a.agent.SetModel(id) - a.model = a.agent.Model() - if a.model == "" { - a.model = id + a.modelSwitched(id, window, true) +} + +func (a *app) modelSwitched(id string, window int, remember bool) { + 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). @@ -3125,7 +3143,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) @@ -3207,7 +3227,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) 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..672579c5b4 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,24 @@ 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 { + err = waitModelSelection(context.Background(), conv.Agent) + } 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 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") + } + } +}