Fix: Atomic job deletion and database cleanup (#302)

backend/app/db.py

@@ -12,15 +12,18 @@
 
 async def init_db():
     async with aiosqlite.connect(DB_PATH) as db:
-        await db.execute("""
+        await db.execute(
+            """
             CREATE TABLE IF NOT EXISTS org_jobs (
                 id TEXT PRIMARY KEY,
                 org_name TEXT,
                 status TEXT,
                 created_at TEXT DEFAULT (datetime('now'))
             )
-        """)
-        await db.execute("""
+        """
+        )
+        await db.execute(
+            """
             CREATE TABLE IF NOT EXISTS findings (
                 id              TEXT PRIMARY KEY,
                 job_id          TEXT NOT NULL,
@@ -43,35 +46,43 @@ async def init_db():
                 version         INTEGER DEFAULT 1,
                 created_at      TEXT DEFAULT (datetime('now'))
             )
-        """)
-        await db.execute("""
+        """
+        )
+        await db.execute(
+            """
             CREATE TABLE IF NOT EXISTS jobs (
                 job_id       TEXT PRIMARY KEY,
                 project_name TEXT,
                 scan_method  TEXT,
                 created_at   TEXT DEFAULT (datetime('now'))
             )
-        """)
-        await db.execute("""
+        """
+        )
+        await db.execute(
+            """
             CREATE TABLE IF NOT EXISTS verify_outcomes (
                 id TEXT PRIMARY KEY,
                 job_id TEXT NOT NULL,
                 passed INTEGER NOT NULL,
                 new_issues_introduced INTEGER DEFAULT 0,
                 verified_at TEXT DEFAULT (datetime('now'))
             )
-        """)
+        """
+        )
 
-        await db.execute("""
+        await db.execute(
+            """
             CREATE TABLE IF NOT EXISTS contributor_stats (
                 github_username TEXT PRIMARY KEY,
                 findings_closed INTEGER DEFAULT 0,
                 fixes_passed INTEGER DEFAULT 0,
                 prs_merged INTEGER DEFAULT 0,
                 last_updated TEXT DEFAULT (datetime('now'))
             )
-        """)
-        await db.execute("""
+        """
+        )
+        await db.execute(
+            """
             CREATE TABLE IF NOT EXISTS dependency_links (
                 id TEXT PRIMARY KEY,
                 org_job_id TEXT NOT NULL,
@@ -80,8 +91,10 @@ async def init_db():
                 package_version TEXT,
                 created_at TEXT DEFAULT (datetime('now'))
             )
-        """)
-        await db.execute("""
+        """
+        )
+        await db.execute(
+            """
             CREATE TABLE IF NOT EXISTS fixes (
                 id              TEXT PRIMARY KEY,
                 job_id          TEXT NOT NULL,
@@ -91,7 +104,8 @@ async def init_db():
                 fix_type        TEXT,   -- 'insert' | 'delete' | 'mixed' | 'none'
                 created_at      TEXT DEFAULT (datetime('now'))
             )
-        """)
+        """
+        )
 
         db.row_factory = aiosqlite.Row
         cursor = await db.execute("PRAGMA table_info(findings)")
@@ -223,10 +237,14 @@ async def update_job_status(
 
 
 async def delete_job(db: aiosqlite.Connection, job_id: str):
-    await db.execute("DELETE FROM jobs WHERE job_id = ?", (job_id,))
-    await db.execute("DELETE FROM findings WHERE job_id = ?", (job_id,))
-    await db.execute("DELETE FROM verify_outcomes WHERE job_id = ?", (job_id,))
-    await db.commit()
+    try:
+        await db.execute("DELETE FROM findings WHERE job_id = ?", (job_id,))
+        await db.execute("DELETE FROM verify_outcomes WHERE job_id = ?", (job_id,))
+        await db.execute("DELETE FROM jobs WHERE job_id = ?", (job_id,))
+        await db.commit()
+    except Exception:
+        await db.rollback()
+        raise
 
 
 async def create_findings(
@@ -435,7 +453,8 @@ async def get_leaderboard_stats():
     """Fetches all contributors sorted by their weighted score."""
     async with aiosqlite.connect(DB_PATH) as db:
         db.row_factory = aiosqlite.Row
-        cursor = await db.execute("""
+        cursor = await db.execute(
+            """
             SELECT 
                 github_username,
                 findings_closed,
@@ -445,7 +464,8 @@ async def get_leaderboard_stats():
                 (fixes_passed * 3) + (prs_merged * 2) + (findings_closed * 1) as total_score
             FROM contributor_stats
             ORDER BY total_score DESC
-        """)
+        """
+        )
         rows = await cursor.fetchall()
         return [dict(row) for row in rows]
 

backend/app/main.py

@@ -1303,17 +1303,28 @@ async def delete_job_endpoint(job_id: str):
     except ValueError as e:
         raise HTTPException(status_code=400, detail=str(e))
 
-    if not job_dir.exists():
-        raise HTTPException(status_code=404, detail="Job not found")
-
-    safe_rmtree(job_dir)
-
     db = await get_db()
     try:
+        job = await get_job(db, job_id)
+        if not job:
+            raise HTTPException(
+                status_code=404, detail=f"No job found with id '{job_id}'"
+            )
         await delete_job(db, job_id)
+    except HTTPException:
+        raise
+    except Exception:
+        logger.exception(f"Failed to delete job {job_id} from db")
+        raise HTTPException(status_code=500, detail="Database error during deletion")
     finally:
         await db.close()
 
+    if job_dir.exists():
+        try:
+            safe_rmtree(job_dir)
+        except Exception:
+            logger.warning(f"Failed to delete workspace directory for job {job_id}")
+
     return {"deleted": True}
 
 

backend/tests/test_job_endpoints.py

@@ -219,3 +219,114 @@ def test_stream_job_completed(self):
 
         if JOB_ID in ACTIVE_SCANS:
             del ACTIVE_SCANS[JOB_ID]
+
+
+class TestDeleteJob:
+    @patch("app.main.get_job", new_callable=AsyncMock)
+    @patch("app.main.safe_job_dir")
+    @patch("app.main.safe_rmtree")
+    def test_delete_success(self, mock_rmtree, mock_safe_job_dir, mock_get_job):
+        mock_get_job.return_value = {"job_id": JOB_ID}
+        from unittest.mock import MagicMock
+
+        mock_job_dir = MagicMock()
+        mock_job_dir.exists.return_value = True
+        mock_safe_job_dir.return_value = mock_job_dir
+
+        db = db_mock(True)
+        # Mock successful execution
+        db.execute = AsyncMock()
+        db.commit = AsyncMock()
+
+        with patch("app.main.get_db", AsyncMock(return_value=db)):
+            res = client.delete(f"/jobs/{JOB_ID}")
+
+        assert res.status_code == 200
+        assert res.json() == {"deleted": True}
+
+        # Verify db logic called
+        assert db.execute.call_count == 3  # DELETE x3
+        assert db.commit.called
+        assert not db.rollback.called
+
+        # Verify rmtree called because job_dir exists and db commit didn't fail
+        mock_rmtree.assert_called_once_with(mock_job_dir)
+
+    @patch("app.main.get_job", new_callable=AsyncMock)
+    @patch("app.main.safe_job_dir")
+    @patch("app.main.safe_rmtree")
+    def test_delete_missing_directory(
+        self, mock_rmtree, mock_safe_job_dir, mock_get_job
+    ):
+        mock_get_job.return_value = {"job_id": JOB_ID}
+        from unittest.mock import MagicMock
+
+        mock_job_dir = MagicMock()
+        mock_job_dir.exists.return_value = False
+        mock_safe_job_dir.return_value = mock_job_dir
+
+        db = db_mock(True)
+        db.execute = AsyncMock()
+        db.commit = AsyncMock()
+
+        with patch("app.main.get_db", AsyncMock(return_value=db)):
+            res = client.delete(f"/jobs/{JOB_ID}")
+
+        assert res.status_code == 200
+        assert res.json() == {"deleted": True}
+
+        # Verify db was cleaned
+        assert db.commit.called
+
+        # Verify rmtree NOT called
+        mock_rmtree.assert_not_called()
+
+    @patch("app.main.get_job", new_callable=AsyncMock)
+    @patch("app.main.safe_job_dir")
+    @patch("app.main.safe_rmtree")
+    def test_delete_db_failure(self, mock_rmtree, mock_safe_job_dir, mock_get_job):
+        mock_get_job.return_value = {"job_id": JOB_ID}
+        from unittest.mock import MagicMock
+
+        mock_job_dir = MagicMock()
+        mock_job_dir.exists.return_value = True
+        mock_safe_job_dir.return_value = mock_job_dir
+
+        db = db_mock(True)
+        db.execute = AsyncMock(side_effect=Exception("DB Error"))
+        db.rollback = AsyncMock()
+
+        with patch("app.main.get_db", AsyncMock(return_value=db)):
+            res = client.delete(f"/jobs/{JOB_ID}")
+
+        assert res.status_code == 500
+        assert "Database error" in res.json()["detail"]
+
+        # Verify rollback called
+        assert db.rollback.called
+
+        # Verify rmtree NOT called since db failed
+        mock_rmtree.assert_not_called()
+
+    @patch("app.main.get_job", new_callable=AsyncMock)
+    @patch("app.main.safe_job_dir")
+    @patch("app.main.safe_rmtree")
+    def test_delete_missing_job_returns_404(
+        self, mock_rmtree, mock_safe_job_dir, mock_get_job
+    ):
+        mock_get_job.return_value = None  # Job not found in DB
+
+        db = db_mock(True)
+        db.execute = AsyncMock()
+
+        with patch("app.main.get_db", AsyncMock(return_value=db)):
+            res = client.delete(f"/jobs/{JOB_ID}")
+
+        assert res.status_code == 404
+        assert "No job found" in res.json()["detail"]
+
+        # Verify delete_job was not called (execute not called for DELETE)
+        assert db.execute.call_count == 0
+
+        # Verify rmtree NOT called
+        mock_rmtree.assert_not_called()