#427 - [Backend] Audio must never be deleted inline — every path goes through the trash with a 7-day grace - rbrooks/Quest-Board

claude-bot commented

2026-08-28 21:19:28 +00:00

Contributor

Severity: CRITICAL. Found by investigating why a real session's audio was gone two days after recording. This deliberately changes the spec — Ryan's call, 2026-08-28.

What happened

The Shifting Caverns, 2026-08-26:

time (UTC)	event
04:08:14	transcript written
04:08:41	`process_audio: audio deleted after processing` — `shutil.rmtree`, gone
05:14:23	GM content-approved the session
05:15:54	GM changed audio retention to `retain_days` / 21

The retention change came 67 minutes too late, and the approval was irrelevant — the audio had already been destroyed during process_audio, an hour before the GM touched anything.

Confirmed unrecoverable: no WAVs anywhere on the host, volume dir mtime 04:08:41.324885769 matching the worker log to the millisecond, no deleted-but-open handles, no filesystem snapshots. The nightly borg archive that morning ran 03:45:34 → 04:15:39 — straddling the deletion — and recorded the volume directory empty, having walked that path after the delete. It missed the only copy by minutes.

The defect

_delete_audio_after_processing (app/tasks/reminder_tasks.py) does exactly what its docstring says:

"In delete_after_processing mode the raw per-speaker WAV directory is removed immediately and the session jumps straight to the terminal approved state (there is nothing left to approve or reprocess), skipping the GM trash/grace flow."

So the trash mechanism — sessions.audio_trashed_at plus audio_trash_retention_days (default 7) — applies only to retain_days and retain_indefinitely. Under delete_after_processing it is bypassed entirely. audio_trashed_at is NULL on every row in the production database, because that path has never trashed anything.

The reasoning ("there is nothing left to approve or reprocess") is wrong in the case that matters. There is always something to reprocess: the pipeline changes. This milestone alone rewrote transcription, extraction, validation and composition, and the single most useful validation available — re-running a real recording through the new code — was impossible because every recording had already been destroyed at the moment its first transcript was written.

Proposed fix — the spec change

No code path deletes raw audio inline. Ever. Deletion happens only after a minimum grace period, in every retention mode:

delete_after_processing stops meaning delete now. It means trash on success — set audio_trashed_at, leave the files, let the sweeper remove them once the grace has elapsed.
The grace floor is 7 days minimum, enforced in code, not merely defaulted. A configured audio_trash_retention_days below 7 is clamped up, so a misconfiguration cannot reintroduce immediate loss.
The session no longer jumps straight to terminal approved on that path; it goes to whatever state means "processed, audio in trash, recoverable".
Restoring from trash (admin.py already clears audio_trashed_at) must work for these sessions too.

Worth deciding as part of this: whether delete_after_processing should be renamed, since it will no longer describe what it does. trash_after_processing is honest.

Acceptance criteria

No retention mode deletes raw audio during process_audio; the inline shutil.rmtree is gone
Every mode routes through the trash, setting audio_trashed_at
A grace floor of 7 days is enforced in code and cannot be configured below
Audio trashed under the former delete_after_processing mode is restorable through the existing admin flow
A test asserts audio still exists on disk immediately after process_audio completes in every mode — it must fail against the current implementation
A test asserts the sweeper does not delete before the grace has elapsed, and does after
The mode's name and its Admin description match what it now does

Note

v4.1.0 - Data Durability & Recovery already lists "retention safety" in its description, and this belongs to that theme. It is in v4.0.1 because v4.0.0 has shipped a pipeline whose output nobody can re-verify against real audio, and every session recorded between now and the fix is another one lost.

**Severity: CRITICAL.** Found by investigating why a real session's audio was gone two days after recording. **This deliberately changes the spec** — Ryan's call, 2026-08-28. ## What happened The Shifting Caverns, 2026-08-26: | time (UTC) | event | |---|---| | 04:08:14 | transcript written | | **04:08:41** | `process_audio: audio deleted after processing` — `shutil.rmtree`, gone | | 05:14:23 | GM content-approved the session | | 05:15:54 | GM changed audio retention to `retain_days` / 21 | The retention change came **67 minutes too late**, and the approval was irrelevant — the audio had already been destroyed during `process_audio`, an hour before the GM touched anything. Confirmed unrecoverable: no WAVs anywhere on the host, volume dir mtime `04:08:41.324885769` matching the worker log to the millisecond, no deleted-but-open handles, no filesystem snapshots. The nightly borg archive that morning ran 03:45:34 → 04:15:39 — *straddling* the deletion — and recorded the volume directory **empty**, having walked that path after the delete. It missed the only copy by minutes. ## The defect `_delete_audio_after_processing` (`app/tasks/reminder_tasks.py`) does exactly what its docstring says: > *"In delete_after_processing mode the raw per-speaker WAV directory is removed **immediately** and the session jumps straight to the terminal `approved` state (there is nothing left to approve or reprocess), **skipping the GM trash/grace flow**."* So the trash mechanism — `sessions.audio_trashed_at` plus `audio_trash_retention_days` (default 7) — applies only to `retain_days` and `retain_indefinitely`. Under `delete_after_processing` it is bypassed entirely. `audio_trashed_at` is NULL on **every row in the production database**, because that path has never trashed anything. The reasoning ("there is nothing left to approve or reprocess") is wrong in the case that matters. There is always something to reprocess: the pipeline changes. This milestone alone rewrote transcription, extraction, validation and composition, and the single most useful validation available — re-running a real recording through the new code — was impossible because every recording had already been destroyed at the moment its first transcript was written. ## Proposed fix — the spec change **No code path deletes raw audio inline. Ever.** Deletion happens only after a minimum grace period, in every retention mode: - `delete_after_processing` stops meaning *delete now*. It means **trash on success** — set `audio_trashed_at`, leave the files, let the sweeper remove them once the grace has elapsed. - The grace floor is **7 days minimum**, enforced in code, not merely defaulted. A configured `audio_trash_retention_days` below 7 is clamped up, so a misconfiguration cannot reintroduce immediate loss. - The session no longer jumps straight to terminal `approved` on that path; it goes to whatever state means "processed, audio in trash, recoverable". - Restoring from trash (`admin.py` already clears `audio_trashed_at`) must work for these sessions too. Worth deciding as part of this: whether `delete_after_processing` should be renamed, since it will no longer describe what it does. `trash_after_processing` is honest. ## Acceptance criteria - [ ] No retention mode deletes raw audio during `process_audio`; the inline `shutil.rmtree` is gone - [ ] Every mode routes through the trash, setting `audio_trashed_at` - [ ] A grace floor of 7 days is enforced in code and cannot be configured below - [ ] Audio trashed under the former `delete_after_processing` mode is restorable through the existing admin flow - [ ] A test asserts audio still exists on disk immediately after `process_audio` completes in *every* mode — it must fail against the current implementation - [ ] A test asserts the sweeper does not delete before the grace has elapsed, and does after - [ ] The mode's name and its Admin description match what it now does ## Note `v4.1.0 - Data Durability & Recovery` already lists "retention safety" in its description, and this belongs to that theme. It is in v4.0.1 because v4.0.0 has shipped a pipeline whose output nobody can re-verify against real audio, and every session recorded between now and the fix is another one lost.

claude-bot added this to the v4.0.1 - Retention Safety & Secrets at Rest milestone

2026-08-28 21:20:44 +00:00

claude-bot added the

backend

bug

recording

labels

2026-08-28 21:20:58 +00:00

rbrooks referenced this issue from a commit

2026-08-29 00:26:25 +00:00

fix(backend): never delete raw audio inline (#427, #432)

claude-bot commented

2026-08-29 00:26:43 +00:00

Author

Contributor

Implemented in d4881b9 on fix/v4.0.1-retention-safety.

What changed

apply_post_processing_audio_retention no longer deletes. It marks the session trashed, stamps audio_trashed_at, and keeps audio_file_path — the sweeper needs it to find the directory and the admin restore flow needs it to bring the session back.

The mode is renamed to trash_after_processing, with delete_after_processing accepted on read and normalised through normalise_audio_retention_mode. Every install predating this has the old value written into app_settings and into campaigns.audio_retention_mode, so a rename without an alias would have silently dropped those installs back to the default. No data migration needed.

Two things that would have made this cosmetic

Neither was in the issue, and both were found by reading the sweeper rather than by running anything.

Sweeper pass 2 would have deleted the audio anyway. It removes any session whose retention window has elapsed, and its only guard was if not session.audio_file_path. A trashed session keeps its path — and trash-on-success stamps the trash at the exact moment the transcript is written — so the very next Beat tick would have deleted it, grace unobserved. Pass 2 now skips trashed sessions; they belong to pass 1 and its grace.

That race was already reachable without this change: a GM who trashed an older session by hand, whose retention window had passed, lost it on the next tick rather than after seven days.

An expiring window still deleted directly. The criterion "every mode routes through the trash" was not satisfied by retain_days, which deleted straight from pass 2. It now trashes instead, so deletion is always reached through the same recoverable window.

⚠️ This changes effective retention: audio under retain_days now lives for the configured window plus the trash grace. That is the intent — an expiring window should give the GM a last chance to notice rather than delete the only copy on a scheduled task — but it is a disk-usage change worth knowing about, and it is called out in the CHANGELOG.

Verification

The sweeper test was run against the code with the pass-2 guard reverted and fails there with "grace period was not honoured". The trash-on-success test fails against the old implementation on its first assert, since shutil.rmtree had already removed the directory.

1,378 backend tests and 440 frontend tests pass.

Acceptance criteria

No retention mode deletes raw audio during process_audio; the inline shutil.rmtree is gone
Every mode routes through the trash, setting audio_trashed_at
A grace floor of 7 days is enforced in code and cannot be configured below
Audio trashed under the former delete_after_processing mode is restorable through the existing admin flow — it reaches the same trashed state the admin restore endpoint already clears
A test asserts audio still exists on disk immediately after process_audio completes — and fails against the current implementation
A test asserts the sweeper does not delete before the grace has elapsed, and does after
The mode's name and its Admin description match what it now does — Admin now reads "Move to trash after processing (recommended)", and the warning about losing the recording is replaced with an explanation of the recovery window

Not done here

Production is on retain_days/21 globally with both campaigns inheriting it, and no session currently has audio on disk, so there is nothing to migrate or rescue. The exposure was mitigated by configuration; this makes it safe by construction.

Implemented in `d4881b9` on `fix/v4.0.1-retention-safety`. ## What changed `apply_post_processing_audio_retention` no longer deletes. It marks the session `trashed`, stamps `audio_trashed_at`, and **keeps `audio_file_path`** — the sweeper needs it to find the directory and the admin restore flow needs it to bring the session back. The mode is renamed to `trash_after_processing`, with `delete_after_processing` accepted on read and normalised through `normalise_audio_retention_mode`. Every install predating this has the old value written into `app_settings` *and* into `campaigns.audio_retention_mode`, so a rename without an alias would have silently dropped those installs back to the default. No data migration needed. ## Two things that would have made this cosmetic Neither was in the issue, and both were found by reading the sweeper rather than by running anything. **Sweeper pass 2 would have deleted the audio anyway.** It removes any session whose retention window has elapsed, and its only guard was `if not session.audio_file_path`. A trashed session keeps its path — and trash-on-success stamps the trash at the exact moment the transcript is written — so the very next Beat tick would have deleted it, grace unobserved. Pass 2 now skips `trashed` sessions; they belong to pass 1 and its grace. That race was **already reachable** without this change: a GM who trashed an older session by hand, whose retention window had passed, lost it on the next tick rather than after seven days. **An expiring window still deleted directly.** The criterion *"every mode routes through the trash"* was not satisfied by `retain_days`, which deleted straight from pass 2. It now trashes instead, so deletion is always reached through the same recoverable window. ⚠️ **This changes effective retention**: audio under `retain_days` now lives for the configured window **plus** the trash grace. That is the intent — an expiring window should give the GM a last chance to notice rather than delete the only copy on a scheduled task — but it is a disk-usage change worth knowing about, and it is called out in the CHANGELOG. ## Verification The sweeper test was run against the code with the pass-2 guard reverted and fails there with *"grace period was not honoured"*. The trash-on-success test fails against the old implementation on its first assert, since `shutil.rmtree` had already removed the directory. 1,378 backend tests and 440 frontend tests pass. ## Acceptance criteria - [x] No retention mode deletes raw audio during `process_audio`; the inline `shutil.rmtree` is gone - [x] Every mode routes through the trash, setting `audio_trashed_at` - [x] A grace floor of 7 days is enforced in code and cannot be configured below - [x] Audio trashed under the former `delete_after_processing` mode is restorable through the existing admin flow — it reaches the same `trashed` state the admin restore endpoint already clears - [x] A test asserts audio still exists on disk immediately after `process_audio` completes — and fails against the current implementation - [x] A test asserts the sweeper does not delete before the grace has elapsed, and does after - [x] The mode's name and its Admin description match what it now does — Admin now reads *"Move to trash after processing (recommended)"*, and the warning about losing the recording is replaced with an explanation of the recovery window ## Not done here Production is on `retain_days`/21 globally with both campaigns inheriting it, and no session currently has audio on disk, so there is nothing to migrate or rescue. The exposure was mitigated by configuration; this makes it safe by construction.

Rows
Columns

[Backend] Audio must never be deleted inline — every path goes through the trash with a 7-day grace #427

What happened

The defect

Proposed fix — the spec change

Acceptance criteria

Note

What changed

Two things that would have made this cosmetic

Verification

Acceptance criteria

Not done here