From 728fccfaecb07274862526ef450ac37d6a7e1bdd Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 7 Sep 2026 11:28:33 +0000 Subject: [PATCH] Don't no-op DELETE for cache-evicted Postgres scans get() and list() already serve SaaS job history from Postgres after RAM eviction, but delete() returned False without touching the DB whenever the job was missing from the in-memory cache. DELETE /jobs/{id} then returned 204 while the scan row stayed in scan_jobs and reappeared on the next list. Co-authored-by: dmitryflynn --- api/jobs/manager.py | 21 ++++++--- test_pg_jobs.py | 103 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 116 insertions(+), 8 deletions(-) diff --git a/api/jobs/manager.py b/api/jobs/manager.py index 8f29e7f..d1fbf8a 100644 --- a/api/jobs/manager.py +++ b/api/jobs/manager.py @@ -566,16 +566,25 @@ def delete(self, job_id: str, _evict_only: bool = False) -> bool: record — used by capacity eviction on the Postgres backend so trimming RAM never destroys job history (the JSON backend has no such distinction and always removes the file). + + User deletes (_evict_only=False) must hit Postgres even when the job is + no longer in `_jobs`. get()/list() serve cache-evicted history from the + DB without rehydrating the cache, so a cache miss is the common case + for SaaS history — not a reason to no-op the durable delete. """ with self._lock: - if job_id not in self._jobs: - return False - del self._jobs[job_id] + present = job_id in self._jobs + if present: + del self._jobs[job_id] if self._pg and self._pg_store is not None: - if not _evict_only: - self._pg_store.delete_sync(job_id) # explicit user delete only - return True + if _evict_only: + return present + deleted = self._pg_store.delete_sync(job_id) + return present or deleted + + if not present: + return False # JSON backend: remove the on-disk record (never follow a malicious job_id). path = self.store._safe_path(job_id) if self.store else None diff --git a/test_pg_jobs.py b/test_pg_jobs.py index 6e46456..f7993a7 100644 --- a/test_pg_jobs.py +++ b/test_pg_jobs.py @@ -7,12 +7,16 @@ • the store + manager are import-safe and fail soft when psycopg is absent (so a misconfigured/unreachable DB degrades, never crashes a scan), • the 0003 migration SQL parses into executable statements. + • user DELETE reaches Postgres even after the job has been cache-evicted + (get/list serve history from the DB without rehydrating `_jobs`). """ import threading +import time from pathlib import Path +from unittest.mock import MagicMock -import pytest - +from api.jobs.manager import JobManager, ScanJob +from api.models.scan_request import ScanRequest from api.storage.pg_store import PgScanStore @@ -58,3 +62,98 @@ def test_scan_jobs_migration_parses(): stmts = _split_sql(sql) assert any("CREATE TABLE scan_jobs" in s for s in stmts) assert any("scan_jobs_org_created_idx" in s for s in stmts) + + +# ── User delete vs cache-evicted Postgres history ────────────────────────────── + + +class _FakePgStore: + """In-memory stand-in for PgScanStore so delete/get paths can be asserted + without a live database.""" + + def __init__(self): + self.records: dict[str, dict] = {} + self.delete_calls: list[str] = [] + + def get_sync(self, job_id: str): + rec = self.records.get(job_id) + return dict(rec) if rec is not None else None + + def list_sync(self, limit: int = 50, org_id: str = ""): + rows = list(self.records.values()) + if org_id: + rows = [r for r in rows if r.get("org_id") == org_id] + rows.sort(key=lambda r: r.get("created_at") or 0, reverse=True) + return [dict(r) for r in rows[:limit]] + + def load_recent_sync(self, limit: int = 500): + return self.list_sync(limit=limit) + + def delete_sync(self, job_id: str) -> bool: + self.delete_calls.append(job_id) + return self.records.pop(job_id, None) is not None + + def save_sync(self, record: dict) -> None: + jid = record.get("job_id") + if jid: + self.records[jid] = dict(record) + + +def _pg_manager(store: _FakePgStore) -> JobManager: + m = JobManager.__new__(JobManager) + m._jobs = {} + m._lock = threading.RLock() + m._pg = True + m._pg_store = store + m.store = MagicMock() + return m + + +def _completed_job(job_id: str = "job-evicted", org_id: str = "org-a") -> ScanJob: + job = ScanJob( + job_id=job_id, + config=ScanRequest(target="scanme.example.com"), + org_id=org_id, + status="completed", + created_at=time.time() - 13 * 3600, + completed_at=time.time() - 100, + ) + return job + + +def test_delete_cache_evicted_job_removes_postgres_row(): + """SaaS history lives in Postgres after RAM eviction. DELETE must still + remove the durable row — otherwise the API returns 204 and the scan + reappears on the next list/get.""" + store = _FakePgStore() + mgr = _pg_manager(store) + job = _completed_job() + store.save_sync(job.to_dict()) + # Simulate TTL/capacity eviction: gone from RAM, row still in PG. + assert job.job_id not in mgr._jobs + + assert mgr.get(job.job_id, org_id=job.org_id) is not None + assert mgr.delete(job.job_id) is True + assert store.delete_calls == [job.job_id] + assert store.get_sync(job.job_id) is None + assert mgr.get(job.job_id, org_id=job.org_id) is None + + +def test_delete_missing_postgres_job_returns_false(): + store = _FakePgStore() + mgr = _pg_manager(store) + assert mgr.delete("no-such-job") is False + assert store.delete_calls == ["no-such-job"] + + +def test_evict_only_does_not_delete_postgres_row(): + store = _FakePgStore() + mgr = _pg_manager(store) + job = _completed_job("job-cached") + store.save_sync(job.to_dict()) + mgr._jobs[job.job_id] = job + + assert mgr.delete(job.job_id, _evict_only=True) is True + assert job.job_id not in mgr._jobs + assert store.delete_calls == [] + assert store.get_sync(job.job_id) is not None