[Backend] Commit the transcript before deleting session audio in process_audio #404

New issue

Closed

opened 2026-08-25 20:44:43 +00:00 by claude-bot · 2 comments

claude-bot commented

2026-08-25 20:44:43 +00:00

Contributor

Severity: MEDIUM

Found in the August 2026 session lifecycle review (#319).

On the success path of transcription, the raw audio is deleted from disk before the database transaction that saves the transcript is committed — so a database hiccup at exactly the wrong moment can leave a session with no audio, no persisted transcript, and (because the same commit failure likely also breaks the failure-handling path) a status stuck at processing forever.

Evidence

webapp/backend/app/tasks/reminder_tasks.py:2494-2531 (apply_post_processing_audio_retention) — shutil.rmtree(session_dir, ignore_errors=True) at :2524 runs synchronously inside this function.
webapp/backend/app/tasks/reminder_tasks.py:2315 — apply_post_processing_audio_retention(...) is called (triggering the rmtree) before await db.commit() at :2319, which is the commit that actually persists session.transcript/session.summary to the database.

Failure scenario
A 3-hour transcription pipeline finishes successfully; the retention policy is the default delete_after_processing, so the raw WAVs are deleted immediately as part of wrapping up. A split-second later, the database connection blips (network partition, connection pool exhaustion) and db.commit() fails. The transcript and summary that were computed are never persisted, the raw audio that would have let anyone retry is already gone, and because the commit failure likely also prevents the failure-path status update from landing cleanly, the session is left in an inconsistent, possibly-stuck state with nothing left to recover from.

Proposed fix
Reorder the two operations — call await db.commit() first to durably persist the transcript and summary, and only then run apply_post_processing_audio_retention to delete the audio. If the commit fails, the audio is still on disk and the session can simply be retried from the top.

Acceptance criteria

db.commit() for the transcript/summary happens before the audio directory is deleted in process_audio's success path.
A simulated commit failure after transcription leaves the raw audio in place (verifiable by an injected exception in a test).

**Severity: MEDIUM** Found in the August 2026 session lifecycle review (#319). On the success path of transcription, the raw audio is deleted from disk before the database transaction that saves the transcript is committed — so a database hiccup at exactly the wrong moment can leave a session with no audio, no persisted transcript, and (because the same commit failure likely also breaks the failure-handling path) a status stuck at `processing` forever. **Evidence** - `webapp/backend/app/tasks/reminder_tasks.py:2494-2531` (`apply_post_processing_audio_retention`) — `shutil.rmtree(session_dir, ignore_errors=True)` at `:2524` runs synchronously inside this function. - `webapp/backend/app/tasks/reminder_tasks.py:2315` — `apply_post_processing_audio_retention(...)` is called (triggering the rmtree) before `await db.commit()` at `:2319`, which is the commit that actually persists `session.transcript`/`session.summary` to the database. **Failure scenario** A 3-hour transcription pipeline finishes successfully; the retention policy is the default `delete_after_processing`, so the raw WAVs are deleted immediately as part of wrapping up. A split-second later, the database connection blips (network partition, connection pool exhaustion) and `db.commit()` fails. The transcript and summary that were computed are never persisted, the raw audio that would have let anyone retry is already gone, and because the commit failure likely also prevents the failure-path status update from landing cleanly, the session is left in an inconsistent, possibly-stuck state with nothing left to recover from. **Proposed fix** Reorder the two operations — call `await db.commit()` first to durably persist the transcript and summary, and only then run `apply_post_processing_audio_retention` to delete the audio. If the commit fails, the audio is still on disk and the session can simply be retried from the top. **Acceptance criteria** - [ ] `db.commit()` for the transcript/summary happens before the audio directory is deleted in `process_audio`'s success path. - [ ] A simulated commit failure after transcription leaves the raw audio in place (verifiable by an injected exception in a test).

claude-bot added this to the v4.1.0 - Data Durability & Recovery milestone

2026-08-25 20:44:43 +00:00

claude-bot added the

enhancement

backend

labels

2026-08-25 20:44:43 +00:00

claude-bot commented

2026-08-29 01:21:03 +00:00

Author

Contributor

This is resolved by #427's fix (d4881b9 on fix/v4.0.1-retention-safety) — flagging rather than closing, since it is your v4.1.0 milestone to plan.

The hazard here was the ordering in process_audio:

await apply_post_processing_audio_retention(db, session, campaign, session_dir)
await db.commit()

The retention step ran shutil.rmtree(session_dir) — an irreversible filesystem operation — and the commit came after. A commit failure meant the audio was gone and the transcript unsaved: the worst possible pairing, and exactly what this issue was opened about.

There is no longer a delete there at all. #427 changed that function to mark the session trashed and stamp audio_trashed_at, touching only the ORM object. Verified by grep: the sole remaining mention of rmtree inside the function is in its docstring, describing what it used to do.

So the ordering no longer matters in the way that made it dangerous. If the commit fails now, the transaction rolls back, the trash stamp goes with it, and the bytes are still on disk — the sweeper will pick the session up on a later run.

Worth noting the failure mode has inverted, which is the safe direction: previously a mid-path failure destroyed audio that should have been kept; now it keeps audio that should have been trashed. Both are recoverable by the Beat sweep, but only one of them was ever destructive.

Suggest closing once #427 merges, unless you want the commit-before-side-effect ordering enforced as a general principle beyond this one call site — which would be a different and broader piece of work than this issue describes.

**This is resolved by #427's fix** (`d4881b9` on `fix/v4.0.1-retention-safety`) — flagging rather than closing, since it is your v4.1.0 milestone to plan. The hazard here was the ordering in `process_audio`: ```python await apply_post_processing_audio_retention(db, session, campaign, session_dir) await db.commit() ``` The retention step ran `shutil.rmtree(session_dir)` — an irreversible filesystem operation — and the commit came after. A commit failure meant the audio was gone *and* the transcript unsaved: the worst possible pairing, and exactly what this issue was opened about. **There is no longer a delete there at all.** #427 changed that function to mark the session trashed and stamp `audio_trashed_at`, touching only the ORM object. Verified by grep: the sole remaining mention of `rmtree` inside the function is in its docstring, describing what it used to do. So the ordering no longer matters in the way that made it dangerous. If the commit fails now, the transaction rolls back, the trash stamp goes with it, and the bytes are still on disk — the sweeper will pick the session up on a later run. Worth noting the failure mode has **inverted**, which is the safe direction: previously a mid-path failure destroyed audio that should have been kept; now it keeps audio that should have been trashed. Both are recoverable by the Beat sweep, but only one of them was ever destructive. Suggest closing once #427 merges, unless you want the commit-before-side-effect ordering enforced as a general principle beyond this one call site — which would be a different and broader piece of work than this issue describes.

claude-bot referenced this issue

2026-08-29 05:58:03 +00:00

v4.0.1 — retention safety and secrets at rest #435

claude-bot commented

2026-08-29 07:33:29 +00:00

Author

Contributor

Closing — resolved by #427, now deployed to production in v4.0.1 (db77a4e).

The hazard was shutil.rmtree running before db.commit(), so a commit failure meant the audio was destroyed and the transcript unsaved. apply_post_processing_audio_retention no longer performs any filesystem operation at all — it marks the session trashed and stamps audio_trashed_at, and nothing else. A failed commit now rolls back the trash stamp and leaves the bytes on disk for a later sweep.

Verified on the deployed release: production reports backend 4.0.1, the retention step is the trash path, and the only remaining mention of rmtree inside that function is in its docstring describing the old behaviour.

If you later want commit-before-side-effect enforced as a general principle across the async pipelines rather than at this one call site, that is a broader piece of work than this issue describes — #416 ("close the eleven manual-intervention stuck states") is closer to the right home for it.

Closing — resolved by #427, now deployed to production in **v4.0.1** (`db77a4e`). The hazard was `shutil.rmtree` running before `db.commit()`, so a commit failure meant the audio was destroyed *and* the transcript unsaved. `apply_post_processing_audio_retention` no longer performs any filesystem operation at all — it marks the session trashed and stamps `audio_trashed_at`, and nothing else. A failed commit now rolls back the trash stamp and leaves the bytes on disk for a later sweep. Verified on the deployed release: production reports `backend 4.0.1`, the retention step is the trash path, and the only remaining mention of `rmtree` inside that function is in its docstring describing the old behaviour. If you later want commit-before-side-effect enforced as a general principle across the async pipelines rather than at this one call site, that is a broader piece of work than this issue describes — #416 ("close the eleven manual-intervention stuck states") is closer to the right home for it.

claude-bot closed this issue

2026-08-29 07:33:40 +00:00