From e8b38f15b554b7c9ca30a11d1fda0fc8d43e7283 Mon Sep 17 00:00:00 2001 From: Ruslan Strazhnyk Date: Sun, 12 Jul 2026 18:00:10 +0200 Subject: [PATCH 1/2] fix: persist one-shot sessions when --save-session/auto_save is enabled --save-session only flipped appConfig.AutoSave, but runOneShot (the -p and positional-arg dispatch path) never consulted it or called session.SaveSession, so scripted/CI one-shot runs were silently never persisted regardless of the flag. --- README.md | 1 + main.go | 34 ++++++++++++++++++ main_oneshot_test.go | 84 ++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 112 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 6a6c420..76bdb02 100644 --- a/README.md +++ b/README.md @@ -159,6 +159,7 @@ The multimodal path works because qmax converts image attachments into OpenAI `i - Anthropic: set `ANTHROPIC_API_KEY`, pass `--anthropic-api-key`, or save it through the interactive key prompt. - QualityMax: run `qmax-code login` for browser login, or `qmax-code login --api-key qm-YOUR-API-KEY`. - QualityMax credentials are stored in `~/.qmax-code/auth.json` with `0600` permissions. Run `/disconnect` in the REPL to remove saved QualityMax auth. +- Use `qmax-code --save-session` to force saving the current session for that run, even if auto-save is disabled in the config — this applies to interactive REPL sessions as well as one-shot `-p` / positional-arg runs (so a scripted invocation can still be resumed later with `--resume last`). Not applicable to the `cc`/`codex` CLI backends, which manage their own native session/resume state. - Anthropic keys saved by the prompt are stored in the OS keychain under the `qmax-code` service; remove them with your platform keychain tool, or use `ANTHROPIC_API_KEY` for session-only auth. - Known credential patterns are redacted from API errors, command output, local test output, and optional telemetry before display or reporting. diff --git a/main.go b/main.go index 4f7a367..8b332e1 100644 --- a/main.go +++ b/main.go @@ -32,6 +32,7 @@ func main() { oneShot := flag.String("p", "", "Run a single prompt and exit (non-interactive)") resumeID := flag.String("resume", "", "Resume a previous session by ID (or 'last')") listSessions := flag.Bool("list-sessions", false, "List recent sessions and exit") + saveSession := flag.Bool("save-session", false, "Save this session on exit (overrides auto-save setting)") verbose := flag.Bool("verbose", false, "Show tool calls and raw responses") professional := flag.Bool("professional", false, "Disable cat personality, be direct and professional") quiet := flag.Bool("q", false, "Quiet mode — no banner, minimal output (for CI)") @@ -145,6 +146,9 @@ func main() { // Load persistent user config appConfig := api.LoadQMaxCodeConfig() + // --save-session is an explicit per-run opt-in. It must override a + // persisted auto_save=false setting without changing that setting on disk. + applySaveSessionFlag(appConfig, *saveSession) // Apply color theme before constructing any UI components. tui.ApplyTheme(tui.ThemeByName(appConfig.Theme)) @@ -421,6 +425,18 @@ func main() { // scripting / cloud-session invocation bypassed the cc backend and hit the // Anthropic API. Now we route through cliAgent when configured, falling // back to the API agent only when no CLI backend is active. + // One-shot sessions only accumulate into ag.History via the Cerebras and + // direct-API branches below (cliAgent runs cc/codex as a separate process + // and manages its own native --resume state, so there's nothing here for + // qmax-code's session store to persist). Generate the ID up front so a + // crash mid-run still leaves a partial session file behind. + oneShotSessionID := session.GenerateSessionID() + saveOneShotSession := func() { + if shouldSaveOneShotSession(appConfig.AutoSave, ag.History) { + _ = session.SaveSession(oneShotSessionID, ag.History, ag.Cfg.Context.ProjectID, ag.Usage, ag.Cfg.Model) + } + } + runOneShot := func(prompt string) error { if cliAgent != nil { // CLI backends stream their own output (tool icons, glamour-rendered @@ -443,10 +459,12 @@ func main() { term := tui.NewTerminal() defer term.Close() _, err := ag.RunStreaming(prompt, term) + saveOneShotSession() return err } // Direct-API path: non-streaming. Print the returned text ourselves. result, err := ag.Run(prompt) + saveOneShotSession() if err != nil { return err } @@ -509,6 +527,22 @@ func resolveModel(m string) string { return api.ResolveClaudeModel(m) } +// applySaveSessionFlag applies the explicit per-run save-session override. +// Keeping this separate makes the flag behavior testable without starting the +// interactive terminal or loading credentials. +func applySaveSessionFlag(cfg *api.Config, enabled bool) { + if enabled { + cfg.AutoSave = true + } +} + +// shouldSaveOneShotSession gates the one-shot session write: only persist +// when auto-save is on (--save-session forces this via applySaveSessionFlag) +// and there's actually a conversation to save. +func shouldSaveOneShotSession(autoSave bool, history []api.Message) bool { + return autoSave && len(history) > 0 +} + // isValidModelName reports whether m is a recognized model identifier. // QUA-579: pre-fix, any string was forwarded to the API and produced a // confusing 401/400. Now we fail fast with a list of valid choices. diff --git a/main_oneshot_test.go b/main_oneshot_test.go index cd74358..a7e1554 100644 --- a/main_oneshot_test.go +++ b/main_oneshot_test.go @@ -2,20 +2,90 @@ package main import ( "errors" + "os" "testing" "github.com/qualitymax/qmax-code/internal/agent" + "github.com/qualitymax/qmax-code/internal/api" + "github.com/qualitymax/qmax-code/internal/session" "github.com/qualitymax/qmax-code/internal/tui" ) +func TestSaveSessionFlagOverridesAutoSaveWithoutChangingOtherConfig(t *testing.T) { + cfg := api.DefaultConfig() + cfg.AutoSave = false + applySaveSessionFlag(cfg, true) + if !cfg.AutoSave { + t.Fatal("--save-session should enable auto-save for the current run") + } +} + +func TestSaveSessionFlagDoesNotChangeDisabledConfigWhenAbsent(t *testing.T) { + cfg := api.DefaultConfig() + cfg.AutoSave = false + applySaveSessionFlag(cfg, false) + if cfg.AutoSave { + t.Fatal("auto-save should remain disabled when --save-session is absent") + } +} + +func TestShouldSaveOneShotSession(t *testing.T) { + history := []api.Message{{Role: "user", Content: "hi"}} + if shouldSaveOneShotSession(false, history) { + t.Error("must not save when auto-save is off") + } + if shouldSaveOneShotSession(true, nil) { + t.Error("must not save an empty history") + } + if !shouldSaveOneShotSession(true, history) { + t.Error("must save when auto-save is on and history is non-empty") + } +} + +// TestOneShotSessionPersistsToDisk is the regression test for the gap where +// --save-session (or a persisted auto_save=true) only flipped a config field +// that the one-shot path (-p / positional-arg mode) never consulted, so +// `qmax-code -p "..."` never wrote a session file for /resume to find. This +// exercises the same shouldSaveOneShotSession + session.SaveSession pair that +// runOneShot's saveOneShotSession closure calls. +func TestOneShotSessionPersistsToDisk(t *testing.T) { + origHome := os.Getenv("HOME") + t.Cleanup(func() { os.Setenv("HOME", origHome) }) + os.Setenv("HOME", t.TempDir()) + + sessionID := session.GenerateSessionID() + history := []api.Message{ + {Role: "user", Content: "test the login flow"}, + {Role: "assistant", Content: "done"}, + } + + if !shouldSaveOneShotSession(true /* autoSave */, history) { + t.Fatal("expected gate to allow saving") + } + if err := session.SaveSession(sessionID, history, 7, api.TokenUsage{}, "sonnet"); err != nil { + t.Fatalf("SaveSession: %v", err) + } + + loaded, err := session.LoadSession(sessionID) + if err != nil { + t.Fatalf("expected one-shot session to be persisted, LoadSession failed: %v", err) + } + if len(loaded.Messages) != len(history) { + t.Errorf("loaded %d messages, want %d", len(loaded.Messages), len(history)) + } + if loaded.ProjectID != 7 { + t.Errorf("loaded ProjectID = %d, want 7", loaded.ProjectID) + } +} + // fakeCLIAgent is a tiny CLIAgent implementation that records whether its // Run method was called. Used to verify the QUA-576 dispatch fix without // needing a real claude/codex binary. type fakeCLIAgent struct { - called bool - prompt string - result string - runErr error + called bool + prompt string + result string + runErr error } func (f *fakeCLIAgent) Run(userMsg string, _ *tui.Terminal) (string, error) { @@ -23,9 +93,9 @@ func (f *fakeCLIAgent) Run(userMsg string, _ *tui.Terminal) (string, error) { f.prompt = userMsg return f.result, f.runErr } -func (f *fakeCLIAgent) Cancel() {} -func (f *fakeCLIAgent) Cleanup() {} -func (f *fakeCLIAgent) SetOutputVerbose(bool) {} +func (f *fakeCLIAgent) Cancel() {} +func (f *fakeCLIAgent) Cleanup() {} +func (f *fakeCLIAgent) SetOutputVerbose(bool) {} // dispatchForTest mirrors the dispatch logic in main.go's `runOneShot` // closure. Keeping it in sync with that closure is the regression contract From c5d026fa866baff94f9f0bf341b3ede51adf2bb4 Mon Sep 17 00:00:00 2001 From: Ruslan Strazhnyk Date: Sun, 12 Jul 2026 18:28:57 +0200 Subject: [PATCH 2/2] fix: surface one-shot session save failures instead of discarding them Matches the /save REPL handler's contract of telling the user when SaveSession fails, per sigilix review on #142. --- main.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/main.go b/main.go index 8b332e1..f47c6bb 100644 --- a/main.go +++ b/main.go @@ -432,8 +432,11 @@ func main() { // crash mid-run still leaves a partial session file behind. oneShotSessionID := session.GenerateSessionID() saveOneShotSession := func() { - if shouldSaveOneShotSession(appConfig.AutoSave, ag.History) { - _ = session.SaveSession(oneShotSessionID, ag.History, ag.Cfg.Context.ProjectID, ag.Usage, ag.Cfg.Model) + if !shouldSaveOneShotSession(appConfig.AutoSave, ag.History) { + return + } + if err := session.SaveSession(oneShotSessionID, ag.History, ag.Cfg.Context.ProjectID, ag.Usage, ag.Cfg.Model); err != nil { + fmt.Fprintf(os.Stderr, "Failed to save session: %v\n", err) } }