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..f47c6bb 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,21 @@ 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) { + 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) + } + } + runOneShot := func(prompt string) error { if cliAgent != nil { // CLI backends stream their own output (tool icons, glamour-rendered @@ -443,10 +462,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 +530,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