fix: rotate cloud sessions on project switch

[Desperado]

When /project changes the active project, finalize the existing cloud session and immediately begin a new one for the selected project.

The local conversation is retained, but cloud history and token totals are scoped to the appropriate project.

Tests:

• go test ./internal/session ./internal/repl
• go vet ./...
• go test ./... (unrelated baseline failure: internal/vnc TestDialVNCNoRetryOnNon5xx exceeds its 2s timing assertion on both this branch and main)

[sigilix]

Sigilix Overview

Effort: 4/5 (large)

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

Introduces project-scoped cloud session rotation so that switching the active project finalizes the current cloud session and starts a new one, preserving local conversation history while correctly scoping cloud uploads and token totals. The implementation adds historyStart and tokenStart tracking to CloudSessionTracker to slice the local history for each project's cloud upload. A P1 logic bug exists where SwitchProject resets the tracker state before the newly started session's fields are populated, creating a window where a concurrent CompleteCurrent call would operate on zeroed bounds.

Important files

File	Score	Notes	Next step
internal/session/cloud_session.go	5/5	Adds historyStart, tokenStart, and projectID fields to CloudSessionTracker and implements SwitchProject and CompleteCurrent to scope cloud session data per project.	Fix the state-reset race in SwitchProject where t.cloudID, t.projectID, t.historyStart, and t.tokenStart are zeroed before StartWithHistory populates the new session, risking an out-of-bounds slice or zero-token upload if CompleteCurrent is called concurrently.
internal/session/cloud_session_test.go	4/5	Adds TestCloudTracker_SwitchProject_FinalizesOldSessionAndScopesNewHistory covering the full project-switch lifecycle including per-project token accounting.	Add a test case where SwitchProject is called on a tracker with an empty cloudID to confirm it delegates to StartWithHistory and returns false, and a concurrent-finalization scenario to expose the state-reset race.
internal/repl/repl.go	3/5	Wires SwitchProject into the /project REPL command and switches start/complete calls to use the new history-aware methods.	Verify that ag.Usage.TotalTokens() and len(ag.History) are captured atomically at the point of calling SwitchProject to avoid a TOCTOU mismatch between the two values.

Confidence: 2/5

The state-reset race in SwitchProject leaves the tracker in a vulnerable intermediate state that could cause incorrect cloud uploads or out-of-bounds history slicing under concurrent access.

• Lines 89-93 in cloud_session.go: SwitchProject zeroes t.cloudID, t.projectID, t.historyStart, and t.tokenStart before calling StartWithHistory, so any intervening CompleteCurrent will see zeroed bounds and upload the full history or compute negative tokens.
• Lines 95-97 in cloud_session.go: CompleteCurrent uses t.historyStart to slice messages but the bounds check only clamps negative start to 0; if historyStart exceeds len(messages) due to the reset race, start is clamped to 0 and the entire local history leaks into the project-scoped upload.
• Line 101 in cloud_session.go: projectTokens is computed as totalTokens - t.tokenStart; after the zeroing in SwitchProject, t.tokenStart is 0, so the subtraction yields the full cumulative total rather than the per-project delta.
• Lines 226-230 in repl.go: switchCloudProject(pid) is called before ag.Cfg.Context.ProjectID is updated, meaning if SwitchProject reads the config's project ID internally in a future refactor, it would see the stale value.

Suggested labels: bug

---

_{Posted · f5628a0 · 0 findings — View review
Sigilix · 0 of 50 reviews used in past 5h}

[qualitymaxapp]

✅ QualityMax Pipeline

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

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

[sigilix]

Out-of-range historyStart uploads full replacement history to wrong cloud session

When historyStart exceeds the current len(messages) (for example, after /load replaces ag.History with a shorter saved session), CompleteCurrent resets start to 0 and uploads messages[0:]. This violates the project-scoping invariant: messages that predate the active cloud session are uploaded to the finalizing session. Cap start at len(messages) instead so the slice is empty when local history has been truncated.

Example:

historyStart = 5 after a cloud session began with 5 local messages
after /load, len(messages) = 2
actual: projectMessages = messages[0:] (uploads 2 loaded messages to the old project's cloud session)
expected: projectMessages = messages[5:] = nil (uploads nothing)

Current:

func (t *CloudSessionTracker) CompleteCurrent(client *api.APIClient, totalTokens int, messages []api.Message) {
	start := t.historyStart
	if start < 0 || start > len(messages) {
		start = 0
	}

Proposed:

start := t.historyStart
if start < 0 {
	start = 0
}
if start > len(messages) {
	start = len(messages)
}
projectMessages := messages[start:]

Suggested change

	func (t *CloudSessionTracker) CompleteCurrent(client *api.APIClient, totalTokens int, messages []api.Message) {
	start := t.historyStart
	if start < 0 || start > len(messages) {
	start = 0
	}
	start := t.historyStart
	if start < 0 {
	start = 0
	}
	if start > len(messages) {
	start = len(messages)
	}
	projectMessages := messages[start:]

More Info

• Threat model: A user who starts a cloud-tracked REPL, then /loads a shorter saved session, and later exits (or switches projects again) causes the finalizing cloud session to receive messages from the loaded session. Those messages may belong to a different project or an earlier conversation, leaking context and inflating the event count/token totals for the wrong project.
• Specific code citations: internal/session/cloud_session.go lines 117-121: CompleteCurrent slices messages[start:] after the start > len(messages) guard resets start to 0. The retrieval snapshot for internal/repl/repl.go shows /load replacing ag.History without touching the tracker.
• Existing protections: The projectTokens < 0 clamp prevents negative token totals, but there is no equivalent guard for the history slice. SwitchProject and StartWithHistory only set historyStart to len(messages) at session creation; neither /load nor any other visible path updates it when ag.History is replaced.
• Proposed mitigation: Split the bounds check so start > len(messages) caps start at len(messages), producing an empty slice when local history has been truncated. Optionally reset historyStart/tokenStart when /load replaces history, but the local cap is the minimal fix.
• Alternative mitigations considered: Resetting the tracker on /load is broader and would lose the in-progress cloud session; capping the slice preserves the existing session while avoiding cross-project data leakage.
• Severity calibration: Score 4: the failure path requires a /load after cloud session start, but when it happens it silently uploads wrong messages to the wrong project. It is not a crash but a data-scoping/information-leak bug.

Prompt To Fix With AI

This is a comment left during a code review.
Path: internal/session/cloud_session.go
Line: 117-121

Comment:
**Out-of-range historyStart uploads full replacement history to wrong cloud session**

When `historyStart` exceeds the current `len(messages)` (for example, after `/load` replaces `ag.History` with a shorter saved session), `CompleteCurrent` resets `start` to `0` and uploads `messages[0:]`. This violates the project-scoping invariant: messages that predate the active cloud session are uploaded to the finalizing session. Cap `start` at `len(messages)` instead so the slice is empty when local history has been truncated.

Example:
historyStart = 5 after a cloud session began with 5 local messages
after /load, len(messages) = 2
actual: projectMessages = messages[0:] (uploads 2 loaded messages to the old project's cloud session)
expected: projectMessages = messages[5:] = nil (uploads nothing)

Threat model:
A user who starts a cloud-tracked REPL, then `/load`s a shorter saved session, and later exits (or switches projects again) causes the finalizing cloud session to receive messages from the loaded session. Those messages may belong to a different project or an earlier conversation, leaking context and inflating the event count/token totals for the wrong project.

Specific code citations:
`internal/session/cloud_session.go` lines 117-121: `CompleteCurrent` slices `messages[start:]` after the `start > len(messages)` guard resets `start` to `0`. The retrieval snapshot for `internal/repl/repl.go` shows `/load` replacing `ag.History` without touching the tracker.

Existing protections:
The `projectTokens < 0` clamp prevents negative token totals, but there is no equivalent guard for the history slice. `SwitchProject` and `StartWithHistory` only set `historyStart` to `len(messages)` at session creation; neither `/load` nor any other visible path updates it when `ag.History` is replaced.

Proposed mitigation:
Split the bounds check so `start > len(messages)` caps `start` at `len(messages)`, producing an empty slice when local history has been truncated. Optionally reset `historyStart`/`tokenStart` when `/load` replaces history, but the local cap is the minimal fix.

Alternative mitigations considered:
Resetting the tracker on `/load` is broader and would lose the in-progress cloud session; capping the slice preserves the existing session while avoiding cross-project data leakage.

Severity calibration:
Score 4: the failure path requires a `/load` after cloud session start, but when it happens it silently uploads wrong messages to the wrong project. It is not a crash but a data-scoping/information-leak bug.

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