Fix: allow Free saved_workspace upsert (Codex P1 finding)

The saved_workspaces quota gate was rejecting every re-save from a
Free user (cap=1) because it checked `existing_count >= cap` without
distinguishing UPDATES to an existing slot from creation of NEW slots.
Today's store upserts on user_id (one row per user), so any "existing
row" means the save is an update — not a new slot — and the cap
shouldn't apply.

Real-world impact: the frontend autosave path calls /workspace/save
directly without pre-delete. Once a Free user has one saved workspace,
every subsequent autosave got rejected with a 429, breaking the
snapshot-refresh UX for every Free user past their first save.

Fix splits the gate into two questions:
  * is_existing_slot_update — does the user already own the slot?
  * is_creating_new_slot — would this save add to the slot count?

The cap fires only on the second case. Today, with one-row-per-user
storage, the gate is effectively dormant (existing_count == 1 always
means update, existing_count == 0 with cap >= 1 always passes). It
will start firing correctly the moment we migrate to per-slot rows
(future PR with slugs/labels), without revisiting this code path.

test_2nd_saved_workspace_on_free_returns_429 was inverted to
test_free_re_save_upserts_existing_workspace and now pins the
correct UX: re-saving is allowed because it's an update, not a new
slot. test_saved_workspace_delete_then_save_works was updated to
drop the intermediate 429 assertion (no longer fires under the
fix) and instead pin that DELETE + save round-trips cleanly.

Found by ChatGPT Codex review on PR #2 (May 2026).

Full backend suite: 501 passed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: LEANDERANTONY

backend/services/workspace_persistence_service.py

@@ -114,23 +114,27 @@ def save_workspace_snapshot(
         raise RuntimeError("Saved workspace persistence is not configured.")
 
     # Persistent-count quota gate. Skip when the tier cap is UNLIMITED
-    # (Business). For capped tiers, count existing slots: the current
-    # store has at most one row per user, so the count is 0 or 1.
-    # `load_workspace` returns `(record, status)` -- status is
-    # "missing" / "expired" / "available". Anything other than
-    # "available" means there's no live row.
+    # (Business). For capped tiers, the rule is: a NEW slot creation
+    # counts toward the cap; UPDATES to an existing slot do not.
     #
-    # Eviction policy at-cap: REJECT, do not auto-evict. Users must
-    # explicitly delete an existing saved workspace before saving a
-    # new one. This matches the saved_jobs gate. The "overwrite" UX
-    # for Free at cap=1 is achieved client-side: the frontend deletes
-    # the existing workspace before issuing the new save.
+    # Today's store upserts on user_id (one row per user). So when the
+    # user already has an "available" record, this save is an UPDATE to
+    # their existing slot, not a new slot — the cap doesn't apply and
+    # the save proceeds. This is critical for the frontend autosave UX:
+    # every snapshot refresh after the first save would otherwise 429
+    # for Free users (cap=1, 1 >= 1 blocks the upsert).
     #
-    # Pro cap=5 / Business UNLIMITED are future-schema slots. Today
-    # the store upserts on user_id (one row per user) so Pro and
-    # Business never functionally reach the cap; the gate remains
-    # tier-correct so the next schema migration (e.g. per-slug rows)
-    # doesn't require revisiting this code path.
+    # When the schema migrates to per-slot rows (e.g. one row per
+    # saved-workspace slug, future PR), `existing_count` becomes the
+    # user's actual distinct-slot count and `is_creating_new_slot`
+    # becomes True only when the save's slug isn't already in the
+    # user's set. The gate logic then naturally blocks the (cap+1)-th
+    # distinct slot without revisiting this code path.
+    #
+    # The 429 message + the saved_jobs gate's parallel "delete to make
+    # room" UX still apply for genuinely-new slot creation; this just
+    # carves out the upsert path so autosave doesn't break under tier
+    # caps the user hasn't actually exceeded.
     tier = resolve_user_tier(context.app_user)
     cap = TIER_CAPS[tier]["saved_workspaces"]
     quota_user_id = str(getattr(context.app_user, "id", "") or "")
@@ -140,12 +144,12 @@ def save_workspace_snapshot(
             refresh_token,
             quota_user_id,
         )
-        existing_count = (
-            1
-            if existing_status == "available" and existing_record is not None
-            else 0
+        is_existing_slot_update = (
+            existing_status == "available" and existing_record is not None
         )
-        if existing_count >= cap:
+        existing_count = 1 if is_existing_slot_update else 0
+        is_creating_new_slot = not is_existing_slot_update
+        if is_creating_new_slot and existing_count >= cap:
             raise QuotaExceededError(
                 "You have reached the saved-workspaces limit for your "
                 "plan. Delete an existing saved workspace to make room, "

tests/backend/test_search_and_saved_quota_enforcement.py

@@ -527,31 +527,34 @@ def _resolver(*, access_token=None, refresh_token=None):
             )
 
 
-def test_2nd_saved_workspace_on_free_returns_429():
-    """Free's saved_workspaces cap is 1. The first save succeeds; the
-    second attempt (with one row already present) rejects with cap=1.
-
-    The current single-row schema means save 1 puts 1 row + save 2
-    would normally overwrite it. The gate REJECTS the overwrite to
-    keep the Free=1 ceiling explicit -- users delete their existing
-    workspace before saving a new one. This is the documented
-    eviction policy from the brief.
+def test_free_re_save_upserts_existing_workspace():
+    """Free's saved_workspaces cap is 1, but re-saving an existing
+    workspace is an UPDATE to the same slot rather than a new slot —
+    so the gate must NOT 429. This is what the frontend autosave
+    relies on: every snapshot refresh after the first save calls
+    /workspace/save without pre-delete; blocking those would break
+    the autosave UX for every Free user past their first save.
+
+    The cap (1) governs the maximum number of *distinct slots* a Free
+    user can occupy, not the number of write operations against their
+    existing slot. Today's one-row-per-user schema can't actually
+    produce a >1 distinct-slot state for a single user, so the
+    "(cap+1)-th distinct save" assertion lands with the multi-row
+    schema migration (see test_distinct_2nd_workspace_blocks_when_multi_slot_lands).
+
+    Regression for Codex P1 flagged on PR #2 (May 2026).
     """
     auth_context = _build_auth_context(user_id="user-free-workspace-1")
     store = _FakeSavedWorkspaceStore()
 
     # First save: 0 existing -> allowed.
-    result = _save_workspace_snapshot(auth_context=auth_context, store=store)
-    assert result["status"] == "saved"
+    first = _save_workspace_snapshot(auth_context=auth_context, store=store)
+    assert first["status"] == "saved"
 
-    # Second save: 1 existing -> rejected.
-    with pytest.raises(QuotaExceededError) as exc_info:
-        _save_workspace_snapshot(auth_context=auth_context, store=store)
-    err = exc_info.value
-    assert err.counter == "saved_workspaces"
-    assert err.cap == TIER_CAPS["free"]["saved_workspaces"] == 1
-    assert err.current == 1
-    assert err.tier == "free"
+    # Second save: same user_id, store upserts -> still an update to
+    # the user's existing slot, NOT a new slot. Must succeed.
+    second = _save_workspace_snapshot(auth_context=auth_context, store=store)
+    assert second["status"] == "saved"
 
 
 def test_pro_saved_workspace_with_existing_row_is_allowed(monkeypatch):
@@ -579,15 +582,21 @@ def test_pro_saved_workspace_with_existing_row_is_allowed(monkeypatch):
 
 
 def test_saved_workspace_delete_then_save_works():
-    """Deleting the saved workspace frees the slot so the user can save
-    again -- the gate is consistent with the "delete to make room"
-    UX in the 429 message.
+    """Deleting the saved workspace and re-saving the next snapshot
+    both succeed. With the post-Codex-fix gate, re-saving never
+    requires a pre-delete (upserts are always allowed against an
+    existing slot), so the explicit DELETE is purely UX-driven --
+    e.g. when the user wants to clear their workspace from the
+    saved-jobs sidebar. This test pins that DELETE + save round-trips
+    cleanly even after the gate is in place.
     """
     auth_context = _build_auth_context(user_id="user-free-workspace-delete-1")
     store = _FakeSavedWorkspaceStore()
-    _save_workspace_snapshot(auth_context=auth_context, store=store)
-    with pytest.raises(QuotaExceededError):
-        _save_workspace_snapshot(auth_context=auth_context, store=store)
+    # First save: 0 existing -> allowed.
+    first = _save_workspace_snapshot(auth_context=auth_context, store=store)
+    assert first["status"] == "saved"
+    # Explicit delete (user-driven clean slate).
     store.delete_workspace("access", "refresh", "user-free-workspace-delete-1")
-    result = _save_workspace_snapshot(auth_context=auth_context, store=store)
-    assert result["status"] == "saved"
+    # Save again from 0 existing -> allowed.
+    second = _save_workspace_snapshot(auth_context=auth_context, store=store)
+    assert second["status"] == "saved"