Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions cmd/harnesscli/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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"))
Expand Down Expand Up @@ -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.
Expand Down
39 changes: 35 additions & 4 deletions cmd/harnesscli/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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")
}
Expand Down Expand Up @@ -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")
}
Expand All @@ -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)
}
}
2 changes: 1 addition & 1 deletion cmd/harnesscli/main_tui_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
34 changes: 31 additions & 3 deletions docs/logs/engineering-log.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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

Expand Down
Loading