diff --git a/cmd/harnesscli/main.go b/cmd/harnesscli/main.go index 5b9c3dcc..6518f280 100644 --- a/cmd/harnesscli/main.go +++ b/cmd/harnesscli/main.go @@ -176,7 +176,7 @@ func run(args []string) int { workspacePath := resolveWorkspacePath(*workspace) if *enableTUI { - if err := runTUI(*baseURL, workspacePath, *resume, *planMode); err != nil { + if err := runTUI(*baseURL, workspacePath, *resume, *model, *planMode); err != nil { fmt.Fprintf(stderr, "harnesscli: tui: %v\n", err) return exitClientError } @@ -535,7 +535,13 @@ func resolveWorkspacePath(workspace string) string { return workspacePath } -func newTUIConfig(baseURL, workspace, resumeConversationID string) tui.TUIConfig { +// newTUIConfig assembles the TUI's configuration. +// +// model is the explicitly requested model (the -model flag) and may be empty. +// Empty is meaningful: it lets the model remembered from the last session +// (issue #1424) apply, and failing that the daemon default. A non-empty value +// outranks both — a flag is an instruction, not a preference. Issue #1426. +func newTUIConfig(baseURL, workspace, resumeConversationID, model string) tui.TUIConfig { // Default to auto-detection; HARNESS_COLOR_PROFILE overrides (truecolor, 256, // ansi, none). Resolved and applied in runTUI before the program starts. colorProfile := strings.TrimSpace(os.Getenv("HARNESS_COLOR_PROFILE")) @@ -568,14 +574,15 @@ func newTUIConfig(baseURL, workspace, resumeConversationID string) tui.TUIConfig ResumeConversationID: resumeConversationID, APIKey: apiKey, Theme: themeName, + Model: model, } } -func runTUI(baseURL, workspace, resumeConversationID string, planMode bool) error { +func runTUI(baseURL, workspace, resumeConversationID, model string, planMode bool) error { if !term.IsTerminal(int(os.Stdout.Fd())) { return fmt.Errorf("--tui requires a terminal; pipe output or use without --tui for streaming mode") } - tuiCfg := newTUIConfig(baseURL, workspace, resumeConversationID) + tuiCfg := newTUIConfig(baseURL, workspace, resumeConversationID, model) tuiCfg.PlanMode = planMode // Resolve and apply the color profile to the renderer before building the // model, and store the effective profile back for accurate display. diff --git a/cmd/harnesscli/main_test.go b/cmd/harnesscli/main_test.go index ce7b441b..e53fba7e 100644 --- a/cmd/harnesscli/main_test.go +++ b/cmd/harnesscli/main_test.go @@ -184,7 +184,7 @@ func TestRunCreatesAndStreamsToCompletion(t *testing.T) { } func TestNewTUIConfigIncludesWorkspace(t *testing.T) { - cfg := newTUIConfig("http://127.0.0.1:8080", "/tmp/tui-project", "") + cfg := newTUIConfig("http://127.0.0.1:8080", "/tmp/tui-project", "", "") if cfg.BaseURL != "http://127.0.0.1:8080" { t.Fatalf("BaseURL = %q", cfg.BaseURL) } @@ -200,7 +200,7 @@ func TestNewTUIConfigIncludesWorkspace(t *testing.T) { } func TestNewTUIConfigIncludesResumeConversationID(t *testing.T) { - cfg := newTUIConfig("http://127.0.0.1:8080", "/tmp/tui-project", "conv-resume-42") + cfg := newTUIConfig("http://127.0.0.1:8080", "/tmp/tui-project", "conv-resume-42", "") if cfg.ResumeConversationID != "conv-resume-42" { t.Fatalf("ResumeConversationID = %q, want %q", cfg.ResumeConversationID, "conv-resume-42") } @@ -328,7 +328,7 @@ func TestNewTUIConfigLoadsSavedTheme(t *testing.T) { t.Fatalf("config.Save: %v", err) } - cfg := newTUIConfig("http://127.0.0.1:8080", "/tmp/tui-project", "") + cfg := newTUIConfig("http://127.0.0.1:8080", "/tmp/tui-project", "", "") if cfg.Theme != "ocean" { t.Errorf("TUIConfig.Theme = %q, want %q", cfg.Theme, "ocean") } @@ -339,8 +339,39 @@ func TestNewTUIConfigLoadsSavedTheme(t *testing.T) { func TestNewTUIConfigNoSavedThemeIsEmpty(t *testing.T) { t.Setenv("HOME", t.TempDir()) - cfg := newTUIConfig("http://127.0.0.1:8080", "/tmp/tui-project", "") + cfg := newTUIConfig("http://127.0.0.1:8080", "/tmp/tui-project", "", "") if cfg.Theme != "" { t.Errorf("TUIConfig.Theme = %q, want empty", cfg.Theme) } } + +// TestNewTUIConfigCarriesExplicitModel pins issue #1426: an explicitly +// requested model must reach the TUI. +// +// `harnesscli --tui -model X` parsed the flag and threw it away, so the TUI +// started on the daemon default or — after #1424 — on the remembered model, +// which is precisely the thing a user passing -model is trying to override. +// +// runTUI itself cannot be unit tested (it requires a terminal), so this is the +// closest honest seam: the point where the flag's value becomes TUIConfig.Model. +func TestNewTUIConfigCarriesExplicitModel(t *testing.T) { + cfg := newTUIConfig("http://127.0.0.1:8080", "/tmp/tui-project", "", "gpt-4.1-mini") + + if cfg.Model != "gpt-4.1-mini" { + t.Fatalf("TUIConfig.Model = %q, want %q; an explicitly requested model must reach the TUI", + cfg.Model, "gpt-4.1-mini") + } +} + +// TestNewTUIConfigWithoutModelLeavesItEmpty is the control. Empty must stay +// empty: it is what lets the remembered model (#1424) and then the daemon +// default apply. A fix that defaulted to some model would satisfy the test +// above and silently break both. +func TestNewTUIConfigWithoutModelLeavesItEmpty(t *testing.T) { + cfg := newTUIConfig("http://127.0.0.1:8080", "/tmp/tui-project", "", "") + + if cfg.Model != "" { + t.Fatalf("TUIConfig.Model = %q, want empty so the remembered model and daemon default still apply", + cfg.Model) + } +} diff --git a/cmd/harnesscli/main_tui_test.go b/cmd/harnesscli/main_tui_test.go index a415f7f8..bc42ede7 100644 --- a/cmd/harnesscli/main_tui_test.go +++ b/cmd/harnesscli/main_tui_test.go @@ -13,7 +13,7 @@ func TestRunTUIRequiresTerminal(t *testing.T) { t.Skip("stdout is a terminal in this environment") } - err := runTUI("http://localhost:8080", "/tmp/project", "", false) + err := runTUI("http://localhost:8080", "/tmp/project", "", "", false) if err == nil { t.Fatal("expected non-terminal runTUI call to fail") } diff --git a/docs/logs/engineering-log.md b/docs/logs/engineering-log.md index e7342a13..479d09d1 100644 --- a/docs/logs/engineering-log.md +++ b/docs/logs/engineering-log.md @@ -1,5 +1,33 @@ # Engineering Log +## 2026-09-08 — Issue #1426 --tui -model reaches the TUI + +- Symptom: `harnesscli --tui -model X` parsed the flag and threw it away. The + TUI started on the daemon default or, after #1424, on the remembered model — + which is precisely what a user passing `-model` is trying to override. No + warning, no error: the flag was silently discarded. +- Cause: `main.go:179` called `runTUI(*baseURL, workspacePath, *resume, + *planMode)`. `*model` was never among the arguments, and `newTUIConfig` never + set `TUIConfig.Model`. The non-TUI path (`main.go:216`) passed the flag + correctly all along, so the defect was confined to the TUI branch. +- Fix: `model` threaded from the dispatch through `runTUI` into + `newTUIConfig`, following the route `planMode` already takes. Nothing in the + TUI changed — `selectedModel: cfg.Model` and #1424's `cfg.Model == ""` guard + were already correct and simply had no producer of a non-empty value. +- Precedence, now real: flag, then the model remembered from last session, then + the daemon default. The flag deliberately does **not** write to + `~/.config/harnesscli/config.json`: a flag is a one-off instruction, not a + preference, and silently rewriting the saved model would be a worse bug than + the one being fixed. +- Tests: `TestNewTUIConfigCarriesExplicitModel` and, as its control, + `TestNewTUIConfigWithoutModelLeavesItEmpty` — empty must stay empty, or a fix + that defaulted to some model would pass the first test while breaking #1424. + `runTUI` itself requires a terminal and cannot be unit tested, so + `newTUIConfig` is the closest honest seam; the consumer half is already + covered by `TestExplicitModelBeatsRememberedModel` in the `tui` package. + Neither test alone proves the flag works — producer and consumer are pinned + separately and meet only in the live check. + ## 2026-09-08 — Issue #1424 remember the last used model - Symptom: the TUI forgot the model you picked. `/model`, quit, restart, and you @@ -36,9 +64,9 @@ test suite into something that mutates the developer's machine. When making anything persistent, check what the tests write before checking what the feature reads. -- Noted, not fixed: `runTUI` never receives the `-model` flag, so - `harnesscli --tui -model X` ignores it today. Separate defect; the precedence - guard above is written so wiring it needs no further change here. +- Noted, not fixed here: `runTUI` never receives the `-model` flag, so + `harnesscli --tui -model X` ignored it. Filed as #1426 and fixed there; the + precedence guard above needed no change, exactly as predicted. ## 2026-09-08 — Issue #1422 interrupt test flake, and a diagnosis that was wrong twice