fix: treat missing tarball objects as permanent and defer superseded deletes until commit

[hieptl]

Summary

Automation "run now" failing with an opaque Internal error that repeated on every schedule tick because the automation's tarball object was missing from MinIO while its TarballUpload row survived. This lands fix (2) from the issue in its narrowed form and fix (1) at both delete-before-commit sites; fix (3) (MinIO durability) is infra-side and not part of this repo.

Supersedes the draft #336, which caught bare FileNotFoundError and is marked "do not merge as-is" for the over-trigger described below.

The bug

Two independent defects, both verified against main:

1. A missing tarball object was treated as transient, opaquely. _download_internal_tarball already raises TarballNotFoundError when the upload row is missing, but the subsequent store.read(...) was unguarded. A missing object surfaced as plain FileNotFoundError, sailed past the except PermanentDispatchError handler in dispatch step 4, and landed in _execute_run_safe: the run was marked FAILED with the literal string "Internal error" and the automation stayed enabled, re-failing forever (12 and 16 consecutive failures for the two affected C24 automations).
   Catching bare FileNotFoundError is not a safe fix: s3.py::_handle_client_error maps every ClientError to FileNotFoundError — NoSuchBucket, AccessDenied, and a catch-all covering 5xx / SlowDown / expired credentials. On C24's OOM-crash-looping MinIO, a read landing during a restart would permanently disable a healthy automation.
2. Superseded tarball objects were deleted before the transaction committed. Both regenerate_preset_prompt_tarball (prompt edits) and the git-sync import's _delete_superseded_upload destroyed the old storage object and then soft-deleted its row inside a transaction that commits later. Any post-delete rollback reverted the soft-delete and the tarball_path update but not the storage delete, leaving a live COMPLETED upload row and an automation pointing at an object that no longer exists — exactly the failure state in (1). A related latent bug: except FileNotFoundError: return None around the regeneration read meant a transient storage error during a prompt edit returned HTTP 200 with the prompt column updated while the tarball silently kept the old baked prompt.

The fix

Storage layer — new ObjectNotFoundError(FileNotFoundError), raised only at confirmed-absence sites: the S3 404/NoSuchKey branch, the local store's missing file, and both GCS NotFound handlers. All other storage errors keep raising plain FileNotFoundError, and the seven existing except FileNotFoundError handlers keep working unchanged via the parent class.

Dispatcher — the tarball read catches ObjectNotFoundError and re-raises it as TarballNotFoundError (already a PermanentDispatchError): the run fails with an error_detail naming the missing storage path and the remedy, and the automation is disabled instead of retrying a permanently unreadable object. Transient storage errors keep today's behavior (fail, stay enabled, retry).

Prompt edits — the superseded row is soft-deleted in-transaction; the object delete moves to a FastAPI background task. update_automation declares Depends(get_session, scope="function"): with the default request scope, background tasks run before the dependency-teardown commit (verified against FastAPI 0.136 / Starlette sources), which would reintroduce the bug. Function scope makes the order handler → commit → response → delete, and a commit failure means the delete never runs. The regeneration guard is narrowed to ObjectNotFoundError, so a transient storage error now fails the edit (500, rollback) instead of silently keeping the old prompt.

Git sync — _delete_superseded_upload becomes soft-delete-and-queue; a per-directory savepoint rollback discards its queued entries; the queue is drained best-effort only after the cycle's import commit succeeds.

In both deferred paths the worst case (a crash between commit and delete) leaks an orphaned object whose row is already soft-deleted — recoverable and identifiable, unlike the previous worst case of destroying an object a live row still points at.

Tests

Nine new tests plus five tightened assertions, all extending existing files. Verified to fail with the source changes selectively reverted (the one exception, test_transient_storage_error_is_not_reclassified, guards against the #336-style over-broad fix rather than against main):

• test_storage_s3.py — NoSuchKey/404 now assert ObjectNotFoundError; new test pins that a transient ServiceUnavailable error is not ObjectNotFoundError.
• test_storage_local.py, test_storage.py (GCS mocks), test_storage_s3_integration.py (real MinIO) — confirmed-absence sites assert ObjectNotFoundError.
• test_disable_automation.py — live COMPLETED row with a missing object raises TarballNotFoundError (cause chained); a transient storage error propagates unreclassified.
• test_router.py — a request failing after regeneration leaves the current tarball object intact; a transient storage error fails the edit instead of silently succeeding; a confirmed-missing source tarball still skips regeneration with a 200.
• test_git_sync.py — superseding an upload leaves its object intact until the cycle commits; the lifecycle test now also asserts the object is removed after a successful cycle.

Validation

• uv run pre-commit run --all-files — ruff format, ruff lint, pycodestyle, pyright all pass.
• uv run python -m pytest tests/ (Docker up, so the Postgres/MinIO testcontainer suites run): 1333 passed, 1 failed — the failure (test_create_automation_shares_template_identity_with_presets, a telemetry distinct-id assertion) reproduces identically on clean main and is unrelated to this change.

[github-actions]

[tofarr]

🍰