fix: persist one-shot sessions when --save-session is enabled

[Desperado]

Summary

• --save-session (and a persisted auto_save=true) only set appConfig.AutoSave; the one-shot dispatch path (-p / positional-arg mode) never read that flag or called session.SaveSession, so scripted/CI single-prompt runs were never persisted regardless of the setting.
• Added saveOneShotSession, called after the direct-API and Cerebras branches (the two paths that populate ag.History), gated by a small shouldSaveOneShotSession helper.
• The cc/codex CLI backends are intentionally untouched — they manage their own native --resume session state, so there's nothing for qmax-code's session store to persist there. Documented in README.

Test plan

• go build ./...
• go test ./...
• Added TestShouldSaveOneShotSession (gate logic) and TestOneShotSessionPersistsToDisk (real save/load round trip against a temp $HOME) in main_oneshot_test.go

[sigilix]

Sigilix Overview

Effort: 3/5 (medium)

Quality gates

• ✅ PR title follows convention
• ✅ PR description is complete
• ℹ️ PR is linked to an issue — No Closes #N / Closes SIG-N keyword found in PR body or commit messages.

Summary — latest push

Fixes one-shot sessions (invoked via -p or positional argument) so they are persisted to disk when --save-session is passed or auto_save=true is configured, closing a gap where scripted/CI runs silently discarded session history. The --save-session flag explicitly overrides a persisted auto_save=false without mutating the on-disk config, and save failures are now surfaced to stderr rather than swallowed. A P2 testing finding notes that the saveOneShotSession closure itself lacks direct unit coverage, though the gate logic and round-trip persistence are tested.

Important files

File	Score	Notes	Next step
main.go	4/5	Adds the --save-session flag, wires it to override appConfig.AutoSave at runtime, and calls saveOneShotSession() after the direct-API and Cerebras one-shot branches to persist ag.History.	Add a unit or integration test that exercises the saveOneShotSession closure directly to confirm it calls session.SaveSession with the correct arguments and handles errors, closing the P2 testing gap.
main_oneshot_test.go	3/5	Adds unit tests for applySaveSessionFlag, shouldSaveOneShotSession, and a round-trip persistence test TestOneShotSessionPersistsToDisk.	Add a test case verifying behavior when session.SaveSession fails (e.g., read-only HOME) to confirm the silent discard is acceptable or needs handling.
README.md	1/5	Documents the --save-session flag behavior for one-shot and interactive sessions and clarifies it does not apply to cc/codex backends.	Add a cross-reference link to the --resume documentation so users can immediately see how to resume a saved one-shot session.

Confidence: 4/5

The change has a narrow, well-scoped surface with explicit gate logic and round-trip tests, though the closure wiring in main.go lacks direct unit coverage for the save call itself.

• Verify that applySaveSessionFlag correctly overrides auto_save=false without writing back to the config file on disk, ensuring the per-run flag remains ephemeral.
• Confirm that saveOneShotSession() is unreachable on the cliAgent path (lines ~447-460), matching the documented intent that cc/codex backends manage their own native --resume state.
• Check that oneShotSessionID is generated before the one-shot runs so a mid-run crash still leaves a partial session file, and verify no ID collisions occur under rapid CI invocation.
• Ensure the fmt.Fprintf(os.Stderr, ...) error path for failed saves is sufficient for CI visibility and doesn't need a non-zero exit code to signal failure.

Suggested labels: bug

---

_{Posted · c5d026f · 1 finding — View review
Proof: 1 model-only
runner-verified = CI receipt · reproduced = sandbox observed diff · grounded = deterministic detector/worker-token · model-only = model judgment only
Dismiss @sigilix dismiss <reason> (not-a-bug | bad-anchor | already-covered | too-minor | wrong-context) · Re-run /sigilix review}

[qualitymaxapp]

✅ QualityMax Pipeline

Gate	Result
🔍 AI Review	✅ Clean
🧪 Repo Tests	✅ 415/415 passed (go)
🤖 AI Tests	✅ 47/51 passed

_{Powered by QualityMax — AI-Powered Test Automation}

[sigilix]

One-shot session save discards SaveSession error silently

The saveOneShotSession closure calls session.SaveSession and discards the error (_ = session.SaveSession(...)). The sibling handler for saving sessions (the /save command in repl.go) prints the error to the user with fmt.Sprintf("Failed to save: %v", err). If SaveSession fails for the one-shot path (e.g., disk full, permission denied), the user receives no indication that the session was not persisted, potentially losing the session without knowing. Fix: log the error to stderr or propagate it so the user can be made aware.

More Info

• Threat model: User runs qmax-code -p ... with --save-session, expects session to be saved. If SaveSession fails, the session is lost silently and the user cannot resume later.
• Specific code citations: Rule-establishing sibling: repl.go /save handler (retrieval snapshot lines 301-315) calls session.SaveSession and prints error on failure. Violating site: main.go new saveOneShotSession closure (line ~429) uses _ = session.SaveSession(...).
• Existing protections: None — error is unconditionally discarded.
• Proposed mitigation: Replace _ = session.SaveSession(...) with an error check that logs the failure to stderr, e.g., if err := session.SaveSession(...); err != nil { fmt.Fprintf(os.Stderr, "Failed to save session: %v\n", err) }.
• Alternative mitigations considered: Returning the error would change the calling signature and may not be appropriate after successful agent output; logging is sufficient.
• Severity calibration: Score 2: silent data loss that only triggers under disk/permission edge cases, but sibling handler shows the expected user-visible error contract.

Prompt To Fix With AI

This is a comment left during a code review.
Path: main.go
Line: 436

Comment:
**One-shot session save discards SaveSession error silently**

The `saveOneShotSession` closure calls `session.SaveSession` and discards the error (`_ = session.SaveSession(...)`). The sibling handler for saving sessions (the `/save` command in repl.go) prints the error to the user with `fmt.Sprintf("Failed to save: %v", err)`. If SaveSession fails for the one-shot path (e.g., disk full, permission denied), the user receives no indication that the session was not persisted, potentially losing the session without knowing. Fix: log the error to stderr or propagate it so the user can be made aware.

Threat model:
User runs `qmax-code -p ...` with --save-session, expects session to be saved. If SaveSession fails, the session is lost silently and the user cannot resume later.

Specific code citations:
Rule-establishing sibling: `repl.go` /save handler (retrieval snapshot lines 301-315) calls `session.SaveSession` and prints error on failure. Violating site: `main.go` new `saveOneShotSession` closure (line ~429) uses `_ = session.SaveSession(...)`.

Existing protections:
None — error is unconditionally discarded.

Proposed mitigation:
Replace `_ = session.SaveSession(...)` with an error check that logs the failure to stderr, e.g., `if err := session.SaveSession(...); err != nil { fmt.Fprintf(os.Stderr, "Failed to save session: %v\n", err) }`.

Alternative mitigations considered:
Returning the error would change the calling signature and may not be appropriate after successful agent output; logging is sufficient.

Severity calibration:
Score 2: silent data loss that only triggers under disk/permission edge cases, but sibling handler shows the expected user-visible error contract.

How can I resolve this? If you propose a fix, please make it concise.

[Desperado]

Fixed in c5d026f — now logs Failed to save session: %v to stderr instead of discarding the error, matching the /save REPL handler's contract.

[sigilix]

Confirmed — the fix in c5d026f addresses the finding correctly. The original code in saveOneShotSession used _ = session.SaveSession(...) which silently discarded errors. The /save REPL handler at internal/repl/repl.go:307-309 checks the error and surfaces it via term.PrintError(fmt.Sprintf("Failed to save: %v", err)). Logging Failed to save session: %v to stderr in the one-shot path now matches that contract. Note: the indexed main branch (at 1249932) still shows two other SaveSession calls in internal/repl/repl.go that discard errors — autoSave() at line 73 (_ = session.SaveSession(...)) and saveAndExit() at line 81 (_ = session.SaveSession(...)). Those are REPL paths where silent failure may be acceptable (auto-save on interrupt, save-and-exit), but they're worth a quick sanity check if you want full consistency.

[sigilix]

Approved — no issues in your changes. 1 note about code outside this PR below.

1 finding outside the diff

File	Scope	Finding
main_oneshot_test.go:1	out-of-diff	TestOneShotSessionPersistsToDisk depends on an unreviewable SetHomeDir helper that may not be concurrency-safe