fix: clear cron session in DB when not cached (cold-cache after restart)

[vanducng]

Fixes #365

Problem

Cron-triggered agents get stuck in tool call loops after server restarts. The LLM sees contradictory history from previous runs and enters a loop trying to reconcile.

Root cause: PGSessionStore.Reset() only cleared the in-memory cache. After a restart the cache is empty, so Reset was a silent no-op. The subsequent GetOrCreate loaded the full accumulated history from DB — defeating the reset added in #294.

Flow (before fix)

Server restart → cache wiped
Cron fires → Reset(key) → cache empty → NO-OP
           → Save(key)  → cache empty → NO-OP
           → GetOrCreate(key) → loads bloated history from DB
           → LLM confused by stale context → tool loop → failure

Fix

When the session isn't in cache, Reset now issues a direct DB UPDATE to clear messages and summary:

UPDATE sessions SET messages = '[]', summary = ''
WHERE session_key = $1 AND tenant_id = $2

Warm-cache path is unchanged (early return before DB code).

Impact

All 3 callers of Reset() benefit:

• Cron handler (gateway_cron.go) — target fix
• Consumer handler (gateway_consumer_handlers.go) — same improvement
• WS sessions method (sessions.go) — harmless, UI sessions are always warm

Test plan

• go build ./... passes
• go vet ./... passes
• go test -race ./internal/store/pg/ — all pass
• 4 unit tests prove the bug and verify the fix:
  • TestReset_ColdCache_FallsBackToDB — proves DB fallback is reached
  • TestReset_WarmCache_ClearsHistory — warm path unchanged
  • TestSave_ColdCache_IsNoOp — acceptable after fix
  • TestResetAfterGetOrCreate_FixVerification — alternative fix path works

[mrgoonie]

@claude review

[claude]

Claude finished @mrgoonie's task in 1m 36s —— View job

---

PR Review: fix/cron-session-reset-cold-cache

Tasks:

• Gather context
• Read changed files
• Analyze the fix
• Post review feedback

---

Summary

The fix is correct and addresses the root cause cleanly. Two issues worth addressing before merge, one of them a real concurrency bug.

---

Issues

🔴 Lock held during DB call (sessions_ops.go:33–49)

Reset() acquires s.mu.Lock() and holds it across the db.ExecContext() call in the cold-cache path. A DB call under a mutex blocks all concurrent cache reads and writes for the full round-trip duration.

Delete() (same file, lines 52–65) correctly releases the lock before touching the DB. Reset() should follow the same pattern: check + clear cache under the lock, then release before the DB fallback.

func (s *PGSessionStore) Reset(ctx context.Context, key string) {
    s.mu.Lock()
    if data, ok := s.cache[sessionCacheKey(ctx, key)]; ok {
        data.Messages = []providers.Message{}
        data.Summary = ""
        data.Updated = time.Now()
        s.mu.Unlock()
        return
    }
    s.mu.Unlock()

    // Now safe to do the DB call without holding the lock
    tid := tenantIDForInsert(ctx)
    s.db.ExecContext(ctx, ...)
}

Fix this →

🟡 DB error silently discarded (sessions_ops.go:45–49)

Reset() returns nothing (interface-mandated void), but the ExecContext result is not even captured:

s.db.ExecContext(ctx, ...)  // error ignored

Delete() returns and propagates err. Since Reset can't return an error without changing the interface, a warning log would at least make failures observable:

if _, err := s.db.ExecContext(ctx, ...); err != nil {
    slog.Warn("sessions.reset.db_fallback", "key", key, "error", err)
}

Fix this →

---

Observations (no action required)

Test approach (sessions_reset_test.go:15–40): TestReset_ColdCache_FallsBackToDB verifies the DB path is reached by catching the nil-db panic. It's clever and does prove the fix path is exercised, but it reads oddly — a reader has to understand that "panic = fix verified" is intentional. A brief clarifying comment in the test (or a t.Skip("integration test needed for full DB verify") note) would help future readers. Not a blocker.

TruncateHistory / SetHistory cold-cache gaps: These two methods in sessions_ops.go have the same silent no-op behavior on cold cache, but their callers (loop_history.go) are only invoked during an active agent run where the session is already warm. So they are not affected by this bug — just worth noting for completeness.

---