From 7960809b3090d78e6cde9bdfde9f1a55e32b869a Mon Sep 17 00:00:00 2001 From: Codex Date: Wed, 15 Jul 2026 13:06:53 +0200 Subject: [PATCH] Self-heal stale local_path pointers on read Backups completed before the keep-local-backups feature never had local_path nulled out after the local archive was deleted, so the new "Local copy: Kept" indicator was trusting a stale DB column instead of reality. GET /api/backups and GET /api/backups/{id} now verify the file still exists and clear local_path if it doesn't, which also covers any future case of a local file disappearing outside the app. Co-Authored-By: Claude Sonnet 5 --- CLAUDE.md | 1 + backend/app/backup_service.py | 13 +++++++++-- backend/tests/test_backup_service.py | 34 ++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 2f06376..8298914 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -125,6 +125,7 @@ Important behavior: - Retention runs after backup completion and hourly from the worker. - A completed backup whose local archive is missing (never kept, or deleted via "Delete local copy") can be re-fetched from the remote into `local_backup_dir` via `POST /api/backups/{id}/download`; this is a transient `local_downloading` state that resumes on worker restart. Since `local_backup_dir` is the actual Proxmox storage dump directory, a downloaded archive shows up in the Proxmox UI for the operator to restore from directly — this app never runs the restore itself. - A locally-retained (or downloaded) copy can be removed independently of the remote object via `DELETE /api/backups/{id}/local`, without affecting `state` or `remote_path`. +- `local_path` is not trusted blindly: `GET /api/backups` and `GET /api/backups/{id}` verify the file still exists on disk and clear `local_path` in SQLite if it doesn't, so a stale pointer (e.g. left over from a version that didn't null it, or a file removed outside the app) never shows as "kept" in the UI. ## Applying updates after pulling changes diff --git a/backend/app/backup_service.py b/backend/app/backup_service.py index 0d901cd..6456d7b 100644 --- a/backend/app/backup_service.py +++ b/backend/app/backup_service.py @@ -130,11 +130,20 @@ def list_backups(guest_vmid: int | None = None, state: str | None = None) -> lis query += " WHERE " + " AND ".join(where) query += " ORDER BY started_at DESC LIMIT 500" with db() as conn: - return [dict(row) for row in conn.execute(query, args).fetchall()] + rows = [dict(row) for row in conn.execute(query, args).fetchall()] + return [_reconcile_local_path(row) for row in rows] + + +def _reconcile_local_path(backup: dict) -> dict: + """Self-heal a stale local_path left over from a file that no longer exists.""" + local_path = backup.get("local_path") + if local_path and not Path(local_path).exists(): + return update_backup(backup["id"], local_path=None) + return backup def get_backup(backup_id: str) -> dict: - return _backup_row(backup_id) + return _reconcile_local_path(_backup_row(backup_id)) def purge_deleted_backup(backup_id: str) -> dict: diff --git a/backend/tests/test_backup_service.py b/backend/tests/test_backup_service.py index 92dd917..5c78cfb 100644 --- a/backend/tests/test_backup_service.py +++ b/backend/tests/test_backup_service.py @@ -573,6 +573,40 @@ def test_retention_delete_also_removes_local_copy( assert final["local_path"] is None +def test_get_and_list_backups_clear_stale_local_path_for_missing_file(isolated_db, no_notifications, tmp_path): + from app import backup_service + + missing_local = tmp_path / "gone.tar.zst" + + with db() as conn: + conn.execute( + """ + INSERT INTO backups( + id, guest_vmid, guest_name, guest_type, node, state, + local_path, remote_path, started_at, completed_at, updated_at + ) + VALUES ('stale-local', 110, 'test-guest', 'vm', 'pve', 'completed', + ?, 'onedrive:pve-cloud-backup/stale-local/archive.tar.zst', ?, ?, ?) + """, + (str(missing_local), utc_now(), utc_now(), utc_now()), + ) + + fetched = backup_service.get_backup("stale-local") + assert fetched["local_path"] is None + + with db() as conn: + row = conn.execute("SELECT local_path FROM backups WHERE id = 'stale-local'").fetchone() + assert row["local_path"] is None + + with db() as conn: + conn.execute( + "UPDATE backups SET local_path = ? WHERE id = 'stale-local'", + (str(missing_local),), + ) + listed = backup_service.list_backups() + assert listed[0]["local_path"] is None + + def test_recover_unfinished_resumes_local_downloading(isolated_db, no_notifications, monkeypatch): from app import backup_service