fix(bot): never sweep a directory that holds a real recording (#399) #447

Merged

claude-bot merged 1 commit from fix/399-never-sweep-real-audio into main 2026-08-29 22:39:38 +00:00

Conversation 0 Commits 1 Files changed 4 +504 -23

claude-bot commented 2026-08-29 19:19:50 +00:00

Contributor

Copy link

Closes the data-loss half of #399 (CRITICAL).

The defect

The startup sweep deleted any session directory that was unmarked and older than an hour — and the marker meaning "the backend owns this" was only written after a successful upload ack. Four ordinary situations therefore left hours of captured speech looking exactly like abandoned scratch:

• the upload POST failing (backend restart, deploy, a few seconds of network trouble)
• a crash between the ack and the marker write
• a crash mid-recording, leaving raw .s16le capture on disk
• #397's own branch, where the backend declines to overwrite an existing transcript

That last one isn't in the issue. It was introduced by a fix, in this same milestone, and it's the sharpest version of the bug — it tells the GM in bold that "Nothing was lost — this recording is saved separately" and then returns without marking anything. The reassurance and the deletion are eleven lines apart.

The sweep's docstring asserted "the backend never received the upload so there is nothing to preserve." That sentence is exactly backwards: an upload the backend never received is precisely when the bot holds the only copy.

The fix

The test is no longer "is it marked and old" but "can I prove there is nothing here to lose." A directory or raw file containing a non-zero-byte track is kept regardless of age or marker, and reported in the log. Only provably-empty scratch is removed.

Disk growth is recoverable; a deleted session is not — the same standard #427 settled on for the backend, now applied to the bot.

Beyond stopping the loss, a failed handoff is retried on the next startup, before anything sweeps. A manifest written when the session directory is created carries the exact payload, so the retry replays the original request rather than reconstructing one. That matters concretely: duration_seconds would otherwise default to 0, which silently disables the backend's coverage guards — the mechanism behind #421. The backend refusing to overwrite is recorded as terminal, so that take is kept but not re-asked on every start.

Two existing tests asserted the destructive behaviour

test_cleanup_removes_old_orphaned_session_dir wrote a non-empty 123.wav and required the sweep to delete it. test_cleanup_removes_old_raw_wav did the same for raw capture. They were the defect written down as the specification — exactly the pattern #441 exists to hunt — and are inverted here, with the history noted in their docstrings so nobody "fixes" them back.

Verification

Mutation-checked: with the content guard bypassed, three tests fail, including the issue's own scenario (four-hour session, upload fails during maintenance, bot redeployed two hours later).

218 bot tests pass (was 211), 1,409 backend.

Not done here

Finalising a mid-recording crash's raw tracks into WAVs and handing them off. They're now preserved and logged with their path rather than deleted, which stops the data loss — but converting them needs session metadata the bot no longer holds at that point. #399 stays open for it rather than being closed on a partial.

Note on branches

This and PR #446 were briefly stacked by mistake — #446 also carried the gitleaks commit. Both have been rebased onto main and force-pushed, so each PR is now a single independent commit. Worth a fresh look at #446's diff if you'd already reviewed it.

🤖 Generated with Claude Code

Closes the data-loss half of #399 (CRITICAL). ## The defect The startup sweep deleted any session directory that was unmarked and older than an hour — and the marker meaning *"the backend owns this"* was only written **after** a successful upload ack. Four ordinary situations therefore left hours of captured speech looking exactly like abandoned scratch: - the upload POST failing (backend restart, deploy, a few seconds of network trouble) - a crash between the ack and the marker write - a crash mid-recording, leaving raw `.s16le` capture on disk - **#397's own branch**, where the backend declines to overwrite an existing transcript That last one isn't in the issue. It was introduced **by a fix, in this same milestone**, and it's the sharpest version of the bug — it tells the GM in bold that *"**Nothing was lost** — this recording is saved separately"* and then returns without marking anything. The reassurance and the deletion are eleven lines apart. The sweep's docstring asserted *"the backend never received the upload so there is nothing to preserve."* That sentence is exactly backwards: an upload the backend never received is precisely when the bot holds the **only** copy. ## The fix The test is no longer "is it marked and old" but **"can I prove there is nothing here to lose."** A directory or raw file containing a non-zero-byte track is kept regardless of age or marker, and reported in the log. Only provably-empty scratch is removed. Disk growth is recoverable; a deleted session is not — the same standard #427 settled on for the backend, now applied to the bot. **Beyond stopping the loss**, a failed handoff is retried on the next startup, before anything sweeps. A manifest written when the session directory is created carries the exact payload, so the retry replays the original request rather than reconstructing one. That matters concretely: `duration_seconds` would otherwise default to `0`, which silently disables the backend's coverage guards — the mechanism behind #421. The backend refusing to overwrite is recorded as terminal, so that take is kept but not re-asked on every start. ## Two existing tests asserted the destructive behaviour `test_cleanup_removes_old_orphaned_session_dir` wrote a **non-empty** `123.wav` and required the sweep to delete it. `test_cleanup_removes_old_raw_wav` did the same for raw capture. They were the defect written down as the specification — exactly the pattern #441 exists to hunt — and are inverted here, with the history noted in their docstrings so nobody "fixes" them back. ## Verification Mutation-checked: with the content guard bypassed, three tests fail, including the issue's own scenario (four-hour session, upload fails during maintenance, bot redeployed two hours later). **218 bot tests pass** (was 211), 1,409 backend. ## Not done here Finalising a mid-recording crash's raw tracks into WAVs and handing them off. They're now **preserved and logged with their path** rather than deleted, which stops the data loss — but converting them needs session metadata the bot no longer holds at that point. #399 stays open for it rather than being closed on a partial. ## Note on branches This and PR #446 were briefly stacked by mistake — #446 also carried the gitleaks commit. Both have been rebased onto `main` and force-pushed, so each PR is now a single independent commit. Worth a fresh look at #446's diff if you'd already reviewed it. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-bot added 1 commit 2026-08-29 19:19:51 +00:00

fix(bot): never sweep a directory that holds a real recording (#399) ... 1f799f0942

The startup sweep deleted any session directory that was unmarked and older
than an hour, and the marker meaning "the backend owns this" was only written
*after* a successful upload ack. Four ordinary situations therefore left hours
of captured speech looking exactly like abandoned scratch:

  - the upload POST failing (backend restart, deploy, brief network trouble)
  - a crash between the ack and the marker write
  - a crash mid-recording, leaving raw .s16le capture on disk
  - #397's own branch, where the backend declines to overwrite an existing
    transcript — which tells the GM in bold that **nothing was lost** and then
    returns without marking anything

The last one is not in the issue. It was introduced by a fix, in this same
milestone, and is the sharpest version of the bug: the reassurance and the
deletion are eleven lines apart.

The sweep's docstring asserted "the backend never received the upload so there
is nothing to preserve". That sentence is exactly backwards — an upload the
backend never received is precisely when the bot holds the only copy.

So the test is no longer "is it marked and old" but "can I prove there is
nothing here to lose". A directory or raw file containing a non-zero-byte
track is kept regardless of age or marker, and reported. Only provably-empty
scratch is removed. Disk growth is recoverable; a deleted session is not —
the standard #427 settled on for the backend, applied to the bot.

Beyond stopping the loss, a failed handoff is now retried on the next startup,
before anything sweeps. A manifest written when the session directory is
created carries the exact payload, so the retry replays the original request
rather than reconstructing one — notably duration_seconds, which defaulted to
0 and would have silently disabled the backend's coverage guards (#421). The
backend refusing to overwrite is recorded as terminal, so it is kept but not
re-asked every start.

Two existing tests asserted the destructive behaviour — a non-empty WAV,
required to be deleted — and are inverted here. They were the defect written
down as the specification, which is the pattern #441 exists to hunt.

Verified by mutation: with the content check bypassed, the three tests that
matter fail, including the issue's own scenario (failed upload, restart past
the age threshold). 218 bot tests pass, 1,409 backend.

Not done here: finalising a mid-recording crash's raw tracks into WAVs and
handing them off. They are preserved and logged rather than converted; #399
stays open for that.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

rbrooks force-pushed fix/399-never-sweep-real-audio from 1f799f0942 to 825ec24287 2026-08-29 22:28:03 +00:00 Compare

claude-bot merged commit d550e95ac8 into main 2026-08-29 22:39:38 +00:00

claude-bot deleted branch fix/399-never-sweep-real-audio 2026-08-29 22:39:39 +00:00

claude-bot referenced this pull request from a commit 2026-08-29 22:39:39 +00:00

Merge PR #447: never sweep a directory that holds a real recording (#399)

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

backend

Issue related to the backend

  

bot

Issue related to the bot

  

bug

Something is not working

  

duplicate

This issue or pull request already exists

  

enhancement

New feature

  

frontend

Issue related to the frontend

  

help wanted

Need some help

  

invalid

Something is wrong

  

question

More information is needed

  

recording

Issue related to the recording system

  

security

Security hardening / vulnerability work

  

wontfix

This won't be fixed

No labels

backend

bot

bug

duplicate

enhancement

frontend

help wanted

invalid

question

recording

security

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

rbrooks/Quest-Board!447

No description provided.