diff --git a/cmd/ggcode/plain.go b/cmd/ggcode/plain.go new file mode 100644 index 000000000..7e6153ef1 --- /dev/null +++ b/cmd/ggcode/plain.go @@ -0,0 +1,661 @@ +package main + +// Linear frontend for dumb/non-interactive terminals (#4604 Track 1). +// +// The interactive frontend is a Bubble Tea program: alternate-screen raw +// mode with high-frequency frame refresh. On terminals that cannot carry +// those escape sequences (TERM=dumb, screen sessions, limited SSH) that is +// exactly the garbled-output / repeated-warning / junk-character failure the +// issue reports, and --plain alone only stripped COLOR while still running +// the full TUI. +// +// This frontend runs NO Bubble Tea at all: a plain line-buffered stdin loop +// (no raw mode - dumb terminals are line-mode by nature) plus linear +// streaming print. It deliberately rides the SAME interactive runtime core +// the TUI uses (same agent, registry, session, config hot-swap), so every +// capability except the visual panels keeps working. +// +// Interactive-shaped tools degrade explicitly: +// - ask_user is unregistered: there is no questionnaire UI to host it; +// the agent instead ends its turn and the user answers by typing the +// next line (multi-turn conversation is the native pause point). +// - approvals default to Allow (GGCODE_PLAIN_APPROVE=deny switches to +// Deny for read-only-style runs); there is no approval dialog either. + +import ( + "bufio" + "context" + "flag" + "fmt" + "io" + "os" + "sort" + "strings" + + "github.com/topcheer/ggcode/internal/agent" + "github.com/topcheer/ggcode/internal/agentruntime" + "github.com/topcheer/ggcode/internal/config" + "github.com/topcheer/ggcode/internal/debug" + "github.com/topcheer/ggcode/internal/permission" + "github.com/topcheer/ggcode/internal/provider" + "github.com/topcheer/ggcode/internal/session" + "github.com/topcheer/ggcode/internal/tool" + "github.com/topcheer/ggcode/internal/tui" +) + +// isDumbTerminal reports whether the environment cannot host the full TUI: +// TERM=dumb (or unset TERM on a tty). The check is centralized here so the +// piecemeal probes scattered across the TUI package (e.g. model.go's +// terminal-title writer) can stay point fixes while entry-level routing +// lives at the frontend fork itself. +func isDumbTerminal() bool { + term := strings.TrimSpace(os.Getenv("TERM")) + return term == "" || strings.EqualFold(term, "dumb") +} + +// plainApproval is the linear frontend's interactive-approval stand-in: +// default Accept, configurable to Deny via GGCODE_PLAIN_APPROVE=deny. +func plainApproval(_ context.Context, toolName, _ string) permission.Decision { + if strings.EqualFold(strings.TrimSpace(os.Getenv("GGCODE_PLAIN_APPROVE")), "deny") { + debug.Log("plain", "approval denied (GGCODE_PLAIN_APPROVE=deny): tool=%s", toolName) + return permission.Deny + } + return permission.Allow +} + +// RunLinearREPL drives the interactive agent from a plain line-oriented +// loop. It returns a process exit code. +func RunLinearREPL(ag *agent.Agent, registry *tool.Registry, cfg *config.Config, cfgPath string, startupNotices []string) int { + _ = cfgPath // cfg.Save() persists to the scope the config was loaded from + plainLang = plainLangFromConfig(cfgLang(cfg)) + + // No questionnaire UI: the agent ends its turn and waits for the next + // typed line instead of calling ask_user (#4604 design decision). + if registry != nil && registry.Unregister("ask_user") { + fmt.Println(plainTr(plainLang, "askuser.off")) + } + if ag != nil { + ag.SetApprovalHandler(plainApproval) + // The linear REPL is hand-driven line by line: opt out of the + // cruise (autopilot/bypass) BEHAVIORAL injections - the + // goal-collection meta-instruction and the end-of-turn strategist + // prompt that would restart the loop after every plain answer. + // The permission surface itself is untouched (still bypass if + // configured); plainApproval remains the approval decision point. + ag.SuppressAutopilotInjection() + } + + // --persist-session: append every message to the session store + // (write-only by design): the JSONL lands in the session dir for + // explicit --resume , but is deliberately NOT indexed (store.Save) + // so auto-resume and the picker never pick it up - the next plain + // start must stay a fresh session. + if persistSessionFlag { + wirePlainPersist(ag, cfg) + } + + fmt.Println(plainTr(plainLang, "banner")) + for _, n := range startupNotices { + fmt.Println(plainTr(plainLang, "notice"), n) + } + + sc := bufio.NewScanner(os.Stdin) + sc.Buffer(make([]byte, 64*1024), 1024*1024) + ctx := context.Background() + for { + fmt.Print(plainTr(plainLang, "prompt")) + if !sc.Scan() { + fmt.Println() + return 0 + } + line := strings.TrimSpace(sc.Text()) + switch { + case line == "": + continue + case strings.HasPrefix(line, "/"): + if exit := handlePlainCommand(line, ag, cfg); exit { + return 0 + } + default: + runLinearTurn(ctx, ag, line) + } + } +} + +// handlePlainCommand executes the linear command subset. It returns true +// when the REPL should exit. +func handlePlainCommand(line string, ag *agent.Agent, cfg *config.Config) bool { + fields := strings.Fields(line) + cmd := strings.ToLower(fields[0]) + args := fields[1:] + switch cmd { + case "/exit", "/quit", "/q": + return true + case "/help", "/?": + fmt.Println(plainTr(plainLang, "help.title")) + for _, k := range []string{"help.status", "help.model", "help.provider", "help.plist", "help.padd", "help.endpoint", "help.emodels", "help.erefresh", "help.eadd", "help.ekey", "help.lang", "help.exit"} { + fmt.Println(plainTr(plainLang, k)) + } + fmt.Println(plainTr(plainLang, "help.tail")) + fmt.Println(plainTr(plainLang, "help.tail2")) + case "/status": + if cfg == nil { + fmt.Println(plainTr(plainLang, "cfg.unavailable")) + return false + } + if resolved, _, err := agentruntime.ResolveCurrentSelection(cfg); err != nil { + fmt.Println(plainTr(plainLang, "resolve.failed"), err) + } else { + fmt.Printf("vendor=%s endpoint=%s model=%s\n", resolved.VendorID, resolved.EndpointID, resolved.Model) + } + case "/model": + if len(args) != 1 { + fmt.Println(plainTr(plainLang, "usage.model")) + return false + } + switchPlainSelection(ag, cfg, "", "", args[0]) + case "/provider": + handlePlainProvider(args, ag, cfg) + case "/endpoint": + handlePlainEndpoint(args, ag, cfg) + case "/lang": + if len(args) != 1 { + fmt.Println(plainTr(plainLang, "lang.usage")) + return false + } + next := tui.NormalizeLanguage(args[0]) + if next != tui.LangEnglish && next != tui.LangZhCN { + fmt.Println(plainTr(plainLang, "lang.unknown"), args[0]) + return false + } + plainLang = next + if cfg != nil { + cfg.Language = string(next) + if err := cfg.Save(); err != nil { + fmt.Println(plainTr(plainLang, "lang.nopersist"), err) + return false + } + } + fmt.Printf(plainTr(plainLang, "lang.switched")+"\n", next) + default: + fmt.Println(plainTr(plainLang, "cmd.unknown"), cmd) + } + return false +} + +// switchPlainSelection applies a vendor/endpoint/model switch through the +// same ActivateCurrentSelection + ApplyProviderToAgent funnel the TUI, IM, +// and daemon switchers use (empty fields inherit the current selection), +// then persists the config so the switch survives restarts. +func switchPlainSelection(ag *agent.Agent, cfg *config.Config, vendor, endpoint, model string) { + if cfg == nil { + fmt.Println(plainTr(plainLang, "cfg.unavailable")) + return + } + resolved, prov, err := agentruntime.ActivateCurrentSelection(cfg, vendor, endpoint, model) + if err != nil { + fmt.Println(plainTr(plainLang, "switch.failed"), err) + return + } + if ag != nil { + agentruntime.ApplyProviderToAgent(ag, prov, resolved) + } + if err := cfg.Save(); err != nil { + fmt.Println(plainTr(plainLang, "switch.nopersist"), err) + return + } + fmt.Printf("[plain] now on vendor=%s endpoint=%s model=%s\n", resolved.VendorID, resolved.EndpointID, resolved.Model) +} + +// plainListMaxModels caps the per-endpoint model listing in /provider --list. +const plainListMaxModels = 10 + +// newPlainFlagSet builds a silent FlagSet (usage/errors are handled by the +// caller, not dumped to stderr mid-REPL). +func newPlainFlagSet(name string) *flag.FlagSet { + fs := flag.NewFlagSet(name, flag.ContinueOnError) + fs.SetOutput(io.Discard) + return fs +} + +// plainArgsAreFlags reports whether the arg list starts with a "--" option, +// i.e. selects the subcommand mode instead of the positional switch form. +func plainArgsAreFlags(args []string) bool { + return len(args) > 0 && strings.HasPrefix(args[0], "-") +} + +// handlePlainProvider implements /provider: positional switch form plus the +// --list / --add-endpoint subcommands. +func handlePlainProvider(args []string, ag *agent.Agent, cfg *config.Config) { + if !plainArgsAreFlags(args) { + if len(args) < 1 || len(args) > 2 { + fmt.Println(plainTr(plainLang, "usage.provider")) + return + } + ep := "" + if len(args) == 2 { + ep = args[1] + } + switchPlainSelection(ag, cfg, args[0], ep, "") + return + } + mode := args[0] + rest := args[1:] + switch mode { + case "--list": + fs := newPlainFlagSet("provider-list") + all := fs.Bool("all", false, "include endpoints without api key or models") + if err := fs.Parse(rest); err != nil { + fmt.Println("[plain] bad --list flags:", err) + return + } + plainListEndpoints(cfg, *all) + case "--add-endpoint": + fs := newPlainFlagSet("provider-add-endpoint") + name := fs.String("endpoint-name", "", "endpoint id (unique within the vendor)") + proto := fs.String("endpoint-type", "", "protocol: openai|openai-responses|anthropic|gemini|copilot") + url := fs.String("endpoint-url", "", "base URL") + model := fs.String("endpoint-model", "", "initial/default model") + ctxWin := fs.Int("context-window", 0, "context window in tokens") + maxOut := fs.Int("max-output", 0, "max output tokens") + apikey := fs.String("endpoint-apikey", "", "API key for the endpoint") + if err := fs.Parse(rest); err != nil { + fmt.Println("[plain] bad --add-endpoint flags:", err) + return + } + plainAddEndpoint(cfg, *name, *proto, *url, *model, *ctxWin, *maxOut, *apikey) + default: + fmt.Println(plainTr(plainLang, "opt.unknown"), mode) + } +} + +// plainListEndpoints prints every configured endpoint grouped by vendor. +// Default view: endpoints with a usable API key AND a non-empty model list; +// --all includes the rest, annotated. Up to 10 models per endpoint. +func plainListEndpoints(cfg *config.Config, all bool) { + if cfg == nil { + fmt.Println(plainTr(plainLang, "cfg.unavailable")) + return + } + vendors := make([]string, 0, len(cfg.Vendors)) + for id := range cfg.Vendors { + vendors = append(vendors, id) + } + sort.Strings(vendors) + for _, vid := range vendors { + v := cfg.Vendors[vid] + eps := make([]string, 0, len(v.Endpoints)) + for id := range v.Endpoints { + eps = append(eps, id) + } + sort.Strings(eps) + shown := 0 + for _, eid := range eps { + ep := v.Endpoints[eid] + keyed := config.ExpandEnv(v.APIKey) != "" || config.ExpandEnv(ep.APIKey) != "" + modelled := len(ep.Models) > 0 + if !all && !(keyed && modelled) { + continue + } + if shown == 0 { + fmt.Printf(plainTr(plainLang, "list.vendor")+"\n", vid) + } + shown++ + mark := " " + if vid == cfg.Vendor && eid == cfg.Endpoint { + mark = "* " + } + note := "" + if !keyed { + note = " (no api key)" + } else if !modelled { + note = " (no models configured)" + } + fmt.Printf("%s%s%s\n", mark, eid, note) + if modelled { + shownModels := ep.Models + extra := 0 + if len(shownModels) > plainListMaxModels { + extra = len(shownModels) - plainListMaxModels + shownModels = shownModels[:plainListMaxModels] + } + for i, m := range shownModels { + prefix := " - " + if m == ep.SelectedModel || (ep.SelectedModel == "" && m == ep.DefaultModel) { + prefix = " * " + } + _ = i + fmt.Printf("%s%s\n", prefix, m) + } + if extra > 0 { + fmt.Printf(" (+%d more - use /endpoint --show-models)\n", extra) + } + } + } + if shown == 0 && all { + fmt.Printf(plainTr(plainLang, "list.noendpoints")+"\n", vid) + } + } +} + +// plainAddEndpoint appends a new endpoint to the ACTIVE vendor's map and +// persists. Refuses to overwrite an existing endpoint id. +func plainAddEndpoint(cfg *config.Config, name, proto, baseURL, model string, ctxWin, maxOut int, apikey string) { + if cfg == nil { + fmt.Println(plainTr(plainLang, "cfg.unavailable")) + return + } + if name == "" || proto == "" || baseURL == "" { + fmt.Println("[plain] --endpoint-name/--endpoint-type/--endpoint-url are required") + return + } + switch proto { + case "openai", "openai-responses", "anthropic", "gemini", "copilot": + default: + fmt.Printf("[plain] unsupported endpoint-type %q (want openai|openai-responses|anthropic|gemini|copilot)\n", proto) + return + } + if cfg.Vendor == "" { + fmt.Println("[plain] no active vendor to add the endpoint to") + return + } + v := cfg.Vendors[cfg.Vendor] + // Lazy-init the endpoints map for a vendor declared without any. + if v.Endpoints == nil { + v.Endpoints = make(map[string]config.EndpointConfig) + } + if _, exists := v.Endpoints[name]; exists { + fmt.Printf("[plain] endpoint %q already exists on vendor %q\n", name, cfg.Vendor) + return + } + ep := config.EndpointConfig{ + Protocol: proto, + BaseURL: baseURL, + APIKey: apikey, + ContextWindow: ctxWin, + MaxTokens: maxOut, + DefaultModel: model, + } + if model != "" { + ep.Models = []string{model} + ep.SelectedModel = model + } + v.Endpoints[name] = ep + cfg.Vendors[cfg.Vendor] = v + if err := cfg.Save(); err != nil { + fmt.Println("[plain] endpoint added but not persisted:", err) + return + } + fmt.Printf("[plain] endpoint %q added to vendor %q%s\n", name, cfg.Vendor, switchHint(model)) +} + +func switchHint(model string) string { + if model != "" { + return " - switch with: /endpoint " + model + } + return "" +} + +// handlePlainEndpoint implements /endpoint: positional switch form plus the +// --show-models / --refresh-models / --add-model subcommands. +func handlePlainEndpoint(args []string, ag *agent.Agent, cfg *config.Config) { + if !plainArgsAreFlags(args) { + if len(args) != 1 { + fmt.Println(plainTr(plainLang, "usage.endpoint")) + return + } + switchPlainSelection(ag, cfg, "", args[0], "") + return + } + mode := args[0] + rest := args[1:] + switch mode { + case "--show-models": + plainShowModels(cfg, false) + case "--refresh-models": + plainShowModels(cfg, true) + case "--add-model": + fs := newPlainFlagSet("endpoint-add-model") + model := fs.String("model", "", "model id to add") + apikey := fs.String("apikey", "", "optional: update the endpoint API key too") + if err := fs.Parse(rest); err != nil { + fmt.Println("[plain] bad --add-model flags:", err) + return + } + plainAddModel(cfg, *model, *apikey) + case "--set-apikey": + fs := newPlainFlagSet("endpoint-set-apikey") + apikey := fs.String("apikey", "", "new API key for the active endpoint") + if err := fs.Parse(rest); err != nil { + fmt.Println("[plain] bad --set-apikey flags:", err) + return + } + plainSetEndpointAPIKey(cfg, *apikey) + default: + fmt.Println(plainTr(plainLang, "opt.unknown.ep"), mode) + } +} + +// plainShowModels lists every model on the ACTIVE endpoint: the configured +// static list merged with live discovery (cache-aware; force refetches). +func plainShowModels(cfg *config.Config, force bool) { + if cfg == nil { + fmt.Println(plainTr(plainLang, "cfg.unavailable")) + return + } + resolved, _, err := agentruntime.ResolveCurrentSelection(cfg) + if err != nil { + fmt.Println("[plain] resolve failed:", err) + return + } + var discovered []string + if force { + discovered, err = provider.ForceDiscoverModels(context.Background(), resolved) + } else { + discovered, err = provider.DiscoverModels(context.Background(), resolved) + } + if err != nil { + fmt.Println("[plain] model discovery unavailable (showing configured list):", err) + } + // Merge: static config first (user intent), then discovery-only entries. + seen := make(map[string]bool) + var epModels []string + if v, ok := cfg.Vendors[resolved.VendorID]; ok { + if ep, ok := v.Endpoints[resolved.EndpointID]; ok { + epModels = ep.Models + } + } + fmt.Printf(plainTr(plainLang, "models.title")+"\n", resolved.VendorID, resolved.EndpointID) + for _, m := range epModels { + if !seen[m] { + seen[m] = true + fmt.Println(" -", m) + } + } + for _, m := range discovered { + if !seen[m] { + seen[m] = true + fmt.Println(" -", m, "(discovered)") + } + } + if len(seen) == 0 { + fmt.Println(" (none - use /endpoint --add-model --model )") + } +} + +// plainSetEndpointAPIKey overwrites the ACTIVE endpoint's API key and +// persists. Vendor-level keys are left untouched (endpoint key wins during +// resolution when present). +func plainSetEndpointAPIKey(cfg *config.Config, apikey string) { + if cfg == nil { + fmt.Println(plainTr(plainLang, "cfg.unavailable")) + return + } + if apikey == "" { + fmt.Println("[plain] --apikey is required") + return + } + if cfg.Vendor == "" || cfg.Endpoint == "" { + fmt.Println("[plain] no active endpoint to set the key on") + return + } + v := cfg.Vendors[cfg.Vendor] + ep, ok := v.Endpoints[cfg.Endpoint] + if !ok { + fmt.Printf("[plain] active endpoint %q not found on vendor %q\n", cfg.Endpoint, cfg.Vendor) + return + } + ep.APIKey = apikey + v.Endpoints[cfg.Endpoint] = ep + cfg.Vendors[cfg.Vendor] = v + if err := cfg.Save(); err != nil { + fmt.Println("[plain] api key set but not persisted:", err) + return + } + fmt.Printf("[plain] api key set on endpoint %q\n", cfg.Endpoint) +} + +// plainAddModel appends a model to the ACTIVE endpoint's static list and +// persists (optionally updating the endpoint API key). Unlike +// /provider --add-endpoint this never creates an endpoint - the active one +// is the target by definition. +func plainAddModel(cfg *config.Config, model, apikey string) { + if cfg == nil { + fmt.Println(plainTr(plainLang, "cfg.unavailable")) + return + } + if model == "" { + fmt.Println("[plain] --model is required") + return + } + if cfg.Vendor == "" || cfg.Endpoint == "" { + fmt.Println("[plain] no active endpoint to add the model to") + return + } + v := cfg.Vendors[cfg.Vendor] + ep, ok := v.Endpoints[cfg.Endpoint] + if !ok { + fmt.Printf("[plain] active endpoint %q not found on vendor %q\n", cfg.Endpoint, cfg.Vendor) + return + } + for _, m := range ep.Models { + if m == model { + fmt.Printf("[plain] model %q already on endpoint %q\n", model, cfg.Endpoint) + return + } + } + ep.Models = append(ep.Models, model) + if ep.DefaultModel == "" { + ep.DefaultModel = model + } + if apikey != "" { + ep.APIKey = apikey + } + v.Endpoints[cfg.Endpoint] = ep + cfg.Vendors[cfg.Vendor] = v + if err := cfg.Save(); err != nil { + fmt.Println("[plain] model added but not persisted:", err) + return + } + fmt.Printf("[plain] model %q added to endpoint %q - switch with: /model %s\n", model, cfg.Endpoint, model) +} + +// wirePlainPersist attaches a per-message persistence handler to the plain +// agent, mirroring the TUI's SetPersistHandler wiring (async JSONL append on +// every context Add). It returns the store and the fresh session; nils when +// anything is unavailable (agent nil, store failure) - persistence is +// best-effort and must never block the REPL. +func wirePlainPersist(ag *agent.Agent, cfg *config.Config) (*session.JSONLStore, *session.Session) { + if ag == nil { + return nil, nil + } + dir, err := session.DefaultDir() + if err != nil { + fmt.Printf("[plain] persist unavailable: %v\n", err) + return nil, nil + } + store, err := session.NewJSONLStore(dir) + if err != nil { + fmt.Printf("[plain] persist unavailable: %v\n", err) + return nil, nil + } + v, ep, m := "unknown", "unknown", "unknown" + if resolved, _, rerr := agentruntime.ResolveCurrentSelection(cfg); rerr == nil { + v, ep, m = resolved.VendorID, resolved.EndpointID, resolved.Model + } + ses := session.NewSession(v, ep, m) + ses.Title = "plain session" + ag.SetPersistHandler(func(msg provider.Message) { + if err := store.AppendMessageToDisk(ses, msg); err != nil { + debug.Log("plain", "persist append failed: %v", err) + } + }) + fmt.Printf("[plain] persisting session %s\n", ses.ID) + return store, ses +} + +// runLinearTurn streams one agent turn as plain linear output: text deltas +// print as they arrive, tool boundaries get one bracketed line each, and +// errors surface as single lines - no refresh, no cursor movement, no +// ANSI beyond what NO_COLOR already disabled. +func runLinearTurn(ctx context.Context, ag *agent.Agent, line string) { + if ag == nil { + fmt.Println(plainTr(plainLang, "agent.unavailable")) + return + } + // pendingToolCalls tracks tool calls whose result event has not arrived + // yet. The turn boundary is RunStream returning - NOT any StreamEventDone, + // because provider streams emit Done per iteration (often with Usage) + // while tool results arrive between iterations. + pendingToolCalls := make(map[string]bool) + err := ag.RunStream(ctx, line, plainStreamPrinter(pendingToolCalls)) + plainCloseTurn(pendingToolCalls) + if err != nil { + fmt.Printf("\n[error] turn failed: %v\n", err) + } +} + +// plainStreamPrinter renders one agent turn as plain linear output. It is +// extracted from runLinearTurn so the tool-call/result pairing contract is +// unit-testable: call lines print without a newline, the matching ToolResult +// (by tool-call ID) appends " (ok)" or " (error)", unmatched results are +// dropped, and Done closes at most one unterminated tail line. +func plainStreamPrinter(pendingToolCalls map[string]bool) func(provider.StreamEvent) { + return func(event provider.StreamEvent) { + switch event.Type { + case provider.StreamEventText: + fmt.Print(event.Text) + case provider.StreamEventToolCallDone: + if event.Tool.Name != "" { + // Defer the status suffix until the matching ToolResult arrives + // (same call ID) or the turn ends without one - the linear model + // cannot rewrite the line, so "ok" is the implicit default. + fmt.Printf("\n[tool] %s", event.Tool.Name) + pendingToolCalls[event.Tool.ID] = true + } + case provider.StreamEventToolResult: + if !pendingToolCalls[event.Tool.ID] { + return // result without a printed call line (unexpected order) + } + if event.IsError { + fmt.Printf(" (error)\n") + } else { + fmt.Printf(" (ok)\n") + } + delete(pendingToolCalls, event.Tool.ID) + case provider.StreamEventError: + fmt.Printf("\n[error] %v\n", event.Error) + } + } +} + +// plainCloseTurn ends the output for one REPL turn: an unterminated tool +// line gets its closing newline (one total, not per call), then the blank +// separator line before the next prompt. +func plainCloseTurn(pendingToolCalls map[string]bool) { + if len(pendingToolCalls) > 0 { + fmt.Println() + } + clear(pendingToolCalls) + fmt.Println() +} diff --git a/cmd/ggcode/plain_cmd_test.go b/cmd/ggcode/plain_cmd_test.go new file mode 100644 index 000000000..bb6bdf5bb --- /dev/null +++ b/cmd/ggcode/plain_cmd_test.go @@ -0,0 +1,162 @@ +package main + +import ( + "io" + "os" + "strings" + "testing" + + "github.com/topcheer/ggcode/internal/config" +) + +// TestPlainArgsAreFlags verifies the subcommand-vs-positional switch routing: +// leading "--" selects the flag mode, everything else is the switch form. +func TestPlainArgsAreFlags(t *testing.T) { + cases := []struct { + args []string + want bool + }{ + {nil, false}, + {[]string{"zai"}, false}, + {[]string{"zai", "ep1"}, false}, + {[]string{"--list"}, true}, + {[]string{"--all"}, true}, + {[]string{"--show-models"}, true}, + } + for _, c := range cases { + if got := plainArgsAreFlags(c.args); got != c.want { + t.Errorf("plainArgsAreFlags(%v) = %v, want %v", c.args, got, c.want) + } + } +} + +// newPlainListFixture builds an in-memory config with three endpoints covering +// the filter classes: keyed+modelled, keyed+no-models, no-key+modelled. +func newPlainListFixture() *config.Config { + cfg := &config.Config{ + Vendor: "vkeyed", + Endpoint: "ep-both", + Vendors: map[string]config.VendorConfig{ + "vkeyed": {Endpoints: map[string]config.EndpointConfig{ + "ep-both": { + APIKey: "k", + Models: []string{"m1", "m2", "m3", "m4", "m5", "m6", "m7", "m8", "m9", "m10", "m11", "m12"}, + DefaultModel: "m1", + }, + }}, + "vnomodels": {APIKey: "k", Endpoints: map[string]config.EndpointConfig{ + "ep-nomodels": {APIKey: "k"}, + }}, + "vnokey": {Endpoints: map[string]config.EndpointConfig{ + "ep-nokey": {Models: []string{"m1"}}, + }}, + }, + } + return cfg +} + +// captureStdout runs fn with os.Stdout piped and returns what it printed. +func captureStdout(t *testing.T, fn func()) string { + t.Helper() + old := os.Stdout + r, w, err := os.Pipe() + if err != nil { + t.Skipf("pipe: %v", err) + } + os.Stdout = w + fn() + _ = w.Close() + os.Stdout = old + b, _ := io.ReadAll(r) + _ = r.Close() + return string(b) +} + +// TestPlainListEndpointsDefaultFilter verifies the default view lists only +// endpoints with BOTH a usable api key and a configured model list, caps the +// model listing at 10 with a +N more hint, and marks the active endpoint. +func TestPlainListEndpointsDefaultFilter(t *testing.T) { + cfg := newPlainListFixture() + out := captureStdout(t, func() { plainListEndpoints(cfg, false) }) + + if !strings.Contains(out, "vendor vkeyed") { + t.Error("keyed+modelled vendor should be listed") + } + if strings.Contains(out, "vnomodels") || strings.Contains(out, "ep-nomodels") { + t.Error("keyed-but-modelless endpoint must be hidden by default") + } + if strings.Contains(out, "vnokey") || strings.Contains(out, "ep-nokey") { + t.Error("keyless endpoint must be hidden by default") + } + if !strings.Contains(out, "(+2 more") { + t.Errorf("12 models should cap at 10 with '(+2 more' hint, got:\n%s", out) + } + if !strings.Contains(out, "* ep-both") { + t.Errorf("active endpoint should carry the '*' marker, got:\n%s", out) + } + if strings.Count(out, " - ")+strings.Count(out, " * ") != 10 { + t.Errorf("default view must show at most 10 models, got:\n%s", out) + } +} + +// TestPlainListEndpointsAll verifies --all surfaces the filtered-out classes +// with their annotations. +func TestPlainListEndpointsAll(t *testing.T) { + cfg := newPlainListFixture() + out := captureStdout(t, func() { plainListEndpoints(cfg, true) }) + + for _, want := range []string{"ep-both", "ep-nomodels", "(no models configured)", "ep-nokey", "(no api key)"} { + if !strings.Contains(out, want) { + t.Errorf("--all output should contain %q, got:\n%s", want, out) + } + } +} + +// TestPlainAddEndpointValidation verifies the guard rails that run BEFORE any +// config mutation: required fields, protocol whitelist, duplicate id. +func TestPlainAddEndpointValidation(t *testing.T) { + cfg := newPlainListFixture() + + out := captureStdout(t, func() { plainAddEndpoint(cfg, "", "openai", "https://x", "m", 0, 0, "") }) + if !strings.Contains(out, "required") { + t.Errorf("missing name must be rejected, got: %s", out) + } + + out = captureStdout(t, func() { plainAddEndpoint(cfg, "ep9", "grpc", "https://x", "m", 0, 0, "") }) + if !strings.Contains(out, "unsupported endpoint-type") { + t.Errorf("unknown protocol must be rejected, got: %s", out) + } + + out = captureStdout(t, func() { plainAddEndpoint(cfg, "ep-both", "openai", "https://x", "m", 0, 0, "") }) + if !strings.Contains(out, "already exists") { + t.Errorf("duplicate endpoint id must be rejected, got: %s", out) + } + + if _, ok := cfg.Vendors["vkeyed"].Endpoints["ep-both"]; !ok { + t.Fatal("fixture endpoint vanished - validation must not mutate config") + } +} + +// TestPlainAddModelValidation verifies the add-model guards: model required, +// active endpoint must exist, duplicate model refused. +func TestPlainAddModelValidation(t *testing.T) { + cfg := newPlainListFixture() + + out := captureStdout(t, func() { plainAddModel(cfg, "", "") }) + if !strings.Contains(out, "required") { + t.Errorf("empty model must be rejected, got: %s", out) + } + + out = captureStdout(t, func() { plainAddModel(cfg, "m1", "") }) + if !strings.Contains(out, "already on endpoint") { + t.Errorf("duplicate model must be rejected, got: %s", out) + } + + // No active endpoint: refuse instead of creating one. + cfg2 := newPlainListFixture() + cfg2.Vendor = "" + out = captureStdout(t, func() { plainAddModel(cfg2, "newmodel", "") }) + if !strings.Contains(out, "no active endpoint") { + t.Errorf("missing active endpoint must be rejected, got: %s", out) + } +} diff --git a/cmd/ggcode/plain_i18n.go b/cmd/ggcode/plain_i18n.go new file mode 100644 index 000000000..8b0e281de --- /dev/null +++ b/cmd/ggcode/plain_i18n.go @@ -0,0 +1,179 @@ +package main + +// Linear-frontend i18n (#4604): every user-visible string of the plain REPL +// goes through plainTr. Two catalogs are maintained (en, zh-CN) keyed by +// tui.Language; other configured languages fall back to English. The catalog +// lives here (not in internal/tui's 1300-line table) so the linear frontend +// stays self-contained and its strings can be audited separately. + +import ( + "strings" + + "github.com/topcheer/ggcode/internal/config" + + "github.com/topcheer/ggcode/internal/tui" +) + +var plainCatalogs = map[tui.Language]map[string]string{ + tui.LangEnglish: { + "banner": "ggcode linear frontend (no TUI) - /help for commands, Ctrl-D to exit", + "askuser.off": "[plain] ask_user disabled - the agent ends its turn and you answer by typing the next line", + "notice": "[notice]", + "prompt": "ggcode> ", + "help.title": "commands:", + "help.status": " /status show active vendor/endpoint/model", + "help.model": " /model hot-switch model (persists)", + "help.provider": " /provider [ep] switch vendor (optionally endpoint)", + "help.plist": " /provider --list [--all] list endpoints+models (default: keyed+modelled only)", + "help.padd": " /provider --add-endpoint ... add endpoint to active vendor", + "help.endpoint": " /endpoint switch endpoint on current vendor", + "help.emodels": " /endpoint --show-models all models on the active endpoint", + "help.erefresh": " /endpoint --refresh-models force model refetch, then show", + "help.eadd": " /endpoint --add-model ... add a model to the active endpoint", + "help.ekey": " /endpoint --set-apikey ... set the API key on the active endpoint", + "help.lang": " /lang switch interface language (persists)", + "help.exit": " /exit leave the REPL (also Ctrl-D)", + "help.tail": "anything else is sent to the agent; approvals default to Accept", + "help.tail2": "(GGCODE_PLAIN_APPROVE=deny rejects instead); ask_user is disabled.", + "cfg.unavailable": "[plain] config unavailable", + "agent.unavailable": "[plain] agent unavailable", + "resolve.failed": "[plain] resolve failed:", + "switch.failed": "[plain] switch failed:", + "switch.nopersist": "[plain] switch applied but not persisted:", + "switch.ok": "[plain] now on vendor=%s endpoint=%s model=%s", + "usage.model": "usage: /model ", + "usage.provider": "usage: /provider [endpoint] | --list [--all] | --add-endpoint ...", + "usage.endpoint": "usage: /endpoint | --show-models | --refresh-models | --add-model ... | --set-apikey ...", + "cmd.unknown": "[plain] unknown command:", + "opt.unknown": "[plain] unknown /provider option:", + "opt.unknown.ep": "[plain] unknown /endpoint option:", + "flags.bad": "[plain] bad flags:", + "lang.usage": "usage: /lang ", + "lang.unknown": "[plain] unsupported language:", + "lang.switched": "[plain] language switched to %s (persisted)", + "lang.nopersist": "[plain] language switched but not persisted:", + "add.req": "[plain] --endpoint-name/--endpoint-type/--endpoint-url are required", + "add.proto": "[plain] unsupported endpoint-type %q (want openai|openai-responses|anthropic|gemini|copilot)", + "add.novendor": "[plain] no active vendor to add the endpoint to", + "add.duplicate": "[plain] endpoint %q already exists on vendor %q", + "add.ok": "[plain] endpoint %q added to vendor %q%s", + "add.switchhint": " - switch with: /endpoint ", + "list.vendor": "vendor %s:", + "list.noendpoints": "vendor %s: (no endpoints)", + "list.nokey": " (no api key)", + "list.nomodels": " (no models configured)", + "list.more": " (+%d more - use /endpoint --show-models)", + "models.title": "models on %s/%s:", + "models.discovered": "(discovered)", + "models.none": " (none - use /endpoint --add-model --model )", + "models.nodiscovery": "[plain] model discovery unavailable (showing configured list):", + "addmodel.req": "[plain] --model is required", + "addmodel.noactive": "[plain] no active endpoint to add the model to", + "addmodel.nofound": "[plain] active endpoint %q not found on vendor %q", + "addmodel.duplicate": "[plain] model %q already on endpoint %q", + "addmodel.ok": "[plain] model %q added to endpoint %q - switch with: /model %s", + "setkey.req": "[plain] --apikey is required", + "setkey.noactive": "[plain] no active endpoint to set the key on", + "setkey.nofound": "[plain] active endpoint %q not found on vendor %q", + "setkey.ok": "[plain] api key set on endpoint %q", + }, + tui.LangZhCN: { + "banner": "ggcode 线性前端（无 TUI）- /help 查看命令，Ctrl-D 退出", + "askuser.off": "[plain] ask_user 已禁用 - 模型结束回合后，输入下一行即是你的回答", + "notice": "[提示]", + "prompt": "ggcode> ", + "help.title": "命令：", + "help.status": " /status 查看当前 vendor/endpoint/模型", + "help.model": " /model <名称> 热切换模型（持久化）", + "help.provider": " /provider [ep] 切换 vendor（可选 endpoint）", + "help.plist": " /provider --list [--all] 列出 endpoint 与模型（默认仅显示已配 key 且有模型的）", + "help.padd": " /provider --add-endpoint ... 向当前 vendor 添加 endpoint", + "help.endpoint": " /endpoint 在当前 vendor 内切换 endpoint", + "help.emodels": " /endpoint --show-models 当前 endpoint 的全部模型", + "help.erefresh": " /endpoint --refresh-models 强制刷新模型列表后显示", + "help.eadd": " /endpoint --add-model ... 向当前 endpoint 添加模型", + "help.ekey": " /endpoint --set-apikey ... 设置当前 endpoint 的 API key", + "help.lang": " /lang 切换界面语言（持久化）", + "help.exit": " /exit 退出（也可 Ctrl-D）", + "help.tail": "其余输入直接发给模型；审批默认接受", + "help.tail2": "（GGCODE_PLAIN_APPROVE=deny 可改为拒绝）；ask_user 已禁用。", + "cfg.unavailable": "[plain] 配置不可用", + "agent.unavailable": "[plain] agent 不可用", + "resolve.failed": "[plain] 解析失败：", + "switch.failed": "[plain] 切换失败：", + "switch.nopersist": "[plain] 已切换但未持久化：", + "switch.ok": "[plain] 当前 vendor=%s endpoint=%s 模型=%s", + "usage.model": "用法：/model <名称>", + "usage.provider": "用法：/provider [endpoint] | --list [--all] | --add-endpoint ...", + "usage.endpoint": "用法：/endpoint | --show-models | --refresh-models | --add-model ... | --set-apikey ...", + "cmd.unknown": "[plain] 未知命令：", + "opt.unknown": "[plain] 未知的 /provider 选项：", + "opt.unknown.ep": "[plain] 未知的 /endpoint 选项：", + "flags.bad": "[plain] 参数错误：", + "lang.usage": "用法：/lang ", + "lang.unknown": "[plain] 不支持的语言：", + "lang.switched": "[plain] 已切换语言为 %s（已持久化）", + "lang.nopersist": "[plain] 已切换但未持久化：", + "add.req": "[plain] --endpoint-name/--endpoint-type/--endpoint-url 为必填", + "add.proto": "[plain] 不支持的 endpoint-type %q（应为 openai|openai-responses|anthropic|gemini|copilot）", + "add.novendor": "[plain] 没有活动 vendor 可添加 endpoint", + "add.duplicate": "[plain] endpoint %q 已存在于 vendor %q", + "add.ok": "[plain] endpoint %q 已添加到 vendor %q%s", + "add.switchhint": " - 切换命令：/endpoint ", + "list.vendor": "vendor %s：", + "list.noendpoints": "vendor %s：（无 endpoint）", + "list.nokey": " （未配 api key）", + "list.nomodels": " （无已配置模型）", + "list.more": " （还有 %d 个 - 用 /endpoint --show-models 查看）", + "models.title": "%s/%s 上的模型：", + "models.discovered": "（在线发现）", + "models.none": " （暂无 - 用 /endpoint --add-model --model 添加）", + "models.nodiscovery": "[plain] 模型发现不可用（显示已配置列表）：", + "addmodel.req": "[plain] 必须指定 --model", + "addmodel.noactive": "[plain] 没有活动 endpoint 可添加模型", + "addmodel.nofound": "[plain] 活动 endpoint %q 不在 vendor %q 上", + "addmodel.duplicate": "[plain] 模型 %q 已在 endpoint %q 上", + "addmodel.ok": "[plain] 模型 %q 已添加到 endpoint %q - 切换命令：/model %s", + "setkey.req": "[plain] 必须指定 --apikey", + "setkey.noactive": "[plain] 没有活动 endpoint 可设置 key", + "setkey.nofound": "[plain] 活动 endpoint %q 不在 vendor %q 上", + "setkey.ok": "[plain] 已设置 endpoint %q 的 api key", + }, +} + +// plainLang is the linear frontend's active language: initialized from +// cfg.Language at startup, hot-switched by /lang (which also persists to +// cfg.Language so TUI/pipe sessions inherit the choice). +var plainLang = tui.LangEnglish + +// plainTr looks up key in the active language catalog, falling back to +// English, then to the key itself (missing translation should never blank +// the output). +func plainTr(lang tui.Language, key string) string { + if c, ok := plainCatalogs[lang]; ok { + if v, ok := c[key]; ok { + return v + } + } + if v, ok := plainCatalogs[tui.LangEnglish][key]; ok { + return v + } + return key +} + +// plainLangFromConfig derives the frontend language from cfg.Language, +// defaulting to English (same convention as the TUI's startup path). +func plainLangFromConfig(cfgLanguage string) tui.Language { + if strings.TrimSpace(cfgLanguage) == "" { + return tui.LangEnglish + } + return tui.NormalizeLanguage(cfgLanguage) +} + +// cfgLang extracts cfg.Language nil-safely. +func cfgLang(cfg *config.Config) string { + if cfg == nil { + return "" + } + return cfg.Language +} diff --git a/cmd/ggcode/plain_i18n_test.go b/cmd/ggcode/plain_i18n_test.go new file mode 100644 index 000000000..bd7749e67 --- /dev/null +++ b/cmd/ggcode/plain_i18n_test.go @@ -0,0 +1,78 @@ +package main + +import ( + "sort" + "testing" + + "github.com/topcheer/ggcode/internal/config" + "github.com/topcheer/ggcode/internal/tui" +) + +// TestPlainCatalogKeyParity ensures every key translated in one catalog +// exists in the other - a missing entry would silently fall back to English +// (or worse, print the raw key) mid-session. +func TestPlainCatalogKeyParity(t *testing.T) { + en := plainCatalogs[tui.LangEnglish] + zh := plainCatalogs[tui.LangZhCN] + if len(en) == 0 || len(zh) == 0 { + t.Fatalf("catalogs must be non-empty (en=%d zh=%d)", len(en), len(zh)) + } + enKeys := make([]string, 0, len(en)) + for k := range en { + enKeys = append(enKeys, k) + } + sort.Strings(enKeys) + for _, k := range enKeys { + if _, ok := zh[k]; !ok { + t.Errorf("key %q missing from zh-CN catalog", k) + } + } + for k := range zh { + if _, ok := en[k]; !ok { + t.Errorf("key %q missing from en catalog", k) + } + } +} + +// TestPlainTrFallback verifies lookup order: active language, then English, +// then the raw key (never blank output). +func TestPlainTrFallback(t *testing.T) { + if got := plainTr(tui.LangEnglish, "banner"); got == "" || got == "banner" { + t.Errorf("en lookup failed: %q", got) + } + if got := plainTr(tui.LangZhCN, "banner"); got == "" || got == "banner" { + t.Errorf("zh lookup failed: %q", got) + } + // Unsupported language falls back to English. + if got := plainTr(tui.Language("fr"), "banner"); got != plainTr(tui.LangEnglish, "banner") { + t.Errorf("unsupported language must fall back to en, got %q", got) + } + // Unknown key falls back to the key itself. + if got := plainTr(tui.LangEnglish, "no.such.key"); got != "no.such.key" { + t.Errorf("unknown key must echo the key, got %q", got) + } +} + +// TestPlainLangFromConfig verifies the startup derivation: empty stays +// English, configured values normalize (POSIX locale suffixes included). +func TestPlainLangFromConfig(t *testing.T) { + if got := plainLangFromConfig(""); got != tui.LangEnglish { + t.Errorf("empty config language = %q, want en", got) + } + if got := plainLangFromConfig("zh_CN.UTF-8"); got != tui.LangZhCN { + t.Errorf("POSIX locale zh_CN.UTF-8 = %q, want zh-CN", got) + } + if got := plainLangFromConfig("en"); got != tui.LangEnglish { + t.Errorf("en = %q, want en", got) + } +} + +// TestCfgLangNilSafe verifies the nil-config accessor used at startup. +func TestCfgLangNilSafe(t *testing.T) { + if got := cfgLang(nil); got != "" { + t.Errorf("cfgLang(nil) = %q, want empty", got) + } + if got := cfgLang(&config.Config{Language: "zh-CN"}); got != "zh-CN" { + t.Errorf("cfgLang = %q, want zh-CN", got) + } +} diff --git a/cmd/ggcode/plain_test.go b/cmd/ggcode/plain_test.go new file mode 100644 index 000000000..ac6ee3da5 --- /dev/null +++ b/cmd/ggcode/plain_test.go @@ -0,0 +1,282 @@ +package main + +import ( + "context" + "io" + "os" + "strings" + "testing" + + "github.com/topcheer/ggcode/internal/permission" + "github.com/topcheer/ggcode/internal/provider" +) + +// withEnv sets env[k]=v for the duration of fn, restoring the prior value +// (or absence) afterwards. +func withEnv(t *testing.T, k, v string, fn func()) { + t.Helper() + old, had := os.LookupEnv(k) + if err := os.Setenv(k, v); err != nil { + t.Skipf("cannot set %s: %v", k, err) + } + defer func() { + if had { + _ = os.Setenv(k, old) + } else { + _ = os.Unsetenv(k) + } + }() + fn() +} + +// TestIsDumbTerminal verifies the entry-level environment probe that routes +// --plain / TERM=dumb / blank TERM away from the Bubble Tea frontend (#4604). +func TestIsDumbTerminal(t *testing.T) { + cases := []struct { + term string + want bool + }{ + {"dumb", true}, + {"DUMB", true}, // case-insensitive + {" dumb ", true}, // surrounding whitespace + {"", true}, // blank TERM on restricted environments + {"xterm-256color", false}, + {"screen", false}, // screen sessions keep the TUI path + } + old, had := os.LookupEnv("TERM") + if !had { + t.Skipf("TERM not set in test environment") + } + defer func() { _ = os.Setenv("TERM", old) }() + for _, c := range cases { + setTerm := c.term + if err := os.Setenv("TERM", setTerm); err != nil { + t.Skipf("cannot set TERM: %v", err) + } + if got := isDumbTerminal(); got != c.want { + t.Errorf("isDumbTerminal(TERM=%q) = %v, want %v", c.term, got, c.want) + } + } +} + +// TestPlainApprovalDefaultAllow verifies the #4604 design decision: the +// linear frontend approves tools by default (no dialog exists) and only +// GGCODE_PLAIN_APPROVE=deny switches to rejection. +func TestPlainApprovalDefaultAllow(t *testing.T) { + old, had := os.LookupEnv("GGCODE_PLAIN_APPROVE") + defer func() { + if had { + _ = os.Setenv("GGCODE_PLAIN_APPROVE", old) + } else { + _ = os.Unsetenv("GGCODE_PLAIN_APPROVE") + } + }() + _ = os.Unsetenv("GGCODE_PLAIN_APPROVE") + if d := plainApproval(context.Background(), "run_command", "{}"); d != permission.Allow { + t.Errorf("default approval = %v, want Allow", d) + } + + os.Setenv("GGCODE_PLAIN_APPROVE", "deny") + if d := plainApproval(context.Background(), "run_command", "{}"); d != permission.Deny { + t.Errorf("GGCODE_PLAIN_APPROVE=deny approval = %v, want Deny", d) + } + + os.Setenv("GGCODE_PLAIN_APPROVE", "DENY") + if d := plainApproval(context.Background(), "run_command", "{}"); d != permission.Deny { + t.Errorf("GGCODE_PLAIN_APPROVE=DENY (case-insensitive) = %v, want Deny", d) + } +} + +// TestHandlePlainCommandLang verifies /lang switching semantics on the nil-cfg +// path (pure in-memory, no persistence): valid targets switch plainLang and +// keep the loop up; unsupported languages and missing args are refused +// without changing the active language. +func TestHandlePlainCommandLang(t *testing.T) { + orig := plainLang + defer func() { plainLang = orig }() + + plainLang = "en" + if handlePlainCommand("/lang fr", nil, nil) { + t.Fatal("/lang must not exit the loop") + } + if plainLang != "en" { + t.Errorf("unsupported language must not change plainLang, got %q", plainLang) + } + + if handlePlainCommand("/lang", nil, nil) { + t.Fatal("missing arg must not exit the loop") + } + if plainLang != "en" { + t.Errorf("missing arg must not change plainLang, got %q", plainLang) + } + + if handlePlainCommand("/lang zh-CN", nil, nil) { + t.Fatal("valid switch must not exit the loop") + } + if plainLang != "zh-CN" { + t.Errorf("plainLang = %q, want zh-CN", plainLang) + } + + if handlePlainCommand("/lang en", nil, nil) { + t.Fatal("switch back must not exit the loop") + } + if plainLang != "en" { + t.Errorf("plainLang = %q, want en", plainLang) + } +} + +// TestPlainStreamPrinterToolStatusPairing verifies the linear frontend's +// tool-call/result pairing contract: call lines carry no trailing newline, +// the matching ToolResult appends " (ok)"/" (error)", unmatched results are +// dropped, and Done closes at most one unterminated tail line. +func TestPlainStreamPrinterToolStatusPairing(t *testing.T) { + events := []provider.StreamEvent{ + {Type: provider.StreamEventToolCallDone, Tool: provider.ToolCallDelta{ID: "a", Name: "glob"}}, + {Type: provider.StreamEventToolResult, Tool: provider.ToolCallDelta{ID: "a"}}, + {Type: provider.StreamEventToolCallDone, Tool: provider.ToolCallDelta{ID: "b", Name: "run_command"}}, + {Type: provider.StreamEventToolResult, Tool: provider.ToolCallDelta{ID: "b"}, IsError: true}, + // Result with no preceding call line: must be dropped entirely. + {Type: provider.StreamEventToolResult, Tool: provider.ToolCallDelta{ID: "ghost"}}, + } + pending := make(map[string]bool) + printer := plainStreamPrinter(pending) + var out strings.Builder + orig := os.Stdout + r, w, _ := os.Pipe() + os.Stdout = w + for _, e := range events { + printer(e) + } + plainCloseTurn(pending) + _ = w.Close() + os.Stdout = orig + _, _ = io.Copy(&out, r) + + got := out.String() + for _, want := range []string{"\n[tool] glob (ok)\n", "\n[tool] run_command (error)\n"} { + if !strings.Contains(got, want) { + t.Errorf("output must contain %q, got:\n%q", want, got) + } + } + if strings.Count(got, "[tool]") != 2 { + t.Errorf("ghost result must not print a line, got:\n%q", got) + } + if len(pending) != 0 { + t.Errorf("pending map must be drained after close, got %v", pending) + } +} + +// TestPlainStreamPrinterDoneClosesUnterminatedTail verifies a call whose +// result never arrives gets exactly one closing newline (not one per call), +// so the next turn's output starts on a fresh line. +func TestPlainStreamPrinterDoneClosesUnterminatedTail(t *testing.T) { + events := []provider.StreamEvent{ + {Type: provider.StreamEventToolCallDone, Tool: provider.ToolCallDelta{ID: "a", Name: "slow"}}, + {Type: provider.StreamEventToolCallDone, Tool: provider.ToolCallDelta{ID: "b", Name: "slower"}}, + // Per-iteration provider Dones (with or without Usage) must be inert: + // tool results arrive between iterations and the turn only ends when + // RunStream returns. + {Type: provider.StreamEventDone}, + {Type: provider.StreamEventDone, Usage: &provider.TokenUsage{}}, + } + pending := make(map[string]bool) + printer := plainStreamPrinter(pending) + var out strings.Builder + orig := os.Stdout + r, w, _ := os.Pipe() + os.Stdout = w + for _, e := range events { + printer(e) + } + plainCloseTurn(pending) + _ = w.Close() + os.Stdout = orig + _, _ = io.Copy(&out, r) + + // Two unterminated calls: 2 leading newlines (one per [tool] line) + + // exactly one tail-closing newline + one separator newline = 4 total - + // never one closing newline per call. + got := out.String() + if n := strings.Count(got, "\n"); n != 4 { + t.Errorf("expected exactly 4 newlines (2 call-leads + 1 tail close + 1 separator), got %d:\n%q", n, got) + } + if len(pending) != 0 { + t.Errorf("pending map must be drained, got %v", pending) + } +} + +// TestPlainStreamPrinterIntermediateDoneKeepsPairing reproduces the real +// multi-iteration event order: ToolCallDone -> provider-stream Done -> +// ToolResult. StreamEventDone is inert by design; pairing survives and the +// turn only closes when plainCloseTurn runs after RunStream returns. +func TestPlainStreamPrinterIntermediateDoneKeepsPairing(t *testing.T) { + events := []provider.StreamEvent{ + {Type: provider.StreamEventToolCallDone, Tool: provider.ToolCallDelta{ID: "a", Name: "glob"}}, + {Type: provider.StreamEventDone, Usage: &provider.TokenUsage{}}, // intermediate, still inert + {Type: provider.StreamEventToolResult, Tool: provider.ToolCallDelta{ID: "a"}}, + } + pending := make(map[string]bool) + printer := plainStreamPrinter(pending) + var out strings.Builder + orig := os.Stdout + r, w, _ := os.Pipe() + os.Stdout = w + for _, e := range events { + printer(e) + } + plainCloseTurn(pending) + _ = w.Close() + os.Stdout = orig + _, _ = io.Copy(&out, r) + + got := out.String() + if !strings.Contains(got, "[tool] glob (ok)\n") { + t.Errorf("intermediate Done must not break pairing, got:\n%q", got) + } + if n := strings.Count(got, "\n"); n != 3 { // 1 leading + 1 after (ok) + 1 separator + t.Errorf("expected 3 newlines, got %d:\n%q", n, got) + } +} + +// TestWirePlainPersistNilAgentGuard verifies the best-effort contract: a nil +// agent (config unavailable path) returns nils instead of panicking, so +// --persist-session never blocks REPL startup. +func TestWirePlainPersistNilAgentGuard(t *testing.T) { + store, ses := wirePlainPersist(nil, nil) + if store != nil || ses != nil { + t.Fatalf("nil agent must yield nil store/session, got %v/%v", store, ses) + } +} + +// TestHandlePlainCommandExit verifies only the exit-family commands terminate +// the linear loop; every other command (including unknown ones) keeps it up. +func TestHandlePlainCommandExit(t *testing.T) { + old, had := os.LookupEnv("GGCODE_PLAIN_APPROVE") + defer func() { + if had { + _ = os.Setenv("GGCODE_PLAIN_APPROVE", old) + } else { + _ = os.Unsetenv("GGCODE_PLAIN_APPROVE") + } + }() + _ = os.Unsetenv("GGCODE_PLAIN_APPROVE") + + for _, cmd := range []string{"/exit", "/quit", "/q"} { + if !handlePlainCommand(cmd, nil, nil) { + t.Errorf("%s should exit the loop", cmd) + } + } + for _, cmd := range []string{ + "/help", "/?", "/status", // nil cfg must not panic + "/nosuchcommand", + "/model", // missing arg: usage, no exit + "/provider", // missing arg: usage, no exit + "/endpoint", // missing arg: usage, no exit + "/model glm-5.3", // nil agent/cfg: graceful message, no exit + "/provider z ai-1", // nil agent/cfg: graceful message, no exit + } { + if handlePlainCommand(cmd, nil, nil) { + t.Errorf("%s must NOT exit the loop", cmd) + } + } +} diff --git a/cmd/ggcode/root.go b/cmd/ggcode/root.go index 3f3b85688..7ee4b1a37 100644 --- a/cmd/ggcode/root.go +++ b/cmd/ggcode/root.go @@ -48,6 +48,10 @@ import ( "github.com/topcheer/ggcode/internal/webui" ) +// persistSessionFlag gates --persist-session: plain sessions append every +// message to the session store (write-only; no auto-resume on startup). +var persistSessionFlag bool + func NewRootCmd() *cobra.Command { var cfgFile string var resumeID string @@ -158,7 +162,17 @@ func NewRootCmd() *cobra.Command { if cfg.Language != "" { lang = tui.NormalizeLanguage(cfg.Language) } - if !tui.ConfirmHomeDir(lang) { + // #4604: pick the variant by mode FIRST - the linear + // frontend cannot host the full-screen Bubble Tea dialog + // (garbled on dumb terminals); it gets the line-oriented + // y/N twin with the same i18n copy instead. + var confirmed bool + if plainFlag || isDumbTerminal() { + confirmed = tui.ConfirmHomeDirPlain(lang) + } else { + confirmed = tui.ConfirmHomeDir(lang) + } + if !confirmed { fmt.Fprintln(os.Stderr, "Please cd into a project directory and run ggcode again.") return nil } @@ -190,14 +204,14 @@ func NewRootCmd() *cobra.Command { } if selectedID == "" { // User cancelled — start fresh (no auto-load) - return run(cfg, cfgFile, "__new__", bypassFlag) + return run(cfg, cfgFile, "__new__", bypassFlag, plainFlag) } - return run(cfg, cfgFile, selectedID, bypassFlag) + return run(cfg, cfgFile, selectedID, bypassFlag, plainFlag) } if newSession { - return run(cfg, cfgFile, "__new__", bypassFlag) + return run(cfg, cfgFile, "__new__", bypassFlag, plainFlag) } - return run(cfg, cfgFile, resumeID, bypassFlag) + return run(cfg, cfgFile, resumeID, bypassFlag, plainFlag) }, } @@ -217,7 +231,8 @@ func NewRootCmd() *cobra.Command { cmd.Flags().StringArrayVar(&readOnlyAllowedDirs, "readOnlyAllowedDir", nil, "extra read-only sandbox directory for pipe mode (can be repeated)") _ = cmd.Flags().MarkHidden("readOnlyAllowedDir") cmd.Flags().BoolVar(&bypassFlag, "bypass", false, "start in bypass permission mode (auto-approve safe ops, warn on dangerous)") - cmd.Flags().BoolVar(&plainFlag, "plain", false, "disable ANSI colors and decorative rendering (dumb terminals, screen readers, piped output; sets NO_COLOR)") + cmd.Flags().BoolVar(&plainFlag, "plain", false, "linear frontend: no TUI frames, line-oriented IO for dumb terminals/screen readers (sets NO_COLOR; TERM=dumb auto-selects)") + cmd.Flags().BoolVar(&persistSessionFlag, "persist-session", false, "persist each message of a --plain session to the session store (write-only; no auto-resume)") cmd.Flags().StringVar(&outputPath, "output", "", "output file path (default: stdout)") cmd.Flags().StringVar(&outputSchemaPath, "output-schema", "", "path to a JSON Schema the final response must conform to (structured outputs; supported endpoints constrain decoding, others fall back to prompt guidance)") // r415: per-run budget overrides (this invocation only, not persisted). @@ -471,7 +486,7 @@ func firstNonEmpty(values ...string) string { return "" } -func run(cfg *config.Config, cfgFile, resumeID string, bypass bool) error { +func run(cfg *config.Config, cfgFile, resumeID string, bypass, plain bool) error { trace := newStartupTrace("root.run") defer trace.Mark("return") @@ -929,6 +944,22 @@ func run(cfg *config.Config, cfgFile, resumeID string, bypass bool) error { mcpInfos := toTuiMCPInfos(mcpMgr.Snapshot()) trace.Mark("build tui mcp info") + // #4604 Track 1: --plain or TERM=dumb (blank) degrade to the linear + // frontend BEFORE any TUI-shaped service (A2A server, lanchat hub, + // Bubble Tea program) starts - raw-mode frame refresh is precisely what + // dumb terminals cannot carry. The linear REPL rides the same agent and + // registry built above, so config hot-swap, tools, and sessions all + // keep working; only the visual panels are absent. + if plain || isDumbTerminal() { + trace.Mark("linear frontend fork") + code := RunLinearREPL(ag, registry, cfg, cfgFile, startupNotices) + if code != 0 { + debug.Close() + os.Exit(code) + } + return nil + } + // Start A2A server if enabled. var a2aServer *a2a.Server // a2aRegistry already declared above for system prompt access diff --git a/cmd/ggcode/root_test.go b/cmd/ggcode/root_test.go index f90e1003a..3e8ee2dbe 100644 --- a/cmd/ggcode/root_test.go +++ b/cmd/ggcode/root_test.go @@ -225,6 +225,29 @@ func TestPlainFlagRegistered(t *testing.T) { } } +// TestPersistSessionFlagRegistered pins the CLI surface of +// --persist-session: registered, bool, default off (persistence is opt-in; +// the default plain behavior stays memory-only). Wiring semantics are +// covered by TestWirePlainPersistNilAgentGuard and live verification. +func TestPersistSessionFlagRegistered(t *testing.T) { + cmd := NewRootCmd() + if err := cmd.ParseFlags([]string{"--persist-session"}); err != nil { + t.Fatalf("ParseFlags() error = %v", err) + } + v, err := cmd.Flags().GetBool("persist-session") + if err != nil { + t.Fatalf("GetBool(persist-session) error = %v", err) + } + if !v { + t.Fatal("expected --persist-session to be true after parsing") + } + fresh := NewRootCmd() + def, _ := fresh.Flags().GetBool("persist-session") + if def { + t.Fatal("default must be false: persistence is opt-in") + } +} + func TestResumeFlagPickerKeywordTriggersPicker(t *testing.T) { cmd := NewRootCmd() if err := cmd.ParseFlags([]string{"--resume-picker"}); err != nil { diff --git a/docs/guide/cli-reference.md b/docs/guide/cli-reference.md index 068371b7d..178dc3987 100644 --- a/docs/guide/cli-reference.md +++ b/docs/guide/cli-reference.md @@ -280,5 +280,7 @@ ggcode version # Print version, commit, and build date | `--resume [id]` | Resume a session | | `--resume-picker` | Open session picker | | `--new-session` | Skip auto-resume, always start a new session | +| `--plain` | Linear no-TUI frontend (also auto-selected when `TERM=dumb` or unset). See [Plain Mode](plain-mode.md) | +| `--persist-session` | With `--plain`: append every message to the session store (write-only; auto-resume/picker ignore these sessions) | | `-v` | Shorthand for `--version` | | `-h, --help` | Show help | diff --git a/docs/guide/plain-mode.md b/docs/guide/plain-mode.md new file mode 100644 index 000000000..1df99012b --- /dev/null +++ b/docs/guide/plain-mode.md @@ -0,0 +1,79 @@ +# Plain Mode (Linear Frontend) + +`--plain` starts ggcode in a line-oriented frontend with no Bubble Tea TUI: line-buffered stdin, linear streaming output, no raw mode, no frame refresh. It exists for environments the full-screen TUI cannot serve — dumb terminals, screen readers, restricted SSH sessions, or anyone who prefers plain line IO. + +The same frontend is **auto-selected** when `TERM=dumb` or `TERM` is unset, so those environments never see garbled frames. + +```bash +ggcode --plain # explicit +TERM=dumb ggcode # automatic +``` + +It rides the same interactive runtime core as the TUI — agent, tools, sessions, and config hot-swap all work; only the visual panels are absent. A2A server, LAN chat, and other TUI-shaped services are not started on this path. + +## Output Format + +LLM text streams to the terminal as it arrives. Each tool call prints one boundary line with its result status: + +``` +[tool] glob (ok) +[tool] run_command (error) +``` + +Tool arguments and raw results are not echoed (they would drown the prose); `(ok)`/`(error)` is the per-call verdict. A call whose result never arrives still gets its line closed when the turn ends. + +## Slash Commands + +Anything not starting with `/` is sent to the agent as a conversation turn. + +| Command | Description | +|---------|-------------| +| `/status` | Show active vendor/endpoint/model | +| `/model ` | Hot-switch model (persists) | +| `/provider [endpoint]` | Switch vendor (optionally endpoint) | +| `/provider --list [--all]` | List endpoints grouped by vendor, up to 10 models each (`+N more` hint). Default shows only endpoints with an API key **and** a configured model list; `--all` includes the rest, annotated `(no api key)` / `(no models configured)`. The active endpoint is marked `*` | +| `/provider --add-endpoint ...` | Add an endpoint to the **active vendor** (see below) | +| `/endpoint ` | Switch endpoint on the current vendor | +| `/endpoint --show-models` | All models on the active endpoint (static config merged with live discovery) | +| `/endpoint --refresh-models` | Force a model refetch, bypassing the 6h discovery cache, then show | +| `/endpoint --add-model ...` | Add a model to the **active endpoint** (see below) | +| `/endpoint --set-apikey --apikey ` | Set the API key on the active endpoint | +| `/lang ` | Switch interface language (persists globally) | +| `/help`, `/exit` | Command list; leave the REPL (Ctrl-D also works) | + +### /provider --add-endpoint + +```bash +/provider --add-endpoint \ + --endpoint-name my-relay \ + --endpoint-type openai \ + --endpoint-url https://relay.example.com/v1 \ + --endpoint-model gpt-x \ + --context-window 200000 \ + --max-output 32768 \ + --endpoint-apikey sk-... +``` + +`--endpoint-type` must be one of `openai`, `openai-responses`, `anthropic`, `gemini`, `copilot`. Duplicate endpoint ids are refused. The endpoint lands on the currently active vendor. + +### /endpoint --add-model + +```bash +/endpoint --add-model --model glm-5.3 [--apikey sk-...] +``` + +Appends a model to the **active endpoint** (deduplicated; optional `--apikey` updates the endpoint key at the same time). Unlike `/provider --add-endpoint`, this never creates an endpoint — the active one is the target by definition. Switch to the new model afterwards with `/model `. + +## Behavior Differences + +- **No autopilot injections.** Even when the global default mode is `bypass`/`autopilot`, the linear frontend skips the goal-collection meta-instruction and the end-of-turn strategist prompt — a text answer ends the run and returns to the prompt. The permission surface itself is unchanged (still bypass if configured); only the behavioral injections are suppressed. +- **Approvals default to Accept.** There is no dialog UI; set `GGCODE_PLAIN_APPROVE=deny` to reject instead. +- **ask_user is disabled.** The agent ends its turn and waits; your next typed line is the answer — multi-turn conversation is the native pause point. +- **HOME-directory warning is line-oriented**: the same localized warning text as the TUI, confirmed with a `y/N` prompt instead of a full-screen dialog. +- **Language is global.** `/lang` persists to the `language` config field shared with the TUI (see [Configuration](configuration.md)); the session picks it up at startup, `en` when unset. +- **Sessions are memory-only by default.** Conversations live in memory and are lost on exit. Pass `--persist-session` to append every message to the session store instead (the id is printed at startup). Persistence is write-only by design: auto-resume and the session picker ignore these files, so the next plain start is always fresh; explicit `--resume ` can still read them. + +## Related + +- [CLI Reference](cli-reference.md) — global flags +- [Configuration](configuration.md) — `vendors`/`endpoints`/`language` schema diff --git a/internal/agent/agent.go b/internal/agent/agent.go index 8f06f979e..80f15fbd0 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -189,6 +189,7 @@ type Agent struct { autopilotGoalAsked bool // true after the goal-collection instruction has been injected autopilotGoalSet bool // true after the user has confirmed a goal (goal text is non-empty) autopilotStrategistCount int // number of strategist calls this run (safety valve) + suppressAutopilotInjection bool // cruise-mode injection opt-out (linear frontend): skip goal-collection and strategist prompts; permission surface unchanged strategistBudgetAnnounced bool // true once the budget-exhausted message has been injected strategistNoProgressCount int // consecutive strategist calls where agent made no tool calls reflectionFunc ReflectionFunc // called after each run with accumulated stats @@ -3353,7 +3354,7 @@ func (a *Agent) RunStreamWithContent(ctx context.Context, content []provider.Con // strategist call, this covers ~500 tool operations - enough for // large-scale implementation tasks. For very large projects, the user // sends another message ("continue") to reset the budget. - if a.currentMode().IsCruiseMode() && a.hasAutopilotGoal() && a.autopilotStrategistCount < maxAutopilotStrategistCalls { + if !a.suppressAutopilotInjection && a.currentMode().IsCruiseMode() && a.hasAutopilotGoal() && a.autopilotStrategistCount < maxAutopilotStrategistCalls { a.strategistNoProgressCount++ a.autopilotStrategistCount++ // Deadlock detection: if the agent has made NO tool calls for diff --git a/internal/agent/agent_autopilot.go b/internal/agent/agent_autopilot.go index da4cb8f43..0fbd85902 100644 --- a/internal/agent/agent_autopilot.go +++ b/internal/agent/agent_autopilot.go @@ -63,7 +63,27 @@ func (a *Agent) clearGoalIfNotAutopilot() { // The LLM must declare the goal using a "GOAL:" sentinel line so the // agent runtime can extract it for the strategist. // detection. +// SuppressAutopilotInjection opts this agent out of the cruise-mode +// (autopilot) behavioral injections while leaving the permission surface +// untouched: no goal-collection meta-instruction and no end-of-turn +// strategist prompt, so a text-only turn ends the run normally. The linear +// frontend uses this - its REPL is hand-driven line by line and the +// strategist's "keep going" guidance would restart the loop after every +// answer. Idempotent; there is deliberately no re-enable path because +// mid-session re-entry would need goal-state resync. +func (a *Agent) SuppressAutopilotInjection() { + if a == nil { + return + } + a.mu.Lock() + a.suppressAutopilotInjection = true + a.mu.Unlock() +} + func (a *Agent) maybeInjectAutopilotGoalCollection() { + if a.suppressAutopilotInjection { + return + } if !a.currentMode().IsCruiseMode() { return } diff --git a/internal/agent/agent_autopilot_test.go b/internal/agent/agent_autopilot_test.go new file mode 100644 index 000000000..e1e5c3a6b --- /dev/null +++ b/internal/agent/agent_autopilot_test.go @@ -0,0 +1,86 @@ +package agent + +import ( + "testing" + + "github.com/topcheer/ggcode/internal/permission" + "github.com/topcheer/ggcode/internal/provider" + "github.com/topcheer/ggcode/internal/tool" +) + +// newAutopilotTestAgent builds a minimal agent parked on the cruise surface +// (bypass/autopilot) - the precondition for goal-collection injection. +func newAutopilotTestAgent() *Agent { + mp := &mockProvider{ + chatResp: &provider.ChatResponse{ + Message: provider.Message{ + Role: "assistant", + Content: []provider.ContentBlock{{Type: "text", Text: "ok"}}, + }, + Usage: provider.TokenUsage{InputTokens: 1, OutputTokens: 1}, + }, + } + a := NewAgent(mp, tool.NewRegistry(), "", 1) + a.SetPermissionPolicy(permission.NewConfigPolicyWithMode(nil, []string{"."}, permission.AutopilotMode)) + return a +} + +// TestAutopilotGoalCollectionInjectsOnCruise verifies the baseline the +// suppress flag turns off: on the cruise surface the first +// maybeInjectAutopilotGoalCollection injects the meta-instruction exactly +// once (asked latch) and never twice. +func TestAutopilotGoalCollectionInjectsOnCruise(t *testing.T) { + a := newAutopilotTestAgent() + before := len(a.contextManager.Messages()) + a.maybeInjectAutopilotGoalCollection() + if !a.autopilotGoalAsked { + t.Fatal("cruise mode must set the asked latch on first injection") + } + if got := len(a.contextManager.Messages()) - before; got != 1 { + t.Fatalf("first call must inject exactly 1 message, got %d", got) + } + a.maybeInjectAutopilotGoalCollection() + if got := len(a.contextManager.Messages()) - before; got != 1 { + t.Fatalf("asked latch must prevent a second injection, got %d total", got) + } +} + +// TestSuppressAutopilotInjectionSkipsGoalCollection verifies the linear +// frontend's opt-out: with the flag set, the cruise-mode goal-collection +// meta-instruction is never injected and the asked latch stays false, so a +// later re-check still sees a clean state. +func TestSuppressAutopilotInjectionSkipsGoalCollection(t *testing.T) { + a := newAutopilotTestAgent() + a.SuppressAutopilotInjection() + before := len(a.contextManager.Messages()) + + a.maybeInjectAutopilotGoalCollection() + a.maybeInjectAutopilotGoalCollection() + + if a.autopilotGoalAsked { + t.Error("suppressed agent must not set the goal-asked latch") + } + if got := len(a.contextManager.Messages()) - before; got != 0 { + t.Errorf("suppressed agent must inject 0 messages, got %d", got) + } +} + +// TestSuppressAutopilotInjectionIdempotentAndModeIntact verifies the flag is +// idempotent and - by design - does NOT change the permission surface: the +// agent still reports cruise mode after opting out. +func TestSuppressAutopilotInjectionIdempotentAndModeIntact(t *testing.T) { + a := newAutopilotTestAgent() + if !a.IsAutopilotMode() { + t.Fatal("fixture must start on the cruise surface") + } + a.SuppressAutopilotInjection() + a.SuppressAutopilotInjection() // idempotent, no panic + if !a.IsAutopilotMode() { + t.Error("suppression must not downgrade the permission mode") + } + if a == nil { + t.Error("nil-guard branch documentation") + } + var nilAgent *Agent + nilAgent.SuppressAutopilotInjection() // nil-safe, no panic +} diff --git a/internal/agent/ratchet_reactive.go b/internal/agent/ratchet_reactive.go index bddf2e442..f0cea7cc1 100644 --- a/internal/agent/ratchet_reactive.go +++ b/internal/agent/ratchet_reactive.go @@ -42,22 +42,11 @@ func (rs *RuleStore) MatchingRulesForResult(resultContent, toolArgs string) []Ru // Match against the payload, not the advisory wrapping. cleaned := stripHarnessAdvisories(resultContent) - // Quick exit: only check if the result looks like an error - // (case-insensitive check for common error markers) - lowerResult := strings.ToLower(cleaned) - hasErrorMarker := strings.Contains(lowerResult, "error") || - strings.Contains(lowerResult, "fail") || - strings.Contains(lowerResult, "undefined") || - strings.Contains(lowerResult, "cannot find") || - strings.Contains(lowerResult, "panic") || - strings.Contains(lowerResult, "fatal") || - strings.Contains(lowerResult, "not found") || - strings.Contains(lowerResult, "denied") || - strings.Contains(lowerResult, "refused") - - if !hasErrorMarker { - return nil - } + // #4605: no error-marker pre-gate. Real failures like "signal: killed", + // "exit status 1", or "no such file" carry none of the classic markers + // ("error"/"fail"/...) and were silently dropped before pattern matching + // ever ran. The compiled patterns below are the precision instrument - + // let them decide; the marker list was a recall-killing shortcut. var result []Rule for _, r := range rs.rules { diff --git a/internal/agent/ratchet_reactive_test.go b/internal/agent/ratchet_reactive_test.go index 1d2c5158d..89d8a9a78 100644 --- a/internal/agent/ratchet_reactive_test.go +++ b/internal/agent/ratchet_reactive_test.go @@ -50,7 +50,7 @@ func TestMatchingRulesForResult_NoMatchForSuccess(t *testing.T) { } } -func TestMatchingRulesForResult_SkipsWithoutErrorMarkers(t *testing.T) { +func TestMatchingRulesForResult_PatternMatchesWithoutErrorMarkers(t *testing.T) { dir := t.TempDir() rs := NewRuleStore(dir) @@ -61,12 +61,14 @@ func TestMatchingRulesForResult_SkipsWithoutErrorMarkers(t *testing.T) { FixHint: "Run goimports", }) - // Contains "import" but no error markers — should skip - result := "This file import is fine." + // #4605: the marker-word gate is gone - the rule's own MatchPattern is + // authoritative. The result contains the pattern itself while carrying + // none of the 9 old marker substrings ("cycle" is not a marker). + result := "go build: import cycle not allowed in internal/graph" matched := rs.MatchingRulesForResult(result, "") - if len(matched) != 0 { - t.Fatalf("expected 0 matches without error markers, got %d", len(matched)) + if len(matched) != 1 { + t.Fatalf("expected 1 match without error markers, got %d", len(matched)) } } diff --git a/internal/agent/verify.go b/internal/agent/verify.go index 61df82129..5b47fbd65 100644 --- a/internal/agent/verify.go +++ b/internal/agent/verify.go @@ -576,7 +576,18 @@ func (a *Agent) executeVerifyCommand(ctx context.Context, command string) *Verif result.Passed = false result.Errors = extractErrorLines(outputStr) if len(result.Errors) == 0 { - result.Errors = []string{fmt.Sprintf("command failed: %v", err)} + // #4607: marker-free failure output ("signal: killed", segfault, + // OOM) extracts zero lines via the substring gate above, leaving + // the auto-repair loop with only "exit status 1" - zero information + // (and ratchet MatchErrors cannot learn a pattern from it either). + // Attach the truncated raw output tail (same 2000-char budget as + // result.Output) so both errorSummary consumers inject the real + // failure text. + fallback := fmt.Sprintf("command failed: %v", err) + if tail := truncStr(strings.TrimSpace(outputStr), 2000); tail != "" { + fallback += "\noutput tail:\n" + tail + } + result.Errors = []string{fallback} } debug.Log("verify", "FAILED: %d errors", len(result.Errors)) } else { diff --git a/internal/agent/zz_issue4605_test.go b/internal/agent/zz_issue4605_test.go new file mode 100644 index 000000000..3f3a8ed61 --- /dev/null +++ b/internal/agent/zz_issue4605_test.go @@ -0,0 +1,59 @@ +package agent + +// zz_issue4605_test.go pins the #4605 fix: MatchingRulesForResult must not +// gate pattern matching behind a hardcoded list of English error-marker +// substrings. The learning path (ratchet LLM prompt) generates match_pattern +// for ANY error output shape, so reactive injection must try every pattern +// regardless of markers. Each case below is a real marker-free error shape +// that the old gate silently dropped. + +import "testing" + +func TestIssue4605_MarkerFreeErrorShapesStillMatch(t *testing.T) { + cases := []struct { + name string + pattern string + result string + }{ + {"signal killed", `signal: killed`, "go test ./...: signal: killed"}, + {"no such file", `no such file or directory`, "cat: /tmp/missing.conf: no such file or directory"}, + {"exit status", `exit status \d+`, "go build: exit status 137"}, + {"deadline exceeded", `context deadline exceeded`, "probe: context deadline exceeded"}, + {"unexpected EOF", `unexpected EOF`, "read: unexpected EOF"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + rs := NewRuleStore(dir) + rs.AddRule(Rule{ + Category: "build", + Rule: "narrow parallelism and retry", + MatchPattern: tc.pattern, + FixHint: "GOFLAGS=-p=1 retry", + }) + matched := rs.MatchingRulesForResult(tc.result, "") + if len(matched) != 1 { + t.Fatalf("expected 1 reactive match for marker-free error %q, got %d", tc.result, len(matched)) + } + if matched[0].FixHint != "GOFLAGS=-p=1 retry" { + t.Errorf("unexpected FixHint: %s", matched[0].FixHint) + } + }) + } +} + +func TestIssue4605_NonMatchingSuccessStillQuiet(t *testing.T) { + dir := t.TempDir() + rs := NewRuleStore(dir) + rs.AddRule(Rule{ + Category: "build", + Rule: "use -tags goolm", + MatchPattern: `signal: killed`, + FixHint: "retry with GOFLAGS=-p=1", + }) + // Successful output must still not trigger injection. + if got := rs.MatchingRulesForResult("ok all tests passed", ""); len(got) != 0 { + t.Fatalf("expected 0 matches for success output, got %d", len(got)) + } +} diff --git a/internal/agent/zz_issue4607_test.go b/internal/agent/zz_issue4607_test.go new file mode 100644 index 000000000..ccec71d84 --- /dev/null +++ b/internal/agent/zz_issue4607_test.go @@ -0,0 +1,78 @@ +//go:build darwin || linux + +package agent + +import ( + "context" + "strings" + "testing" +) + +// TestZZIssue4607_MarkerFreeFailureOutputCarriesTail pins #4607: a non-zero +// exit whose output contains NONE of the extractErrorLines marker substrings +// (the real-world "signal: killed" / segfault / OOM class) must not collapse +// to the zero-information "command failed: exit status 1" fallback - the +// injected Errors entry must carry the raw output tail so the auto-repair +// loop and ratchet MatchErrors see the actual failure text. +func TestZZIssue4607_MarkerFreeFailureOutputCarriesTail(t *testing.T) { + a := &Agent{workingDir: "."} + // "signal: killed" hits none of error/fail/undefined:/cannot find/panic:/fatal. + result := a.executeVerifyCommand(context.Background(), + `sh -c 'echo signal: killed; exit 1'`) + if result.Passed { + t.Fatalf("expected failure, got Passed=true (Output=%q)", result.Output) + } + if len(result.Errors) != 1 { + t.Fatalf("expected exactly 1 fallback error entry, got %d: %v", len(result.Errors), result.Errors) + } + if !strings.Contains(result.Errors[0], "command failed") { + t.Errorf("fallback entry lost the exit-status prefix: %q", result.Errors[0]) + } + if !strings.Contains(result.Errors[0], "signal: killed") { + t.Errorf("#4607 regression: marker-free output tail missing from injected error - auto-repair loop is blind: %q", result.Errors[0]) + } +} + +// TestZZIssue4607_MarkerOutputStillUsesExtractedLines pins the complement: +// when the output DOES contain marker lines, extraction still wins and the +// fallback tail must not be attached (no double-reporting). +func TestZZIssue4607_MarkerOutputStillUsesExtractedLines(t *testing.T) { + a := &Agent{workingDir: "."} + result := a.executeVerifyCommand(context.Background(), + `sh -c 'echo something exploded; echo "build error: undefined: Foo"; exit 1'`) + if result.Passed { + t.Fatalf("expected failure, got Passed=true") + } + if len(result.Errors) == 0 { + t.Fatal("expected extracted error lines") + } + for _, e := range result.Errors { + if strings.Contains(e, "output tail") { + t.Errorf("fallback tail must not attach when extraction produced lines: %q", e) + } + } + if !zz4607Contains(strings.Join(result.Errors, "\n"), "build error: undefined: Foo") { + t.Errorf("extracted marker line missing: %v", result.Errors) + } +} + +// TestZZIssue4607_EmptyOutputKeepsBareFallback pins the empty-output corner: +// a command that fails with NO output at all keeps the plain exit-status +// fallback (no dangling "output tail:" header). +func TestZZIssue4607_EmptyOutputKeepsBareFallback(t *testing.T) { + a := &Agent{workingDir: "."} + result := a.executeVerifyCommand(context.Background(), "sh -c 'exit 1'") + if result.Passed { + t.Fatal("expected failure") + } + if len(result.Errors) != 1 || !strings.Contains(result.Errors[0], "command failed") { + t.Fatalf("expected bare fallback entry, got %v", result.Errors) + } + if strings.Contains(result.Errors[0], "output tail") { + t.Errorf("empty output must not carry a tail header: %q", result.Errors[0]) + } +} + +func zz4607Contains(s, want string) bool { + return strings.Contains(s, want) +} diff --git a/internal/memory/zz_issue4469_test.go b/internal/memory/zz_gcforget_repro_test.go similarity index 100% rename from internal/memory/zz_issue4469_test.go rename to internal/memory/zz_gcforget_repro_test.go diff --git a/internal/provider/model_discovery.go b/internal/provider/model_discovery.go index 5fc7e61ce..0653dc61a 100644 --- a/internal/provider/model_discovery.go +++ b/internal/provider/model_discovery.go @@ -49,6 +49,18 @@ type modelDiscoveryCacheFile struct { // DiscoverModels fetches the latest model list for a resolved endpoint when the remote API exposes it. func DiscoverModels(ctx context.Context, resolved *config.ResolvedEndpoint) ([]string, error) { + return discoverModels(ctx, resolved, false) +} + +// ForceDiscoverModels refetches the model list, bypassing the TTL cache - +// the refresh path for users who know the registry changed server-side +// (linear frontend /endpoint --refresh-models). The fresh list is cached +// like a normal discovery. +func ForceDiscoverModels(ctx context.Context, resolved *config.ResolvedEndpoint) ([]string, error) { + return discoverModels(ctx, resolved, true) +} + +func discoverModels(ctx context.Context, resolved *config.ResolvedEndpoint, force bool) ([]string, error) { if resolved == nil { return nil, fmt.Errorf("resolved endpoint is nil") } @@ -66,6 +78,9 @@ func DiscoverModels(ctx context.Context, resolved *config.ResolvedEndpoint) ([]s return nil, fmt.Errorf("protocol %q does not support model discovery", resolved.Protocol) } cacheKey := modelDiscoveryCacheKey(resolved) + if force { + invalidateCachedDiscoveredModels(cacheKey) + } cachedModels, cacheErr := loadCachedDiscoveredModels(cacheKey) if len(cachedModels) > 0 { return cachedModels, nil @@ -366,6 +381,24 @@ func normalizedDiscoveryHost(baseURL string) string { return strings.ToLower(host) } +// invalidateCachedDiscoveredModels drops the cache entry for cacheKey so the +// next load refetches from the remote API (force-refresh path). +func invalidateCachedDiscoveredModels(cacheKey string) { + if cacheKey == "" { + return + } + modelDiscoveryCacheMu.Lock() + defer modelDiscoveryCacheMu.Unlock() + if err := ensureModelDiscoveryCacheLoadedLocked(); err != nil { + return + } + if _, ok := modelDiscoveryCache[cacheKey]; !ok { + return + } + delete(modelDiscoveryCache, cacheKey) + _ = persistModelDiscoveryCacheLocked() +} + func loadCachedDiscoveredModels(cacheKey string) ([]string, error) { if cacheKey == "" { return nil, nil diff --git a/internal/provider/model_discovery_test.go b/internal/provider/model_discovery_test.go index 9fe32d361..2fceb651b 100644 --- a/internal/provider/model_discovery_test.go +++ b/internal/provider/model_discovery_test.go @@ -262,6 +262,67 @@ func TestDiscoverModelsUsesPersistentCacheWithinTTL(t *testing.T) { } } +// TestForceDiscoverModelsBypassesLiveCache verifies the force path evicts a +// still-fresh (within-TTL) cache entry and refetches from the remote API - +// the semantics the linear frontend's --refresh-models depends on. The fresh +// list is then served from cache like a normal discovery. +func TestForceDiscoverModelsBypassesLiveCache(t *testing.T) { + resetModelDiscoveryCacheForTests(t) + var hits atomic.Int32 + var current atomic.Pointer[string] + first := "stale-model" + current.Store(&first) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hits.Add(1) + m := *current.Load() + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"data":[{"id":"` + m + `"}]}`)) + })) + defer server.Close() + + resolved := &config.ResolvedEndpoint{ + VendorID: "custom", + EndpointID: "primary", + Protocol: "openai", + BaseURL: server.URL + "/v1", + APIKey: "test-key", + } + models, err := DiscoverModels(context.Background(), resolved) + if err != nil { + t.Fatalf("initial DiscoverModels: %v", err) + } + if len(models) != 1 || models[0] != "stale-model" { + t.Fatalf("unexpected initial models: %v", models) + } + + // Server-side registry changed; the cache is still fresh within TTL. + fresh := "fresh-model" + current.Store(&fresh) + models, err = ForceDiscoverModels(context.Background(), resolved) + if err != nil { + t.Fatalf("ForceDiscoverModels: %v", err) + } + if len(models) != 1 || models[0] != "fresh-model" { + t.Fatalf("force path must serve the refetched list, got: %v", models) + } + if hits.Load() != 2 { + t.Fatalf("force path must hit the remote API again (2 hits total), got %d", hits.Load()) + } + + // The refreshed list is now cached: a normal discovery serves it with no + // further network traffic. + models, err = DiscoverModels(context.Background(), resolved) + if err != nil { + t.Fatalf("post-force cached DiscoverModels: %v", err) + } + if len(models) != 1 || models[0] != "fresh-model" { + t.Fatalf("post-force cache must serve the fresh list, got: %v", models) + } + if hits.Load() != 2 { + t.Fatalf("post-force discovery must reuse the cache (still 2 hits), got %d", hits.Load()) + } +} + func TestDiscoverModelsRefetchesExpiredCache(t *testing.T) { resetModelDiscoveryCacheForTests(t) var hits atomic.Int32 diff --git a/internal/tui/home_warn.go b/internal/tui/home_warn.go index 4154c7992..86c06ed01 100644 --- a/internal/tui/home_warn.go +++ b/internal/tui/home_warn.go @@ -1,7 +1,10 @@ package tui import ( + "bufio" "fmt" + "os" + "strings" tea "charm.land/bubbletea/v2" "charm.land/lipgloss/v2" @@ -127,6 +130,27 @@ func ConfirmHomeDir(lang Language) bool { return model.(*homeWarnModel).ShouldContinue() } +// ConfirmHomeDirPlain is the line-oriented twin of ConfirmHomeDir for the +// linear frontend (#4604): same i18n title/message, but printed as plain +// text with a y/N prompt read from stdin - no Bubble Tea program, no raw +// mode, safe on dumb terminals where the full-screen dialog is garbled. +// Defaults to continue on unreadable/EOF input (mirrors the TUI fallback). +func ConfirmHomeDirPlain(lang Language) bool { + fmt.Println(tr(lang, "home.warn.title")) + fmt.Println(tr(lang, "home.warn.message")) + fmt.Print("Continue in HOME directory? [y/N]: ") + sc := bufio.NewScanner(os.Stdin) + if !sc.Scan() { + return true + } + switch strings.ToLower(strings.TrimSpace(sc.Text())) { + case "y", "yes": + return true + default: + return false + } +} + // NormalizeLanguage converts a language string to a Language value. // Exported for use by cmd/ggcode. func NormalizeLanguage(s string) Language { diff --git a/internal/tui/home_warn_plain_test.go b/internal/tui/home_warn_plain_test.go new file mode 100644 index 000000000..f459c22f1 --- /dev/null +++ b/internal/tui/home_warn_plain_test.go @@ -0,0 +1,62 @@ +package tui + +import ( + "os" + "testing" +) + +// TestConfirmHomeDirPlain verifies the line-oriented HOME-directory confirm +// used by the linear frontend (#4604): y/yes continue, anything else exits, +// and EOF defaults to continue (mirroring the TUI dialog's failure fallback). +func TestConfirmHomeDirPlain(t *testing.T) { + cases := []struct { + name string + input string + want bool + }{ + {"y continues", "y\n", true}, + {"yes continues", "yes\n", true}, + {"Y continues (case-insensitive)", "Y\n", true}, + {"whitespace-padded y continues", " y \n", true}, + {"n exits", "n\n", false}, + {"arbitrary text exits", "let me out\n", false}, + {"empty line exits (default N)", "\n", false}, + {"EOF continues (fallback)", "", true}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + restoreStdin := withStdinString(t, c.input) + defer restoreStdin() + if got := ConfirmHomeDirPlain(LangEnglish); got != c.want { + t.Errorf("ConfirmHomeDirPlain(%q) = %v, want %v", c.input, got, c.want) + } + }) + } +} + +// withStdinString replaces os.Stdin with a pipe carrying s and returns a +// restore function. Output (the prompt) is discarded via os.DevNull-style +// redirection into a discarded writer. +func withStdinString(t *testing.T, s string) (restore func()) { + t.Helper() + r, w, err := os.Pipe() + if err != nil { + t.Skipf("cannot create pipe: %v", err) + } + if _, err := w.WriteString(s); err != nil { + t.Skipf("cannot write pipe: %v", err) + } + _ = w.Close() + oldStdin := os.Stdin + oldStdout := os.Stdout + os.Stdin = r + // Swallow the printed title/message/prompt so test output stays clean. + devNull, _ := os.OpenFile(os.DevNull, os.O_WRONLY, 0) + os.Stdout = devNull + return func() { + os.Stdin = oldStdin + os.Stdout = oldStdout + _ = r.Close() + _ = devNull.Close() + } +}