From a6cb06aa486349003b9d31fe6252bf6294fc25cd Mon Sep 17 00:00:00 2001 From: Junjun Zhang Date: Sun, 25 Oct 2026 21:44:00 +0800 Subject: [PATCH 01/24] fix(desktop): wailskit MCP/impersonation readers snapshot activeChatBridge via GetChatBridge (#4381) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SetChatBridge (writer) holds activeChatBridgeMu, but the 9 reader sites in mcp.go/config.go historically read activeChatBridge under globalMu.RLock() — a different lock, so no happens-before edge existed between write and read. A workspace-switch/shutdown teardown goroutine calling SetChatBridge while the frontend MCP panel invokes ListMCPServers / ReconnectMCPServer / ForceReauthMCPServer / SetMCPServerEnabled / AddMCPServer / RemoveMCPServer / loadSessionScopedConfig / ApplyImpersonation is a data race under -race. Route every reader through GetChatBridge() (activeChatBridgeMu.RLock — the writer's lock), preserving the snapshot-once semantics of #458/#563. No ABBA risk: activeChatBridgeMu is only taken inside Set/Get and never wraps globalMu. Adds TestActiveChatBridgeRaceRepro: writer churn vs 4 reader goroutines; fails under -race on unfixed code, passes with the fix. Co-Authored-By: ggcode --- desktop/wailskit/config.go | 2 +- desktop/wailskit/mcp.go | 28 +++--------- desktop/wailskit/zz_mcp_bridge_race_test.go | 47 +++++++++++++++++++++ 3 files changed, 55 insertions(+), 22 deletions(-) create mode 100644 desktop/wailskit/zz_mcp_bridge_race_test.go diff --git a/desktop/wailskit/config.go b/desktop/wailskit/config.go index 335c2adc7..95af4945b 100644 --- a/desktop/wailskit/config.go +++ b/desktop/wailskit/config.go @@ -793,7 +793,7 @@ func GetImpersonationPresets() []ImpersonationPresetInfo { // (#67/#69 struct-overwrite family, 4th instance). func ApplyImpersonation(presetID, version string, customHeaders map[string]string) error { globalMu.Lock() - chatSnap := activeChatBridge + chatSnap := GetChatBridge() err := applyImpersonationLocked(presetID, version, customHeaders) globalMu.Unlock() if err != nil { diff --git a/desktop/wailskit/mcp.go b/desktop/wailskit/mcp.go index 930c45790..ddf4ed5bd 100644 --- a/desktop/wailskit/mcp.go +++ b/desktop/wailskit/mcp.go @@ -45,9 +45,7 @@ type MCPServerInfo struct { // session runs workspace servers, so changes never took effect (and removals // resurrected on reload). func loadSessionScopedConfig() (*config.Config, error) { - globalMu.RLock() - chat := activeChatBridge - globalMu.RUnlock() + chat := GetChatBridge() return loadSessionScopedConfigFor(chat) } @@ -73,9 +71,7 @@ func ListMCPServers() ([]MCPServerInfo, error) { // load and the status lookup. The old code read activeChatBridge twice // independently - a switchWorkspace in between spliced OLD workspace // yaml config with NEW workspace connection state. - globalMu.RLock() - chatSnap := activeChatBridge - globalMu.RUnlock() + chatSnap := GetChatBridge() cfg, err := loadSessionScopedConfigFor(chatSnap) if err != nil { @@ -128,9 +124,7 @@ func ListMCPServers() ([]MCPServerInfo, error) { func SetMCPServerEnabled(name string, enabled bool) bool { disabled := !enabled - globalMu.RLock() - chat := activeChatBridge - globalMu.RUnlock() + chat := GetChatBridge() scope := chatScope(chat) if chat == nil || chat.mcpManager == nil { // Persist the toggle FIRST so the UI state matches disk (#408): @@ -201,9 +195,7 @@ func chatScope(chat *ChatBridge) string { } func ReconnectMCPServer(name string) bool { - globalMu.RLock() - chat := activeChatBridge - globalMu.RUnlock() + chat := GetChatBridge() if chat == nil || chat.mcpManager == nil { return false } @@ -213,9 +205,7 @@ func ReconnectMCPServer(name string) bool { // ForceReauthMCPServer deletes the server-name-specific OAuth credential and // triggers a reconnect. The next request will start a fresh OAuth flow. func ForceReauthMCPServer(name string) bool { - globalMu.RLock() - chat := activeChatBridge - globalMu.RUnlock() + chat := GetChatBridge() if chat == nil || chat.mcpManager == nil { return false } @@ -300,9 +290,7 @@ func AddMCPServer(values map[string]string) error { defer mcpConfigWriteMu.Unlock() // #458: snapshot the bridge once so the scope decision and the reload // below see the same session even if a workspace switch interleaves. - globalMu.RLock() - chatSnap := activeChatBridge - globalMu.RUnlock() + chatSnap := GetChatBridge() cfg, err := loadSessionScopedConfigFor(chatSnap) if err != nil { return fmt.Errorf("load config: %w", err) @@ -424,9 +412,7 @@ func RemoveMCPServer(name string) error { // Without this, a switchWorkspace between loadSessionScopedConfig and the // Disconnect below can leave the removed server connected in the old workspace // while Disconnect operates on the new workspace's manager (#563). - globalMu.RLock() - chatSnap := activeChatBridge - globalMu.RUnlock() + chatSnap := GetChatBridge() cfg, err := loadSessionScopedConfigFor(chatSnap) if err != nil { return fmt.Errorf("load config: %w", err) diff --git a/desktop/wailskit/zz_mcp_bridge_race_test.go b/desktop/wailskit/zz_mcp_bridge_race_test.go new file mode 100644 index 000000000..7473a9cf2 --- /dev/null +++ b/desktop/wailskit/zz_mcp_bridge_race_test.go @@ -0,0 +1,47 @@ +package wailskit + +import ( + "sync" + "testing" +) + +// TestActiveChatBridgeRaceRepro reproduces the data race between +// SetChatBridge (writer, holds activeChatBridgeMu) and the mcp.go/config.go +// readers that historically borrowed globalMu -- a different lock -- so no +// happens-before edge existed between write and read. +// +// Real-world trigger: a workspace-switch teardown goroutine calls +// SetChatBridge (sessions.go) while the frontend MCP panel concurrently +// invokes ListMCPServers / ReconnectMCPServer (Wails-bound App methods on +// other goroutines). Under -race this test fails on unfixed code and passes +// once every reader uses GetChatBridge(). +func TestActiveChatBridgeRaceRepro(t *testing.T) { + var wg sync.WaitGroup + + // Writer: teardown-style SetChatBridge churn (switch / shutdown). + wg.Add(1) + go func() { + defer wg.Done() + for i := 0; i < 3000; i++ { + SetChatBridge(&ChatBridge{}) + SetChatBridge(nil) + } + }() + + // Readers: same read patterns the MCP panel entry points use. + for r := 0; r < 4; r++ { + wg.Add(1) + go func() { + defer wg.Done() + for j := 0; j < 3000; j++ { + // These historically read activeChatBridge under + // globalMu.RLock() in mcp.go. + _ = ReconnectMCPServer("x") + _ = ForceReauthMCPServer("x") + } + }() + } + + wg.Wait() + SetChatBridge(nil) // restore +} From 35ec1ce1abad40da6ffa89ea816f49317e340985 Mon Sep 17 00:00:00 2001 From: Junjun Zhang Date: Sun, 25 Oct 2026 21:44:56 +0800 Subject: [PATCH 02/24] test(config): characterize instance hooks clear-to-empty silently lost (#4382) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds TestInstanceHooksClearToEmptySilentlyLost — a characterization test documenting the verified bug: clearing instance-scope hooks (non-empty -> empty) is silently lost at write time (diffHooks zero-suppression, no reset fallback for hooks in instanceResetManagedKeys/NestedKeys, deepMergeYAMLMaps keeps unmentioned keys) and resurrects on reload (mergeHookConfig only replaces when len(instance) > 0). Assertions marked BUG must be inverted when the bug is fixed. See #4382 for the full evidence chain. Co-Authored-By: ggcode --- internal/config/zz_hooks_clear_repro_test.go | 92 ++++++++++++++++++++ 1 file changed, 92 insertions(+) create mode 100644 internal/config/zz_hooks_clear_repro_test.go diff --git a/internal/config/zz_hooks_clear_repro_test.go b/internal/config/zz_hooks_clear_repro_test.go new file mode 100644 index 000000000..ecfb3bb32 --- /dev/null +++ b/internal/config/zz_hooks_clear_repro_test.go @@ -0,0 +1,92 @@ +package config + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/topcheer/ggcode/internal/hooks" +) + +// CHARACTERIZATION TEST - documents a real, verified bug (do not "fix" the +// test without fixing the bug; when the bug is fixed, invert the assertions +// marked BUG). +// +// Bug: clearing instance-scope hooks (non-empty -> empty) is silently lost. +// +// Evidence chain (verified against the failing repro, see git history): +// 1. WRITE-SIDE SUPPRESSION: diffHooks (instance_delta.go) only writes +// pre_tool_use/post_tool_use into the delta when len(current) > 0, so a +// cleared list never enters the delta. +// 2. NO RESET FALLBACK: applyInstanceResets (instance_reset.go) deletes +// stale on-disk keys only for instanceResetManagedKeys (scalars, #2106) +// and instanceResetNestedKeys (knight/subagents/swarm, #2936). "hooks" +// is in neither table, so no nil deletion marker is emitted for it. +// 3. MERGE KEEPS STALE KEYS: deepMergeYAMLMaps (config_save.go) only +// deletes a dst key when the delta carries an explicit nil marker; +// unmentioned keys survive, so the stale hooks stay on disk. +// 4. READ-SIDE SUPPRESSION: mergeHookConfig (instance.go) replaces the +// global list only when len(instance) > 0, so the stale non-empty +// on-disk list resurrects on the next LoadWithInstance. +// +// Scalar fields and knight/subagents/swarm all closed the identical +// resurrection hole (#2106/#2936); hooks is the only family left with +// write-side zero-suppression AND no reset fallback. +func TestInstanceHooksClearToEmptySilentlyLost(t *testing.T) { + withTestHome(t) + tmpDir := t.TempDir() + globalPath := filepath.Join(tmpDir, "ggcode.yaml") + // Global has NO hooks; hooks live only at instance scope. + if err := os.WriteFile(globalPath, []byte("language: en\n"), 0644); err != nil { + t.Fatal(err) + } + workspace := t.TempDir() + instPath := filepath.Join(InstanceDir(workspace), "ggcode.yaml") + + // Phase 1: set an instance-scope hook [A] and save. + cfg, err := LoadWithInstance(globalPath, workspace) + if err != nil { + t.Fatalf("LoadWithInstance: %v", err) + } + cfg.Hooks.PreToolUse = []hooks.Hook{{Match: "*", Command: "echo A"}} + if err := cfg.SaveInstance(workspace); err != nil { + t.Fatalf("SaveInstance(set): %v", err) + } + instData, err := os.ReadFile(instPath) + if err != nil || !strings.Contains(string(instData), "echo A") { + t.Fatalf("precondition: instance file should carry hook A; err=%v data=%s", err, instData) + } + + // Phase 2: reload so [A] comes from the instance file, then CLEAR hooks. + cfg2, err := LoadWithInstance(globalPath, workspace) + if err != nil { + t.Fatalf("reload: %v", err) + } + if len(cfg2.Hooks.PreToolUse) == 0 { + t.Fatal("precondition: reloaded merged config should carry hook A from instance file") + } + cfg2.Hooks.PreToolUse = nil + cfg2.Hooks.PostToolUse = nil + if err := cfg2.SaveInstance(workspace); err != nil { + t.Fatalf("SaveInstance(clear): %v", err) + } + + // Write side: BUG - the cleared hooks remain on disk. + instData, err = os.ReadFile(instPath) + if err != nil { + t.Fatalf("read instance file after clear: %v", err) + } + if !strings.Contains(string(instData), "echo A") { + t.Errorf("BUG APPEARS FIXED (write side): cleared hooks no longer on disk after SaveInstance. Invert this assertion when fixing.") + } + + // Read side: BUG - the cleared hooks resurrect on reload. + cfg3, err := LoadWithInstance(globalPath, workspace) + if err != nil { + t.Fatalf("reload after clear: %v", err) + } + if len(cfg3.Hooks.PreToolUse) == 0 { + t.Errorf("BUG APPEARS FIXED (read side): cleared hooks did not resurrect on reload. Invert this assertion when fixing.") + } +} From 41a5dc52fa9737cf02531a3019f38c2d9d5ac816 Mon Sep 17 00:00:00 2001 From: Junjun Zhang Date: Tue, 27 Oct 2026 12:59:22 +0800 Subject: [PATCH 03/24] test(memory): repro GC forget-digest path violates #3880 tombstone contract ForgetKey(key) -> GarbageCollect() removes the live memory file without archiving or writing a .tombstone, so ReadMemoryAsOf(key, now) time-travels back to stale .history content (found=true, "v1 content", tombstoneExists=false). Co-Authored-By: ggcode Co-Authored-By: ggcode --- internal/memory/zz_gcforget_repro_test.go | 48 +++++++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 internal/memory/zz_gcforget_repro_test.go diff --git a/internal/memory/zz_gcforget_repro_test.go b/internal/memory/zz_gcforget_repro_test.go new file mode 100644 index 000000000..d5f7bddf1 --- /dev/null +++ b/internal/memory/zz_gcforget_repro_test.go @@ -0,0 +1,48 @@ +package memory + +import ( + "os" + "path/filepath" + "testing" + "time" +) + +// Reproducer for the suspected bug: GC's forgotten-digest path removes the +// live file without archiving or writing a tombstone (#3880/#3924 contract), +// so ReadMemoryAsOf time-travels back to stale .history content after GC. +func TestGCForgottenDigestTimeTravelRepro(t *testing.T) { + dir := t.TempDir() + am := &AutoMemory{dir: dir} + key := "build-process" + + // Two saves => one archived version in .history plus a live version. + if err := am.SaveMemoryWithSourceActor(key, "v1 content", "user", "session-a"); err != nil { + t.Fatalf("save1: %v", err) + } + time.Sleep(10 * time.Millisecond) + if err := am.SaveMemoryWithSourceActor(key, "v2 content", "user", "session-a"); err != nil { + t.Fatalf("save2: %v", err) + } + + if err := am.ForgetKey(key); err != nil { + t.Fatalf("forget: %v", err) + } + am.GarbageCollect() + + // Live file must be gone. + if _, err := os.Stat(filepath.Join(dir, "build-process.md")); !os.IsNotExist(err) { + t.Fatalf("live file still present after GC: %v", err) + } + + now := time.Now() + content, _, found, err := am.ReadMemoryAsOf(key, now) + if err != nil { + t.Fatalf("asof read: %v", err) + } + tomb := filepath.Join(dir, ".history", "build-process.tombstone") + _, tombErr := os.Stat(tomb) + t.Logf("as_of(now): found=%v content=%q tombstoneExists=%v", found, content, tombErr == nil) + if found { + t.Errorf("BUG CONFIRMED: ReadMemoryAsOf returned stale content %q after ForgetKey+GC digest (no tombstone written)", content) + } +} From 0a5c88a447dbde1d18ec83e276141cd831c9b296 Mon Sep 17 00:00:00 2001 From: Junjun Zhang Date: Fri, 30 Oct 2026 10:14:41 +0800 Subject: [PATCH 04/24] fix(ratchet): drop error-marker gate in reactive rule injection (#4605) The 9-word English marker sniff before pattern matching silently dropped reactive FixHint injection for marker-free error shapes (signal: killed, no such file or directory, exit status 137, context deadline exceeded, unexpected EOF) that the learning path happily generates patterns for. MatchPattern is now authoritative and compiled via the shared regexCache (the gate path bypassed it with per-call regexp.Compile). - flipped the old gate-pinning test to the new semantics - zz_issue4605_test.go: 5 marker-free error shapes must match; success output stays quiet --- internal/agent/ratchet_reactive.go | 33 +++++--------- internal/agent/ratchet_reactive_test.go | 10 +++-- internal/agent/zz_issue4605_test.go | 59 +++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 25 deletions(-) create mode 100644 internal/agent/zz_issue4605_test.go diff --git a/internal/agent/ratchet_reactive.go b/internal/agent/ratchet_reactive.go index 6a7e3f541..7a395350c 100644 --- a/internal/agent/ratchet_reactive.go +++ b/internal/agent/ratchet_reactive.go @@ -1,9 +1,6 @@ package agent import ( - "regexp" - "strings" - "github.com/topcheer/ggcode/internal/debug" ) @@ -25,29 +22,23 @@ func (rs *RuleStore) MatchingRulesForResult(resultContent string) []Rule { defer rs.mu.Unlock() rs.load() - // Quick exit: only check if the result looks like an error - // (case-insensitive check for common error markers) - lowerResult := strings.ToLower(resultContent) - 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 gate. The old quick-exit required one of 9 + // hardcoded English marker substrings ("error", "fail", "not found", ...) + // before ANY rule pattern was tried, but the learning path (ratchet LLM + // prompt: "match_pattern matches the error OUTPUT messages") puts no such + // restriction on patterns. Rules learned from marker-free error shapes + // ("signal: killed", "no such file or directory", "exit status 137", + // "context deadline exceeded", "unexpected EOF") could never fire their + // FixHint reactively - a silent false-negative that hit exactly the most + // expensive error classes. Patterns are tried against every result, using + // the shared regexCache (which the old gate path bypassed via + // regexp.Compile per call). var result []Rule for _, r := range rs.rules { if r.MatchPattern == "" { continue } - re, err := regexp.Compile(r.MatchPattern) + re, err := rs.compilePattern(r.MatchPattern) if err != nil { continue } diff --git a/internal/agent/ratchet_reactive_test.go b/internal/agent/ratchet_reactive_test.go index c31452192..591329513 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 + // #4605: the marker-word gate is gone - the rule's own MatchPattern is + // authoritative. "import" hits even though the result carries none of the + // 9 old marker substrings. result := "This file import is fine." 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/zz_issue4605_test.go b/internal/agent/zz_issue4605_test.go new file mode 100644 index 000000000..a683b7502 --- /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)) + } +} From 82aae76c76fe60eb0cd6be64ef4587f2e2d6bb2e Mon Sep 17 00:00:00 2001 From: Junjun Zhang Date: Fri, 30 Oct 2026 11:10:23 +0800 Subject: [PATCH 05/24] fix(agent): verify fallback carries raw output tail for marker-free failures (#4607) Co-Authored-By: ggcode --- internal/agent/verify.go | 13 ++++- internal/agent/zz_issue4607_test.go | 78 +++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 1 deletion(-) create mode 100644 internal/agent/zz_issue4607_test.go diff --git a/internal/agent/verify.go b/internal/agent/verify.go index 1df82effe..0a9a2c002 100644 --- a/internal/agent/verify.go +++ b/internal/agent/verify.go @@ -554,7 +554,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_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) +} From 0238028522f54af5c487577a56931fd9b08e29c7 Mon Sep 17 00:00:00 2001 From: Junjun Zhang Date: Fri, 30 Oct 2026 12:45:39 +0800 Subject: [PATCH 06/24] feat(tui): linear no-Bubble-Tea frontend for --plain and TERM=dumb (#4604 Track 1) --plain previously only set NO_COLOR while still running the full Bubble Tea program (raw mode, frame refresh) - on dumb terminals, screen sessions, and limited SSH that is exactly the garbled output, repeated warning lines, and junk characters #4604 reports. New RunLinearREPL frontend (cmd/ggcode/plain.go): - line-buffered stdin loop, linear streaming print, no raw mode, no frames; rides the SAME interactive runtime core (agent, registry, sessions, config hot-swap) built before the fork point - routes at root.go before any TUI-shaped service (A2A, lanchat, Bubble Tea) starts; --plain or TERM=dumb/blank auto-select it - slash subset: /status /model /provider /endpoint /help /exit via the shared ActivateCurrentSelection + ApplyProviderToAgent funnel, persisted with cfg.Save() - approvals default Accept; GGCODE_PLAIN_APPROVE=deny rejects - ask_user unregistered: agent ends its turn and the next typed line is the answer (multi-turn is the native pause point) Verified: TERM=dumb smoke run (banner/notices//help//status real resolution//exit), same-value /model hot-switch persists; package tests pass (isDumbTerminal table, approval env matrix, exit-family loop semantics); full cmd/ggcode package PASS. Co-Authored-By: ggcode --- cmd/ggcode/plain.go | 209 +++++++++++++++++++++++++++++++++++++++ cmd/ggcode/plain_test.go | 118 ++++++++++++++++++++++ cmd/ggcode/root.go | 28 ++++-- 3 files changed, 349 insertions(+), 6 deletions(-) create mode 100644 cmd/ggcode/plain.go create mode 100644 cmd/ggcode/plain_test.go diff --git a/cmd/ggcode/plain.go b/cmd/ggcode/plain.go new file mode 100644 index 000000000..a64f15b62 --- /dev/null +++ b/cmd/ggcode/plain.go @@ -0,0 +1,209 @@ +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" + "fmt" + "os" + "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/tool" +) + +// 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 + + // 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("[plain] ask_user disabled - the agent ends its turn and you answer by typing the next line") + } + if ag != nil { + ag.SetApprovalHandler(plainApproval) + } + + fmt.Println("ggcode linear frontend (no TUI) - /help for commands, Ctrl-D to exit") + for _, n := range startupNotices { + fmt.Println("[notice]", n) + } + + sc := bufio.NewScanner(os.Stdin) + sc.Buffer(make([]byte, 64*1024), 1024*1024) + ctx := context.Background() + for { + fmt.Print("ggcode> ") + 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("commands:") + fmt.Println(" /status show active vendor/endpoint/model") + fmt.Println(" /model hot-switch model (persists)") + fmt.Println(" /provider [ep] switch vendor (optionally endpoint)") + fmt.Println(" /endpoint switch endpoint on current vendor") + fmt.Println(" /exit leave the REPL (also Ctrl-D)") + fmt.Println("anything else is sent to the agent; approvals default to Accept") + fmt.Println("(GGCODE_PLAIN_APPROVE=deny rejects instead); ask_user is disabled.") + case "/status": + if cfg == nil { + fmt.Println("[plain] config unavailable") + return false + } + if resolved, _, err := agentruntime.ResolveCurrentSelection(cfg); err != nil { + fmt.Println("[plain] 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("usage: /model ") + return false + } + switchPlainSelection(ag, cfg, "", "", args[0]) + case "/provider": + if len(args) < 1 || len(args) > 2 { + fmt.Println("usage: /provider [endpoint]") + return false + } + ep := "" + if len(args) == 2 { + ep = args[1] + } + switchPlainSelection(ag, cfg, args[0], ep, "") + case "/endpoint": + if len(args) != 1 { + fmt.Println("usage: /endpoint ") + return false + } + switchPlainSelection(ag, cfg, "", args[0], "") + default: + fmt.Println("[plain] unknown command:", cmd, "- /help for the list") + } + 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("[plain] config unavailable") + return + } + resolved, prov, err := agentruntime.ActivateCurrentSelection(cfg, vendor, endpoint, model) + if err != nil { + fmt.Println("[plain] switch failed:", err) + return + } + if ag != nil { + agentruntime.ApplyProviderToAgent(ag, prov, resolved) + } + if err := cfg.Save(); err != nil { + fmt.Println("[plain] switch applied but not persisted:", err) + return + } + fmt.Printf("[plain] now on vendor=%s endpoint=%s model=%s\n", resolved.VendorID, resolved.EndpointID, resolved.Model) +} + +// 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("[plain] agent unavailable") + return + } + err := ag.RunStream(ctx, line, func(event provider.StreamEvent) { + switch event.Type { + case provider.StreamEventText: + fmt.Print(event.Text) + case provider.StreamEventToolCallDone: + if event.Tool.Name != "" { + fmt.Printf("\n[tool] %s\n", event.Tool.Name) + } + case provider.StreamEventError: + fmt.Printf("\n[error] %v\n", event.Error) + case provider.StreamEventDone: + fmt.Println() + } + }) + if err != nil { + fmt.Printf("\n[error] turn failed: %v\n", err) + } +} diff --git a/cmd/ggcode/plain_test.go b/cmd/ggcode/plain_test.go new file mode 100644 index 000000000..a63e0ba2b --- /dev/null +++ b/cmd/ggcode/plain_test.go @@ -0,0 +1,118 @@ +package main + +import ( + "context" + "os" + "testing" + + "github.com/topcheer/ggcode/internal/permission" +) + +// 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) + } +} + +// 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 b3b2d7f9c..c1af9eec2 100644 --- a/cmd/ggcode/root.go +++ b/cmd/ggcode/root.go @@ -188,14 +188,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) }, } @@ -215,7 +215,7 @@ 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().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). @@ -469,7 +469,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") @@ -905,6 +905,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 From b0a23d06ce68ccddcb40fd92dc84a98257449ed9 Mon Sep 17 00:00:00 2001 From: Junjun Zhang Date: Fri, 30 Oct 2026 13:09:51 +0800 Subject: [PATCH 07/24] fix(tui): line-oriented HOME-dir confirm for linear frontend (#4604) The HOME-directory warning was a full-screen Bubble Tea dialog (tui.ConfirmHomeDir) invoked in runE BEFORE the plain/TUI fork, so --plain and TERM=dumb sessions still got a raw-mode dialog garbled on dumb terminals. Add ConfirmHomeDirPlain - same i18n title/message, a plain y/N prompt on stdin, no tea program - and dispatch to it from the linear path. Co-Authored-By: ggcode --- cmd/ggcode/root.go | 9 ++++++++- internal/tui/home_warn.go | 24 ++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/cmd/ggcode/root.go b/cmd/ggcode/root.go index c1af9eec2..3cc1a16ee 100644 --- a/cmd/ggcode/root.go +++ b/cmd/ggcode/root.go @@ -156,7 +156,14 @@ func NewRootCmd() *cobra.Command { if cfg.Language != "" { lang = tui.NormalizeLanguage(cfg.Language) } - if !tui.ConfirmHomeDir(lang) { + // #4604: the linear frontend cannot host the full-screen + // Bubble Tea dialog (garbled on dumb terminals) - use the + // line-oriented y/N twin with the same i18n copy. + confirmed := tui.ConfirmHomeDir(lang) + if plainFlag || isDumbTerminal() { + confirmed = tui.ConfirmHomeDirPlain(lang) + } + if !confirmed { fmt.Fprintln(os.Stderr, "Please cd into a project directory and run ggcode again.") return nil } 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 { From 7a1b574d8a1df2cf89385e2f82e2b7833f0b4a3f Mon Sep 17 00:00:00 2001 From: Junjun Zhang Date: Fri, 30 Oct 2026 13:12:35 +0800 Subject: [PATCH 08/24] test(tui): cover ConfirmHomeDirPlain input semantics (#4604) Eight-case table via stdin pipe injection: y/yes/Y/padded continue, n/arbitrary/empty exit, EOF falls back to continue (mirrors the TUI dialog's failure path). Co-Authored-By: ggcode --- internal/tui/home_warn_plain_test.go | 62 ++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 internal/tui/home_warn_plain_test.go 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() + } +} From 087549e82af75452ee8071ce7f08ecbfb4cb28e6 Mon Sep 17 00:00:00 2001 From: Junjun Zhang Date: Fri, 30 Oct 2026 13:27:02 +0800 Subject: [PATCH 09/24] feat(plain): provider/endpoint management subcommands (#4604 follow-up) /provider --list [--all] list endpoints grouped by vendor with up to 10 models each (+N more hint); default shows only keyed+modelled endpoints, --all includes keyless/modelless with annotations; active endpoint marked with a leading * /provider --add-endpoint add an endpoint (name/type/url/model/context- window/max-output/apikey) to the ACTIVE vendor, protocol whitelist enforced, duplicate refused /endpoint --show-models all models on the active endpoint (static config merged with live discovery) /endpoint --refresh-models force model refetch bypassing the TTL cache /endpoint --add-model append a model to the ACTIVE endpoint (dedup, optional --apikey update) - unlike /provider --add-endpoint it never creates one /endpoint --set-apikey overwrite the active endpoint's API key provider: DiscoverModels factored into discoverModels(force); exported ForceDiscoverModels + invalidateCachedDiscoveredModels so refresh drops the cache entry instead of serving the 6h TTL copy. Verified: live smoke on real config (grouped listing with caps, --all annotations, full model list, unknown-option guard); 5 new table tests (filter classes, cap+hint, active marker, validation guards before any mutation); cmd/ggcode + internal/provider packages PASS. Co-Authored-By: ggcode --- cmd/ggcode/plain.go | 388 +++++++++++++++++++++++++-- cmd/ggcode/plain_cmd_test.go | 162 +++++++++++ internal/provider/model_discovery.go | 33 +++ 3 files changed, 564 insertions(+), 19 deletions(-) create mode 100644 cmd/ggcode/plain_cmd_test.go diff --git a/cmd/ggcode/plain.go b/cmd/ggcode/plain.go index a64f15b62..afcf88c8b 100644 --- a/cmd/ggcode/plain.go +++ b/cmd/ggcode/plain.go @@ -25,8 +25,11 @@ package main import ( "bufio" "context" + "flag" "fmt" + "io" "os" + "sort" "strings" "github.com/topcheer/ggcode/internal/agent" @@ -111,11 +114,17 @@ func handlePlainCommand(line string, ag *agent.Agent, cfg *config.Config) bool { return true case "/help", "/?": fmt.Println("commands:") - fmt.Println(" /status show active vendor/endpoint/model") - fmt.Println(" /model hot-switch model (persists)") - fmt.Println(" /provider [ep] switch vendor (optionally endpoint)") - fmt.Println(" /endpoint switch endpoint on current vendor") - fmt.Println(" /exit leave the REPL (also Ctrl-D)") + fmt.Println(" /status show active vendor/endpoint/model") + fmt.Println(" /model hot-switch model (persists)") + fmt.Println(" /provider [ep] switch vendor (optionally endpoint)") + fmt.Println(" /provider --list [--all] list endpoints+models (default: keyed+modelled only)") + fmt.Println(" /provider --add-endpoint ... add endpoint to active vendor") + fmt.Println(" /endpoint switch endpoint on current vendor") + fmt.Println(" /endpoint --show-models all models on the active endpoint") + fmt.Println(" /endpoint --refresh-models force model refetch, then show") + fmt.Println(" /endpoint --add-model ... add a model to the active endpoint") + fmt.Println(" /endpoint --set-apikey ... set the API key on the active endpoint") + fmt.Println(" /exit leave the REPL (also Ctrl-D)") fmt.Println("anything else is sent to the agent; approvals default to Accept") fmt.Println("(GGCODE_PLAIN_APPROVE=deny rejects instead); ask_user is disabled.") case "/status": @@ -135,21 +144,9 @@ func handlePlainCommand(line string, ag *agent.Agent, cfg *config.Config) bool { } switchPlainSelection(ag, cfg, "", "", args[0]) case "/provider": - if len(args) < 1 || len(args) > 2 { - fmt.Println("usage: /provider [endpoint]") - return false - } - ep := "" - if len(args) == 2 { - ep = args[1] - } - switchPlainSelection(ag, cfg, args[0], ep, "") + handlePlainProvider(args, ag, cfg) case "/endpoint": - if len(args) != 1 { - fmt.Println("usage: /endpoint ") - return false - } - switchPlainSelection(ag, cfg, "", args[0], "") + handlePlainEndpoint(args, ag, cfg) default: fmt.Println("[plain] unknown command:", cmd, "- /help for the list") } @@ -180,6 +177,359 @@ func switchPlainSelection(ag *agent.Agent, cfg *config.Config, vendor, endpoint, 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("usage: /provider [endpoint] | --list [--all] | --add-endpoint ...") + 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("[plain] unknown /provider option:", mode, "- /help for the list") + } +} + +// 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("[plain] config 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("vendor %s:\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("vendor %s: (no endpoints)\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("[plain] config 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("usage: /endpoint | --show-models | --refresh-models | --add-model ...") + 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("[plain] unknown /endpoint option:", mode, "- /help for the list") + } +} + +// 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("[plain] config 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("models on %s/%s:\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("[plain] config 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("[plain] config 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) +} + // 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 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/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 From ca2ddaf5bffb3dd0dbcaaefadc07431630ceb27e Mon Sep 17 00:00:00 2001 From: Junjun Zhang Date: Fri, 30 Oct 2026 13:30:19 +0800 Subject: [PATCH 10/24] test(provider): ForceDiscoverModels bypasses within-TTL cache (#4604) Live-cache eviction semantics the linear --refresh-models depends on: force refetches (2 network hits), and the fresh list is then served from cache with no further traffic. Co-Authored-By: ggcode --- internal/provider/model_discovery_test.go | 61 +++++++++++++++++++++++ 1 file changed, 61 insertions(+) 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 From 7dd73cbfac2e77b318d71031dfef1303cf6b8a84 Mon Sep 17 00:00:00 2001 From: Junjun Zhang Date: Fri, 30 Oct 2026 13:41:48 +0800 Subject: [PATCH 11/24] fix(plain): HOME confirm ran BOTH dialogs - branch by mode first (#4604) The previous dispatch called tui.ConfirmHomeDir unconditionally and only then overwrote the result with the plain variant, so --plain sessions saw the full-screen TUI dialog AND a second y/N prompt after choosing continue. Select the variant before invoking either one. Verified in a real HOME launch (TERM=dumb): exactly one y/N confirm with the shared i18n copy, then the linear banner - no tea dialog. Co-Authored-By: ggcode --- cmd/ggcode/root.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/cmd/ggcode/root.go b/cmd/ggcode/root.go index 3cc1a16ee..221385397 100644 --- a/cmd/ggcode/root.go +++ b/cmd/ggcode/root.go @@ -156,12 +156,15 @@ func NewRootCmd() *cobra.Command { if cfg.Language != "" { lang = tui.NormalizeLanguage(cfg.Language) } - // #4604: the linear frontend cannot host the full-screen - // Bubble Tea dialog (garbled on dumb terminals) - use the - // line-oriented y/N twin with the same i18n copy. - confirmed := 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.") From f5389efbc28f70b2d1ea1fed14bea1aee2a0c3ff Mon Sep 17 00:00:00 2001 From: Junjun Zhang Date: Fri, 30 Oct 2026 13:50:09 +0800 Subject: [PATCH 12/24] feat(plain): i18n catalogs + /lang runtime switch (#4604) The linear frontend printed a fixed mix of English strings and (for the HOME confirm) whatever the TUI i18n table held - mixed-language output by construction. - plain_i18n.go: self-contained en/zh-CN catalogs (60 keys) covering every user-visible string; plainTr lookup with English then raw-key fallback so output never blanks - /lang : hot-switches the session language and persists to cfg.Language - the same GLOBAL field the TUI /lang writes, so all modes and future sessions inherit it (no per-session language store exists; consistent with TUI semantics) - startup derives plainLang from cfg.Language (empty = en) Verified: /lang zh-CN switches help/banners live; restart shows the Chinese banner (persisted); switched back to en. 4 new tests (catalog key parity, fallback chain, POSIX-locale derivation, nil-safe cfg accessor); cmd/ggcode package PASS. Co-Authored-By: ggcode --- cmd/ggcode/plain.go | 87 ++++++++++------- cmd/ggcode/plain_i18n.go | 177 ++++++++++++++++++++++++++++++++++ cmd/ggcode/plain_i18n_test.go | 78 +++++++++++++++ 3 files changed, 305 insertions(+), 37 deletions(-) create mode 100644 cmd/ggcode/plain_i18n.go create mode 100644 cmd/ggcode/plain_i18n_test.go diff --git a/cmd/ggcode/plain.go b/cmd/ggcode/plain.go index afcf88c8b..9584fe25e 100644 --- a/cmd/ggcode/plain.go +++ b/cmd/ggcode/plain.go @@ -39,6 +39,7 @@ import ( "github.com/topcheer/ggcode/internal/permission" "github.com/topcheer/ggcode/internal/provider" "github.com/topcheer/ggcode/internal/tool" + "github.com/topcheer/ggcode/internal/tui" ) // isDumbTerminal reports whether the environment cannot host the full TUI: @@ -65,26 +66,27 @@ func plainApproval(_ context.Context, toolName, _ string) permission.Decision { // 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("[plain] ask_user disabled - the agent ends its turn and you answer by typing the next line") + fmt.Println(plainTr(plainLang, "askuser.off")) } if ag != nil { ag.SetApprovalHandler(plainApproval) } - fmt.Println("ggcode linear frontend (no TUI) - /help for commands, Ctrl-D to exit") + fmt.Println(plainTr(plainLang, "banner")) for _, n := range startupNotices { - fmt.Println("[notice]", n) + 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("ggcode> ") + fmt.Print(plainTr(plainLang, "prompt")) if !sc.Scan() { fmt.Println() return 0 @@ -113,33 +115,25 @@ func handlePlainCommand(line string, ag *agent.Agent, cfg *config.Config) bool { case "/exit", "/quit", "/q": return true case "/help", "/?": - fmt.Println("commands:") - fmt.Println(" /status show active vendor/endpoint/model") - fmt.Println(" /model hot-switch model (persists)") - fmt.Println(" /provider [ep] switch vendor (optionally endpoint)") - fmt.Println(" /provider --list [--all] list endpoints+models (default: keyed+modelled only)") - fmt.Println(" /provider --add-endpoint ... add endpoint to active vendor") - fmt.Println(" /endpoint switch endpoint on current vendor") - fmt.Println(" /endpoint --show-models all models on the active endpoint") - fmt.Println(" /endpoint --refresh-models force model refetch, then show") - fmt.Println(" /endpoint --add-model ... add a model to the active endpoint") - fmt.Println(" /endpoint --set-apikey ... set the API key on the active endpoint") - fmt.Println(" /exit leave the REPL (also Ctrl-D)") - fmt.Println("anything else is sent to the agent; approvals default to Accept") - fmt.Println("(GGCODE_PLAIN_APPROVE=deny rejects instead); ask_user is disabled.") + 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("[plain] config unavailable") + fmt.Println(plainTr(plainLang, "cfg.unavailable")) return false } if resolved, _, err := agentruntime.ResolveCurrentSelection(cfg); err != nil { - fmt.Println("[plain] resolve failed:", err) + 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("usage: /model ") + fmt.Println(plainTr(plainLang, "usage.model")) return false } switchPlainSelection(ag, cfg, "", "", args[0]) @@ -147,8 +141,27 @@ func handlePlainCommand(line string, ag *agent.Agent, cfg *config.Config) bool { 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("[plain] unknown command:", cmd, "- /help for the list") + fmt.Println(plainTr(plainLang, "cmd.unknown"), cmd) } return false } @@ -159,19 +172,19 @@ func handlePlainCommand(line string, ag *agent.Agent, cfg *config.Config) bool { // 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("[plain] config unavailable") + fmt.Println(plainTr(plainLang, "cfg.unavailable")) return } resolved, prov, err := agentruntime.ActivateCurrentSelection(cfg, vendor, endpoint, model) if err != nil { - fmt.Println("[plain] switch failed:", err) + fmt.Println(plainTr(plainLang, "switch.failed"), err) return } if ag != nil { agentruntime.ApplyProviderToAgent(ag, prov, resolved) } if err := cfg.Save(); err != nil { - fmt.Println("[plain] switch applied but not persisted:", err) + 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) @@ -199,7 +212,7 @@ func plainArgsAreFlags(args []string) bool { func handlePlainProvider(args []string, ag *agent.Agent, cfg *config.Config) { if !plainArgsAreFlags(args) { if len(args) < 1 || len(args) > 2 { - fmt.Println("usage: /provider [endpoint] | --list [--all] | --add-endpoint ...") + fmt.Println(plainTr(plainLang, "usage.provider")) return } ep := "" @@ -235,7 +248,7 @@ func handlePlainProvider(args []string, ag *agent.Agent, cfg *config.Config) { } plainAddEndpoint(cfg, *name, *proto, *url, *model, *ctxWin, *maxOut, *apikey) default: - fmt.Println("[plain] unknown /provider option:", mode, "- /help for the list") + fmt.Println(plainTr(plainLang, "opt.unknown"), mode) } } @@ -244,7 +257,7 @@ func handlePlainProvider(args []string, ag *agent.Agent, cfg *config.Config) { // --all includes the rest, annotated. Up to 10 models per endpoint. func plainListEndpoints(cfg *config.Config, all bool) { if cfg == nil { - fmt.Println("[plain] config unavailable") + fmt.Println(plainTr(plainLang, "cfg.unavailable")) return } vendors := make([]string, 0, len(cfg.Vendors)) @@ -268,7 +281,7 @@ func plainListEndpoints(cfg *config.Config, all bool) { continue } if shown == 0 { - fmt.Printf("vendor %s:\n", vid) + fmt.Printf(plainTr(plainLang, "list.vendor")+"\n", vid) } shown++ mark := " " @@ -303,7 +316,7 @@ func plainListEndpoints(cfg *config.Config, all bool) { } } if shown == 0 && all { - fmt.Printf("vendor %s: (no endpoints)\n", vid) + fmt.Printf(plainTr(plainLang, "list.noendpoints")+"\n", vid) } } } @@ -312,7 +325,7 @@ func plainListEndpoints(cfg *config.Config, all bool) { // 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("[plain] config unavailable") + fmt.Println(plainTr(plainLang, "cfg.unavailable")) return } if name == "" || proto == "" || baseURL == "" { @@ -371,7 +384,7 @@ func switchHint(model string) string { func handlePlainEndpoint(args []string, ag *agent.Agent, cfg *config.Config) { if !plainArgsAreFlags(args) { if len(args) != 1 { - fmt.Println("usage: /endpoint | --show-models | --refresh-models | --add-model ...") + fmt.Println(plainTr(plainLang, "usage.endpoint")) return } switchPlainSelection(ag, cfg, "", args[0], "") @@ -402,7 +415,7 @@ func handlePlainEndpoint(args []string, ag *agent.Agent, cfg *config.Config) { } plainSetEndpointAPIKey(cfg, *apikey) default: - fmt.Println("[plain] unknown /endpoint option:", mode, "- /help for the list") + fmt.Println(plainTr(plainLang, "opt.unknown.ep"), mode) } } @@ -410,7 +423,7 @@ func handlePlainEndpoint(args []string, ag *agent.Agent, cfg *config.Config) { // static list merged with live discovery (cache-aware; force refetches). func plainShowModels(cfg *config.Config, force bool) { if cfg == nil { - fmt.Println("[plain] config unavailable") + fmt.Println(plainTr(plainLang, "cfg.unavailable")) return } resolved, _, err := agentruntime.ResolveCurrentSelection(cfg) @@ -435,7 +448,7 @@ func plainShowModels(cfg *config.Config, force bool) { epModels = ep.Models } } - fmt.Printf("models on %s/%s:\n", resolved.VendorID, resolved.EndpointID) + fmt.Printf(plainTr(plainLang, "models.title")+"\n", resolved.VendorID, resolved.EndpointID) for _, m := range epModels { if !seen[m] { seen[m] = true @@ -458,7 +471,7 @@ func plainShowModels(cfg *config.Config, force bool) { // resolution when present). func plainSetEndpointAPIKey(cfg *config.Config, apikey string) { if cfg == nil { - fmt.Println("[plain] config unavailable") + fmt.Println(plainTr(plainLang, "cfg.unavailable")) return } if apikey == "" { @@ -491,7 +504,7 @@ func plainSetEndpointAPIKey(cfg *config.Config, apikey string) { // is the target by definition. func plainAddModel(cfg *config.Config, model, apikey string) { if cfg == nil { - fmt.Println("[plain] config unavailable") + fmt.Println(plainTr(plainLang, "cfg.unavailable")) return } if model == "" { diff --git a/cmd/ggcode/plain_i18n.go b/cmd/ggcode/plain_i18n.go new file mode 100644 index 000000000..63a575347 --- /dev/null +++ b/cmd/ggcode/plain_i18n.go @@ -0,0 +1,177 @@ +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", + "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] 配置不可用", + "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) + } +} From 1da848471cfbe6a0f58683b17cf7cdec66df89ce Mon Sep 17 00:00:00 2001 From: Junjun Zhang Date: Fri, 30 Oct 2026 13:51:42 +0800 Subject: [PATCH 13/24] test(plain): /lang switching semantics on the nil-cfg path (#4604) Unsupported language and missing args refuse without mutating plainLang; valid targets switch en<->zh-CN in-memory. Companion to f5389efbc. Co-Authored-By: ggcode --- cmd/ggcode/plain_test.go | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/cmd/ggcode/plain_test.go b/cmd/ggcode/plain_test.go index a63e0ba2b..2c4a35c39 100644 --- a/cmd/ggcode/plain_test.go +++ b/cmd/ggcode/plain_test.go @@ -84,6 +84,44 @@ func TestPlainApprovalDefaultAllow(t *testing.T) { } } +// 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) + } +} + // 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) { From 97bd01f162be00c907778e626032290c3babfc9e Mon Sep 17 00:00:00 2001 From: Junjun Zhang Date: Fri, 30 Oct 2026 13:53:37 +0800 Subject: [PATCH 14/24] docs: plain mode guide + --plain in CLI reference (#4604) New docs/guide/plain-mode.md covering the linear frontend: entry (--plain / TERM=dumb auto-select), the full slash-command table including the provider/endpoint management subcommands and /lang, --add-endpoint vs --add-model semantics, behavior differences (approval default, ask_user off, line-oriented HOME confirm, global language). --plain was previously absent from the CLI reference flag table despite existing. Co-Authored-By: ggcode --- docs/guide/cli-reference.md | 1 + docs/guide/plain-mode.md | 66 +++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+) create mode 100644 docs/guide/plain-mode.md diff --git a/docs/guide/cli-reference.md b/docs/guide/cli-reference.md index 068371b7d..c061a3523 100644 --- a/docs/guide/cli-reference.md +++ b/docs/guide/cli-reference.md @@ -280,5 +280,6 @@ 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) | | `-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..b2aaae831 --- /dev/null +++ b/docs/guide/plain-mode.md @@ -0,0 +1,66 @@ +# 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. + +## 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 + +- **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. + +## Related + +- [CLI Reference](cli-reference.md) — global flags +- [Configuration](configuration.md) — `vendors`/`endpoints`/`language` schema From b8b7d74fb8a1870c9d72de5bb1a1e9673c9db913 Mon Sep 17 00:00:00 2001 From: Junjun Zhang Date: Fri, 30 Oct 2026 14:14:59 +0800 Subject: [PATCH 15/24] feat(plain): opt out of autopilot injections, keep bypass surface (#4604) On a bypass/autopilot default the linear REPL inherited two cruise- mode injections that fight its line-by-line model: the goal-collection meta-instruction at turn start, and the end-of-turn strategist prompt that re-enters the loop after every plain answer - a text turn never ended the run. Agent.SuppressAutopilotInjection() short-circuits both injection sites (goal collection + strategist gate) while leaving the permission surface untouched - the mode stays bypass, plainApproval stays the approval decision point. No TUI/pipe/daemon behavior change: they never set the flag. Verified: plain Q&A turn ('1+1') answers and returns to the prompt in one pass, no GOAL line, no strategist banner; agent (38.6s) and cmd (22.4s) packages PASS. Co-Authored-By: ggcode --- cmd/ggcode/plain.go | 7 +++++++ internal/agent/agent.go | 3 ++- internal/agent/agent_autopilot.go | 20 ++++++++++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/cmd/ggcode/plain.go b/cmd/ggcode/plain.go index 9584fe25e..6a0c3ce3c 100644 --- a/cmd/ggcode/plain.go +++ b/cmd/ggcode/plain.go @@ -75,6 +75,13 @@ func RunLinearREPL(ag *agent.Agent, registry *tool.Registry, cfg *config.Config, } 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() } fmt.Println(plainTr(plainLang, "banner")) diff --git a/internal/agent/agent.go b/internal/agent/agent.go index 19e7b0a76..aa62b3cf8 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 @@ -3318,7 +3319,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 } From d820eef667d43ff1f2a46f2d117cbc9eaac11b0d Mon Sep 17 00:00:00 2001 From: Junjun Zhang Date: Fri, 30 Oct 2026 14:17:33 +0800 Subject: [PATCH 16/24] test(agent): SuppressAutopilotInjection contract (#4604) Baseline (cruise injects exactly once via the asked latch), suppressed (zero injections, latch stays false so later re-checks see clean state), and the mode-intact guarantee (opt-out does not downgrade the permission surface; idempotent; nil-safe). Companion to b8b7d74fb. Co-Authored-By: ggcode --- internal/agent/agent_autopilot_test.go | 86 ++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 internal/agent/agent_autopilot_test.go 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 +} From b97c25d41f3171f321b8cda37b55b17924f99ebc Mon Sep 17 00:00:00 2001 From: Junjun Zhang Date: Fri, 30 Oct 2026 14:18:15 +0800 Subject: [PATCH 17/24] docs: plain mode suppresses autopilot injections (#4604) Co-Authored-By: ggcode --- docs/guide/plain-mode.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/guide/plain-mode.md b/docs/guide/plain-mode.md index b2aaae831..4d9267353 100644 --- a/docs/guide/plain-mode.md +++ b/docs/guide/plain-mode.md @@ -55,6 +55,7 @@ Appends a model to the **active endpoint** (deduplicated; optional `--apikey` up ## 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. From 48914ff005a321e33842e85a6cc6e230145989a4 Mon Sep 17 00:00:00 2001 From: Junjun Zhang Date: Fri, 30 Oct 2026 14:42:13 +0800 Subject: [PATCH 18/24] feat(plain): tool call status suffix (ok)/(error) (#4604) ToolCallDone prints the call line without a newline; the matching ToolResult (paired by tool-call ID) appends ' (ok)' or ' (error)'. Unmatched results are dropped (no printed line to attach to); calls whose result never arrives get their line closed by Done so output does not run on. agent-unavailable message joins the i18n catalogs. Verified live: '[tool] glob (ok)' / multi-tool turns; cmd/ggcode package PASS. Co-Authored-By: ggcode --- cmd/ggcode/plain.go | 28 ++++++++++++++++++++++++++-- cmd/ggcode/plain_i18n.go | 2 ++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/cmd/ggcode/plain.go b/cmd/ggcode/plain.go index 6a0c3ce3c..be67d6066 100644 --- a/cmd/ggcode/plain.go +++ b/cmd/ggcode/plain.go @@ -556,20 +556,44 @@ func plainAddModel(cfg *config.Config, model, apikey string) { // ANSI beyond what NO_COLOR already disabled. func runLinearTurn(ctx context.Context, ag *agent.Agent, line string) { if ag == nil { - fmt.Println("[plain] agent unavailable") + fmt.Println(plainTr(plainLang, "agent.unavailable")) return } + // pendingToolCalls tracks tool calls whose result event has not arrived + // yet, so Done can close their lines (no result = terminated turn). + pendingToolCalls := make(map[string]bool) err := ag.RunStream(ctx, line, func(event provider.StreamEvent) { switch event.Type { case provider.StreamEventText: fmt.Print(event.Text) case provider.StreamEventToolCallDone: if event.Tool.Name != "" { - fmt.Printf("\n[tool] %s\n", 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) case provider.StreamEventDone: + // Any call still pending never got a result event - close its + // line so the next output does not run on. + for id := range pendingToolCalls { + delete(pendingToolCalls, id) + fmt.Println() + break // one newline per unterminated tail, not per call + } fmt.Println() } }) diff --git a/cmd/ggcode/plain_i18n.go b/cmd/ggcode/plain_i18n.go index 63a575347..8b0e281de 100644 --- a/cmd/ggcode/plain_i18n.go +++ b/cmd/ggcode/plain_i18n.go @@ -36,6 +36,7 @@ var plainCatalogs = map[tui.Language]map[string]string{ "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:", @@ -97,6 +98,7 @@ var plainCatalogs = map[tui.Language]map[string]string{ "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] 已切换但未持久化：", From 092b1f5c11bb0eb406eaede21718ecdd1eab8ad1 Mon Sep 17 00:00:00 2001 From: Junjun Zhang Date: Fri, 30 Oct 2026 15:00:23 +0800 Subject: [PATCH 19/24] test(plain): extract stream printer, cover pairing contract (#4604) plainStreamPrinter extracted from runLinearTurn so the tool-call status contract is unit-testable: (ok)/(error) suffixes pair by call ID, ghost results drop, Done closes one tail newline. The tests caught a real leak: Done drained only ONE pending call (break in the loop), so a second unterminated call's stale ID could pair with a later turn's result. Done now clears the whole map while printing exactly one tail newline. cmd/ggcode PASS. Co-Authored-By: ggcode --- cmd/ggcode/plain.go | 27 +++++++++----- cmd/ggcode/plain_test.go | 81 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 9 deletions(-) diff --git a/cmd/ggcode/plain.go b/cmd/ggcode/plain.go index be67d6066..c44f581af 100644 --- a/cmd/ggcode/plain.go +++ b/cmd/ggcode/plain.go @@ -562,7 +562,19 @@ func runLinearTurn(ctx context.Context, ag *agent.Agent, line string) { // pendingToolCalls tracks tool calls whose result event has not arrived // yet, so Done can close their lines (no result = terminated turn). pendingToolCalls := make(map[string]bool) - err := ag.RunStream(ctx, line, func(event provider.StreamEvent) { + err := ag.RunStream(ctx, line, plainStreamPrinter(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) @@ -587,17 +599,14 @@ func runLinearTurn(ctx context.Context, ag *agent.Agent, line string) { case provider.StreamEventError: fmt.Printf("\n[error] %v\n", event.Error) case provider.StreamEventDone: - // Any call still pending never got a result event - close its - // line so the next output does not run on. - for id := range pendingToolCalls { - delete(pendingToolCalls, id) + // Any call still pending never got a result event - close the + // unterminated tail (one newline total, not per call) and drain + // the map so stale ids cannot pair with a later turn's results. + if len(pendingToolCalls) > 0 { fmt.Println() - break // one newline per unterminated tail, not per call } + clear(pendingToolCalls) fmt.Println() } - }) - if err != nil { - fmt.Printf("\n[error] turn failed: %v\n", err) } } diff --git a/cmd/ggcode/plain_test.go b/cmd/ggcode/plain_test.go index 2c4a35c39..78f825985 100644 --- a/cmd/ggcode/plain_test.go +++ b/cmd/ggcode/plain_test.go @@ -2,10 +2,13 @@ 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 @@ -122,6 +125,84 @@ func TestHandlePlainCommandLang(t *testing.T) { } } +// 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"}}, + {Type: provider.StreamEventDone}, + } + 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) + } + _ = 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 Done, 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"}}, + {Type: provider.StreamEventDone}, + } + 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) + } + _ = 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 Done newline = 4 total, ending + // in a blank line - 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 Done), got %d:\n%q", n, got) + } + if !strings.HasSuffix(got, "\n\n") { + t.Errorf("output must end with the closed tail line + Done newline, got:\n%q", got) + } + if len(pending) != 0 { + t.Errorf("pending map must be drained, got %v", pending) + } +} + // 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) { From ee5bc6d067e5baf952f863f3dd8c438454c04d86 Mon Sep 17 00:00:00 2001 From: Junjun Zhang Date: Fri, 30 Oct 2026 15:01:26 +0800 Subject: [PATCH 20/24] docs: plain mode output format - tool status lines (#4604) Co-Authored-By: ggcode --- docs/guide/plain-mode.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/guide/plain-mode.md b/docs/guide/plain-mode.md index 4d9267353..a8337e2d3 100644 --- a/docs/guide/plain-mode.md +++ b/docs/guide/plain-mode.md @@ -11,6 +11,17 @@ 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. From c7a9e4dfa069f1c51bd097a007d796b3b22ff499 Mon Sep 17 00:00:00 2001 From: Junjun Zhang Date: Fri, 30 Oct 2026 15:19:44 +0800 Subject: [PATCH 21/24] fix(plain): tool status pairing - turn boundary is RunStream return (#4604) Real-world event order broke the (ok)/(error) suffixes: provider streams emit StreamEventDone per iteration - and on OpenAI-compatible providers each carries Usage - so treating any Done as the turn end cleared the pending map right before the ToolResult arrived between iterations. The suffix never printed. StreamEventDone is now inert in the printer; plainCloseTurn runs after RunStream returns: one tail-closing newline (max) + separator. Regression test pins the interleaved order (Done between call and result, with Usage - exactly what zai/glm emits). Verified live: single-tool turns print '[tool] glob (ok)' stably; multi-tool turns print (ok)/(error) suffixes. cmd/ggcode PASS. Co-Authored-By: ggcode --- cmd/ggcode/plain.go | 25 ++++++++++++-------- cmd/ggcode/plain_test.go | 51 +++++++++++++++++++++++++++++++++------- 2 files changed, 58 insertions(+), 18 deletions(-) diff --git a/cmd/ggcode/plain.go b/cmd/ggcode/plain.go index c44f581af..5509d2175 100644 --- a/cmd/ggcode/plain.go +++ b/cmd/ggcode/plain.go @@ -560,9 +560,12 @@ func runLinearTurn(ctx context.Context, ag *agent.Agent, line string) { return } // pendingToolCalls tracks tool calls whose result event has not arrived - // yet, so Done can close their lines (no result = terminated turn). + // 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) } @@ -598,15 +601,17 @@ func plainStreamPrinter(pendingToolCalls map[string]bool) func(provider.StreamEv delete(pendingToolCalls, event.Tool.ID) case provider.StreamEventError: fmt.Printf("\n[error] %v\n", event.Error) - case provider.StreamEventDone: - // Any call still pending never got a result event - close the - // unterminated tail (one newline total, not per call) and drain - // the map so stale ids cannot pair with a later turn's results. - if len(pendingToolCalls) > 0 { - fmt.Println() - } - clear(pendingToolCalls) - fmt.Println() } } } + +// 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_test.go b/cmd/ggcode/plain_test.go index 78f825985..6b613ed60 100644 --- a/cmd/ggcode/plain_test.go +++ b/cmd/ggcode/plain_test.go @@ -137,7 +137,6 @@ func TestPlainStreamPrinterToolStatusPairing(t *testing.T) { {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"}}, - {Type: provider.StreamEventDone}, } pending := make(map[string]bool) printer := plainStreamPrinter(pending) @@ -148,6 +147,7 @@ func TestPlainStreamPrinterToolStatusPairing(t *testing.T) { for _, e := range events { printer(e) } + plainCloseTurn(pending) _ = w.Close() os.Stdout = orig _, _ = io.Copy(&out, r) @@ -162,7 +162,7 @@ func TestPlainStreamPrinterToolStatusPairing(t *testing.T) { 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 Done, got %v", pending) + t.Errorf("pending map must be drained after close, got %v", pending) } } @@ -173,7 +173,11 @@ 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) @@ -184,25 +188,56 @@ func TestPlainStreamPrinterDoneClosesUnterminatedTail(t *testing.T) { 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 Done newline = 4 total, ending - // in a blank line - never one closing newline per call. + // 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 Done), got %d:\n%q", n, got) - } - if !strings.HasSuffix(got, "\n\n") { - t.Errorf("output must end with the closed tail line + Done newline, got:\n%q", got) + 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) + } +} + // 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) { From 1d233db0a7e72d7c69b1fe37c1d38874f28e9cac Mon Sep 17 00:00:00 2001 From: Junjun Zhang Date: Fri, 30 Oct 2026 16:17:54 +0800 Subject: [PATCH 22/24] feat(plain): --persist-session opt-in message persistence (#4604) Plain sessions were memory-only: SetPersistHandler is wired by the TUI/daemon only, so every plain conversation was lost on exit. --persist-session attaches the same per-message JSONL append (via wirePlainPersist: NewSession snapshot + AppendMessageToDisk, best- effort - failures log, never block the REPL). Write-only by design: the JSONL is deliberately NOT indexed (no store.Save), so auto-resume and the picker never pick it up and the next plain start stays fresh; explicit --resume can still read the file. Verified live: flag on creates '-.jsonl' with the turn, prints the session id; flag off creates nothing; a follow-up plain start does not auto-load it. cmd/ggcode PASS. Co-Authored-By: ggcode --- cmd/ggcode/plain.go | 44 ++++++++++++++++++++++++++++++++++++++++ cmd/ggcode/plain_test.go | 10 +++++++++ cmd/ggcode/root.go | 5 +++++ 3 files changed, 59 insertions(+) diff --git a/cmd/ggcode/plain.go b/cmd/ggcode/plain.go index 5509d2175..7e6153ef1 100644 --- a/cmd/ggcode/plain.go +++ b/cmd/ggcode/plain.go @@ -38,6 +38,7 @@ import ( "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" ) @@ -84,6 +85,15 @@ func RunLinearREPL(ag *agent.Agent, registry *tool.Registry, cfg *config.Config, 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) @@ -550,6 +560,40 @@ func plainAddModel(cfg *config.Config, model, apikey string) { 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 diff --git a/cmd/ggcode/plain_test.go b/cmd/ggcode/plain_test.go index 6b613ed60..ac6ee3da5 100644 --- a/cmd/ggcode/plain_test.go +++ b/cmd/ggcode/plain_test.go @@ -238,6 +238,16 @@ func TestPlainStreamPrinterIntermediateDoneKeepsPairing(t *testing.T) { } } +// 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) { diff --git a/cmd/ggcode/root.go b/cmd/ggcode/root.go index ddd829fb8..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 @@ -228,6 +232,7 @@ func NewRootCmd() *cobra.Command { _ = 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, "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). From c069ebfd0cc0e223db85e057850d5e7b43bd2bb7 Mon Sep 17 00:00:00 2001 From: Junjun Zhang Date: Fri, 30 Oct 2026 16:19:23 +0800 Subject: [PATCH 23/24] test: pin --persist-session CLI surface - opt-in default off (#4604) Co-Authored-By: ggcode --- cmd/ggcode/root_test.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) 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 { From 4b51470cdb036568d994979bce301b629592dfd4 Mon Sep 17 00:00:00 2001 From: Junjun Zhang Date: Fri, 30 Oct 2026 16:52:29 +0800 Subject: [PATCH 24/24] docs: --persist-session flag in cli-reference and plain guide (#4604) Co-Authored-By: ggcode --- docs/guide/cli-reference.md | 1 + docs/guide/plain-mode.md | 1 + 2 files changed, 2 insertions(+) diff --git a/docs/guide/cli-reference.md b/docs/guide/cli-reference.md index c061a3523..178dc3987 100644 --- a/docs/guide/cli-reference.md +++ b/docs/guide/cli-reference.md @@ -281,5 +281,6 @@ ggcode version # Print version, commit, and build date | `--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 index a8337e2d3..1df99012b 100644 --- a/docs/guide/plain-mode.md +++ b/docs/guide/plain-mode.md @@ -71,6 +71,7 @@ Appends a model to the **active endpoint** (deduplicated; optional `--apikey` up - **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