Session Save() uses UPDATE-only — cron job sessions never persist, retry reads stale cache

[duhd-vnpay]

Bug

PGSessionStore.Save() uses UPDATE sessions ... WHERE session_key = $19. If the session row does not exist in the DB yet (first run of a cron job), the UPDATE affects 0 rows and returns silently. However, AddMessage() has already added messages to the in-memory cache.

This causes two problems:

1. Cron sessions never persist — first-run cron sessions exist only in memory, lost on restart
2. Stale cache on retry — when ExecuteWithRetry retries a failed cron job, the next attempt reads the stale in-memory cache from the failed attempt, polluting the conversation history with leftover messages (e.g., empty user message + "..." assistant fallback)

Root Cause

// sessions_list.go
func (s *PGSessionStore) Save(key string) error {
    // ...
    _, err := s.db.Exec(
        `UPDATE sessions SET messages = $1, ... WHERE session_key = $19`,
        // ...
    )
    return err  // UPDATE 0 rows → no error, but session not persisted
}

The session row is created via a separate createSession() path (e.g., GetOrCreateSession), but the cron agent loop calls AddMessage() + Save() without ensuring the row exists first.

Impact

• Cron job retries (via ExecuteWithRetry) accumulate stale messages in cache across attempts
• The stale messages cause LLM 400 errors when the proxy converts empty content blocks to Anthropic format (messages.0.content.0.text.text: Field required)
• Jobs that fail once tend to keep failing on all retry attempts due to cache pollution

Suggested Fix

Option A: Save() should use INSERT ... ON CONFLICT (session_key) DO UPDATE (UPSERT)

Option B: Clear in-memory cache for the session key when the cron handler starts a new attempt (reset between retries)

Option C: Have the cron handler call GetOrCreateSession() before running the agent loop to ensure the DB row exists

Steps to Reproduce

1. Create a cron job with a valid schedule
2. Ensure the cron agent makes tool calls but fails at iteration 2 (e.g., LLM error)
3. ExecuteWithRetry retries the job
4. The retry reads stale history from cache (empty user + "..." assistant from the failed attempt)
5. This stale history causes the LLM request to fail again
6. All 4 retry attempts fail with the same error

Environment

• GoClaw v1.74.1
• PostgreSQL-backed session store (PGSessionStore)
• Cron retry config: 3 retries with backoff

[duhd-vnpay]

Fix Implemented (local branch SDLC-Goclaw)

Three fixes addressing root cause and defensive hardening:

1. Save() → UPSERT (store/pg/sessions_list.go)

Changed UPDATE sessions ... WHERE session_key = $19 to INSERT ... ON CONFLICT (tenant_id, session_key) DO UPDATE SET ....

First-run cron sessions now persist correctly — no more silent 0-row UPDATEs.

2. Session cache reset before each cron attempt (cmd/gateway_cron.go)

Added sessionStore.Reset(cronCtx, sessionKey) at the start of every handler invocation. This ensures ExecuteWithRetry attempts start with a clean session, preventing stale messages (empty user + "..." fallback) from polluting retries.

// Reset session cache so retries start fresh from DB state
sessionStore.Reset(cronCtx, sessionKey)

3. JSON surrogate sanitization (providers/sanitize_json.go) — defensive

Added sanitizeJSONSurrogates() that strips \uD800–\uDFFF escape sequences from marshaled JSON before sending to any LLM API (Anthropic, OpenAI, Codex). Handles surrogates that leak through json.RawMessage fields.

This prevents the invalid high surrogate in string API rejection observed in production.

Test Results

• 9 new unit tests for sanitizeJSONSurrogates — all pass
• All related packages pass: providers, store, store/pg, cron, cmd
• go build ./... clean

[mrgoonie]

Good catch. The UPDATE-only Save() silently succeeding with 0 rows affected is definitely a bug - it should either UPSERT or return an error when no row exists.

Option A (UPSERT) is the cleanest fix since it handles the "session exists" and "session doesn't exist" cases in a single atomic operation, no conditional logic needed.

The stale cache pollution on retry is a nasty secondary effect - even with the UPSERT fix, we should consider clearing the in-memory cache between retry attempts to prevent failed messages from leaking into the next attempt.

Would you like to submit a PR? If so, Option A + cache clear on retry would be the complete fix.