Skip to content

Evict banned users from free_session slots each admission tick#526

Merged
jahooma merged 1 commit intomainfrom
jahooma/evict-banned-from-slots
Apr 21, 2026
Merged

Evict banned users from free_session slots each admission tick#526
jahooma merged 1 commit intomainfrom
jahooma/evict-banned-from-slots

Conversation

@jahooma
Copy link
Copy Markdown
Contributor

@jahooma jahooma commented Apr 21, 2026

Summary

~55 banned users were sitting on admitted free_session slots because banUser() (Stripe webhook, admin UI, direct SQL) doesn't cascade into free_session — only the one-shot scripts/ban-freebuff-bots.ts happened to delete those rows. Adds evictBanned() in store.ts and calls it from runAdmissionTick in parallel with sweepExpired, so every tick drops any free_session row whose user has banned = true. The tick result and per-tick log line now surface evictedBanned for observability.

Test plan

  • bun run test src/server/free-session/__tests__/ — 52/52 pass (includes two new tests: per-tick eviction + eviction continues when health pauses admission)
  • tsc --noEmit -p web clean
  • After deploy: watch first tick's evictedBanned count drain the ~55 stale rows

🤖 Generated with Claude Code

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 21, 2026

Greptile Summary

This PR fixes a gap where banned users were retaining admitted free_session slots indefinitely because banUser() (via Stripe webhook, admin UI, or direct SQL) does not cascade a delete into free_session. The fix adds evictBanned() in store.ts and runs it in parallel with sweepExpired on every admission tick, ensuring banned users are cleared within one tick period (~seconds) instead of waiting until their session naturally expires.

Key changes:

  • store.ts: New evictBanned() uses a DELETE + EXISTS subquery against the users table (no status filter, so it correctly removes both queued and active rows for banned users)
  • admission.ts: evictBanned added to AdmissionDeps interface, defaultDeps, called in Promise.all alongside sweepExpired, and evictedBanned surfaced in the tick result and structured log line
  • admission.test.ts: Two new tests verify that eviction fires every tick and continues even when admission is paused by an unhealthy fleet
  • The Promise.all ordering ensures both sweep and eviction complete before the admission loop runs, so a freed slot from a banned user can be filled in the same tick

Confidence Score: 5/5

Safe to merge — the change is additive, well-tested, and fixes a real production data hygiene issue with no risk of data loss for legitimate users

The implementation is correct: DELETE + EXISTS is idiomatic and safe, the no-status-filter behavior is intentional and correct (banned queued users should also be removed), both operations complete before admission so freed slots are visible to the same tick, and 2 targeted tests cover the new code paths including the edge case of health-paused admission. No interfaces are broken, no migrations required, and the change degrades gracefully (worst case: zero rows deleted is a no-op).

No files require special attention

Important Files Changed

Filename Overview
web/src/server/free-session/store.ts Adds evictBanned() – a DELETE with an EXISTS subquery against the users table to drop free_session rows for banned users; no status filter (correctly removes both queued and active rows)
web/src/server/free-session/admission.ts Wires evictBanned into AdmissionDeps and calls it in parallel with sweepExpired each tick; surfaces evictedBanned in the tick result and structured log line
web/src/server/free-session/tests/admission.test.ts Adds evictBanned stub to makeAdmissionDeps and two new tests: per-tick eviction count surfaced, and eviction still runs when health pauses admission

Sequence Diagram

sequenceDiagram
    participant Ticker as runTick (setInterval)
    participant Tick as runAdmissionTick
    participant Sweep as sweepExpired
    participant Evict as evictBanned
    participant Health as getFleetHealth
    participant Admit as admitFromQueue (per model)
    participant DB as PostgreSQL

    Ticker->>Tick: call every ADMISSION_TICK_MS
    par parallel cleanup
        Tick->>Sweep: sweepExpired(now, graceMs)
        Sweep->>DB: DELETE active rows past expires_at+grace
        DB-->>Sweep: deleted count
        Sweep-->>Tick: expired
    and
        Tick->>Evict: evictBanned()
        Evict->>DB: DELETE free_session WHERE EXISTS (banned user)
        DB-->>Evict: deleted count
        Evict-->>Tick: evictedBanned
    end
    Tick->>Health: getFleetHealth()
    Health-->>Tick: FleetHealth map
    loop per model
        Tick->>Admit: admitFromQueue(model, health)
        Admit->>DB: pg_try_advisory_xact_lock + SELECT queued + UPDATE to active
        DB-->>Admit: admitted row or empty
        Admit-->>Tick: { admitted, skipped }
    end
    Tick-->>Ticker: AdmissionTickResult { expired, evictedBanned, admitted, ... }
    Ticker->>Ticker: logger.info(metric: freebuff_waiting_room)
Loading

Reviews (1): Last reviewed commit: "Evict banned users from free_session slo..." | Re-trigger Greptile

@jahooma jahooma merged commit 1c92cf9 into main Apr 21, 2026
34 checks passed
@jahooma jahooma deleted the jahooma/evict-banned-from-slots branch April 21, 2026 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant