From 5effa7c96ed4bb132da47a92143d002c281e610c Mon Sep 17 00:00:00 2001 From: Brummel Date: Mon, 20 Apr 2026 15:48:34 +0200 Subject: [PATCH] feat: Add endpoint to delete recordings This commit introduces a new API endpoint for deleting recordings. The endpoint handles the removal of the audio file and its associated sidecar files (transcript, duration). It also invalidates derived artifacts like oneliner.json, document.md, and analysis_input.json. Additionally, it includes checks for path traversal, correct file extensions, and case ownership to ensure security and data integrity. A new `RecordingDeleted` event is also added to the `CaseEventKind` enum. --- server/src/events.rs | 1 + server/src/lib.rs | 17 ++ server/src/main.rs | 2 + server/src/routes/case_actions.rs | 83 ++++++- server/src/routes/mod.rs | 4 + server/src/routes/user_web.rs | 68 +++++- server/src/routes/web.rs | 2 +- server/src/transcribe/recovery.rs | 71 ++++-- server/templates/case_recordings.html | 46 +++- server/tests/delete_recording_test.rs | 305 ++++++++++++++++++++++++++ server/tests/magic_link_test.rs | 2 + 11 files changed, 567 insertions(+), 34 deletions(-) create mode 100644 server/tests/delete_recording_test.rs diff --git a/server/src/events.rs b/server/src/events.rs index a1df45f..111a53d 100644 --- a/server/src/events.rs +++ b/server/src/events.rs @@ -24,6 +24,7 @@ pub const EVENT_CHANNEL_CAPACITY: usize = 256; #[serde(rename_all = "snake_case")] pub enum CaseEventKind { RecordingUploaded, + RecordingDeleted, TranscriptReady, TranscriptFailed, OnelinerUpdated, diff --git a/server/src/lib.rs b/server/src/lib.rs index 15b2ddf..9b5a48c 100644 --- a/server/src/lib.rs +++ b/server/src/lib.rs @@ -20,6 +20,7 @@ use axum::extract::FromRef; use analyze::AnalyzeSender; use config::Config; +use gazetteer::Gazetteer; use magic_link::MagicLinkStore; use transcribe::TranscribeSender; use web_session::SessionStore; @@ -80,6 +81,8 @@ pub struct AppState { pub analyze_busy: AnalyzeBusy, pub transcribe_busy: TranscribeBusy, pub events_tx: events::EventSender, + pub http_client: reqwest::Client, + pub vocab: Arc, } impl FromRef for Arc { @@ -141,6 +144,18 @@ impl FromRef for events::EventSender { } } +impl FromRef for reqwest::Client { + fn from_ref(state: &AppState) -> Self { + state.http_client.clone() + } +} + +impl FromRef for Arc { + fn from_ref(state: &AppState) -> Self { + state.vocab.clone() + } +} + /// Test/simple entrypoint: jobs pushed into either channel are dropped /// because the receivers are not retained. Use [`create_router_with_state`] /// from `main.rs` where real workers own the receivers. @@ -156,6 +171,8 @@ pub fn create_router(config: Arc) -> Router { analyze_busy: AnalyzeBusy(Arc::new(AtomicBool::new(false))), transcribe_busy: TranscribeBusy(Arc::new(AtomicBool::new(false))), events_tx: events::channel(), + http_client: reqwest::Client::new(), + vocab: Arc::new(Gazetteer::empty()), }) } diff --git a/server/src/main.rs b/server/src/main.rs index 944255b..5e6008b 100644 --- a/server/src/main.rs +++ b/server/src/main.rs @@ -175,6 +175,8 @@ async fn main() { analyze_busy: doctate_server::AnalyzeBusy(analyze_busy), transcribe_busy: doctate_server::TranscribeBusy(transcribe_busy), events_tx, + http_client, + vocab, }; let addr = format!("0.0.0.0:{}", config.server_port); diff --git a/server/src/routes/case_actions.rs b/server/src/routes/case_actions.rs index 78f36d5..a0eb207 100644 --- a/server/src/routes/case_actions.rs +++ b/server/src/routes/case_actions.rs @@ -2,9 +2,10 @@ use std::path::{Path, PathBuf}; use std::sync::Arc; use std::time::SystemTime; -use axum::extract::State; +use axum::extract::{Form, State}; use axum::http::HeaderMap; use axum::response::Redirect; +use serde::Deserialize; use time::OffsetDateTime; use time::format_description::well_known::Rfc3339; use tokio::io::AsyncWriteExt; @@ -23,6 +24,7 @@ use crate::error::AppError; use crate::events::{self, CaseEventKind, EventSender}; use crate::paths::{DELETE_MARKER, DeleteMarker, read_delete_marker, write_delete_marker}; use crate::routes::user_web::locate_case_or_404; +use crate::routes::web::validate_filename; /// POST /web/cases/{case_id}/analyze /// @@ -460,6 +462,85 @@ async fn restore_batch( count } +#[derive(Deserialize)] +pub struct DeleteRecordingForm { + pub filename: String, +} + +/// POST /web/cases/{case_id}/recordings/delete +/// +/// Hard-delete a single recording plus its transcript / duration sidecars +/// and invalidate derived artefacts (`oneliner.json`, `document.md`, +/// `analysis_input.json`). Regeneration is left to the existing +/// auto-trigger paths that run on every view request: the analyze +/// auto-trigger (`analyze::auto_trigger::try_enqueue`) and the oneliner +/// self-heal folded into `PipelineState::heal_orphans_if_idle` will pick +/// the case back up on the next page load or SSE-driven reload. +/// +/// Security: `validate_filename` rejects path traversal and non-audio +/// extensions; `locate_case_or_404` enforces case ownership (IDOR) and +/// hides soft-deleted cases. +pub async fn handle_delete_recording( + user: AuthenticatedWebUser, + State(config): State>, + State(events_tx): State, + CaseIdPath(case_id): CaseIdPath, + headers: HeaderMap, + Form(form): Form, +) -> Result { + validate_filename(&form.filename)?; + + let user_root = config.data_path.join(&user.slug); + let case_dir = locate_case_or_404(&user_root, &case_id, &user.slug, "delete recording").await?; + + let stem = form + .filename + .trim_end_matches(".failed") + .trim_end_matches(".m4a"); + + // Audio and its sidecars. NotFound on any of these is fine: the + // transcript / duration may never have been written (silence, failed + // job, or the file got deleted twice). + let targets: [PathBuf; 5] = [ + case_dir.join(&form.filename), + case_dir.join(format!("{stem}.transcript.txt")), + case_dir.join(format!("{stem}.duration.txt")), + case_dir.join(DOCUMENT_FILE), + case_dir.join(ANALYSIS_INPUT_FILE), + ]; + for p in &targets { + remove_if_exists(p).await?; + } + remove_if_exists(&case_dir.join(ONELINER_FILENAME)).await?; + + info!( + slug = %user.slug, + case_id = %case_id, + filename = %form.filename, + "recording deleted" + ); + + events::emit( + &events_tx, + &user.slug, + case_id.to_string(), + CaseEventKind::RecordingDeleted, + ); + + Ok(Redirect::to(&resolve_return_path(&headers))) +} + +async fn remove_if_exists(path: &Path) -> Result<(), AppError> { + match tokio::fs::remove_file(path).await { + Ok(_) => Ok(()), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(e) => Err(AppError::Internal(format!( + "remove {}: {e}", + path.display() + ))), + } +} + #[cfg(test)] mod tests { use super::resolve_return_path; diff --git a/server/src/routes/mod.rs b/server/src/routes/mod.rs index 8f08e92..abb5a3a 100644 --- a/server/src/routes/mod.rs +++ b/server/src/routes/mod.rs @@ -34,6 +34,10 @@ pub fn api_router() -> Router { "/web/cases/{case_id}/recordings", get(user_web::handle_case_recordings), ) + .route( + "/web/cases/{case_id}/recordings/delete", + post(case_actions::handle_delete_recording), + ) .route( "/web/cases/{case_id}/analyze", post(case_actions::handle_analyze_case), diff --git a/server/src/routes/user_web.rs b/server/src/routes/user_web.rs index de91693..18878ed 100644 --- a/server/src/routes/user_web.rs +++ b/server/src/routes/user_web.rs @@ -19,6 +19,7 @@ use crate::case_id::{CaseId, CaseIdPath}; use crate::config::Config; use crate::error::AppError; use crate::events::EventSender; +use crate::gazetteer::Gazetteer; use crate::paths; use crate::routes::case_actions::read_document; use crate::routes::web::{RecordingView, scan_recordings}; @@ -251,14 +252,39 @@ pub(crate) async fn any_document_exists(case_dir: &Path) -> bool { /// Self-heal: if a worker is idle but orphans exist on disk for this user, /// re-enqueue them. Page-load is the trigger; no cron, no periodic task. /// Runs for both pipelines so a page-load on either view cleans both. +/// +/// Also opportunistically regenerates missing oneliners for the user — a +/// recording-delete leaves `oneliner.json` gone, and without this heal +/// call the stale state would only recover on server restart. impl PipelineState { - pub async fn heal_orphans_if_idle(&self, user_root: &Path, slug: &str) { + #[allow(clippy::too_many_arguments)] + pub async fn heal_orphans_if_idle( + &self, + user_root: &Path, + slug: &str, + http_client: &reqwest::Client, + config: &Config, + vocab: &Gazetteer, + events_tx: &EventSender, + ) { if !self.analyze_busy.0.load(Ordering::Acquire) { analyze_recovery::enqueue_pending_for_user(user_root, &self.analyze_tx).await; } if !self.transcribe_busy.0.load(Ordering::Acquire) { transcribe_recovery::enqueue_pending_for_user(user_root, slug, &self.transcribe_tx) .await; + // Oneliner self-heal: if the transcribe worker is busy, it will + // write a fresh oneliner at batch-end anyway — skip to avoid a + // redundant LLM call. + transcribe_recovery::regenerate_missing_oneliners_for_user( + user_root, + slug, + http_client, + config, + vocab, + events_tx, + ) + .await; } } } @@ -268,9 +294,20 @@ pub async fn handle_my_cases( State(config): State>, State(pipeline): State, State(events_tx): State, + State(http_client): State, + State(vocab): State>, ) -> Result, AppError> { let user_root = config.data_path.join(&user.slug); - pipeline.heal_orphans_if_idle(&user_root, &user.slug).await; + pipeline + .heal_orphans_if_idle( + &user_root, + &user.slug, + &http_client, + &config, + &vocab, + &events_tx, + ) + .await; // Auto-analysis: hand every eligible case to the worker before we // render. The common case is "nothing to do" and costs a handful of // stat-calls per case; actual enqueues happen only when pre-conditions @@ -310,10 +347,21 @@ pub async fn handle_case_page( State(config): State>, State(pipeline): State, State(events_tx): State, + State(http_client): State, + State(vocab): State>, CaseIdPath(case_id): CaseIdPath, ) -> Result, AppError> { let user_root = config.data_path.join(&user.slug); - pipeline.heal_orphans_if_idle(&user_root, &user.slug).await; + pipeline + .heal_orphans_if_idle( + &user_root, + &user.slug, + &http_client, + &config, + &vocab, + &events_tx, + ) + .await; let case_dir = locate_case_or_404( &user_root, @@ -381,10 +429,22 @@ pub async fn handle_case_recordings( user: AuthenticatedWebUser, State(config): State>, State(pipeline): State, + State(events_tx): State, + State(http_client): State, + State(vocab): State>, CaseIdPath(case_id): CaseIdPath, ) -> Result, AppError> { let user_root = config.data_path.join(&user.slug); - pipeline.heal_orphans_if_idle(&user_root, &user.slug).await; + pipeline + .heal_orphans_if_idle( + &user_root, + &user.slug, + &http_client, + &config, + &vocab, + &events_tx, + ) + .await; let case_dir = locate_case_or_404( &user_root, diff --git a/server/src/routes/web.rs b/server/src/routes/web.rs index 5aed64b..ac2a2b8 100644 --- a/server/src/routes/web.rs +++ b/server/src/routes/web.rs @@ -140,7 +140,7 @@ fn validate_user_slug(user: &str) -> Result<(), AppError> { Ok(()) } -fn validate_filename(filename: &str) -> Result<(), AppError> { +pub(crate) fn validate_filename(filename: &str) -> Result<(), AppError> { let ok_suffix = filename.ends_with(".m4a") || filename.ends_with(".m4a.failed"); if !ok_suffix || filename.contains('/') || filename.contains('\\') || filename.contains("..") { return Err(AppError::BadRequest("Invalid filename".into())); diff --git a/server/src/transcribe/recovery.rs b/server/src/transcribe/recovery.rs index b70c717..7fe3701 100644 --- a/server/src/transcribe/recovery.rs +++ b/server/src/transcribe/recovery.rs @@ -110,31 +110,43 @@ pub(crate) async fn cases_needing_oneliner(data_path: &Path) -> Vec<(PathBuf, St let Some(slug) = user_entry.file_name().to_str().map(str::to_owned) else { continue; }; - let Ok(mut cases) = tokio::fs::read_dir(&user_path).await else { - continue; - }; - while let Ok(Some(case_entry)) = cases.next_entry().await { - let case_dir = case_entry.path(); - if !case_dir.is_dir() || paths::is_deleted(&case_dir).await { - continue; - } - let (state, _) = paths::read_oneliner_state(&case_dir).await; - let needs_retry = match state { - None | Some(OnelinerState::Error { .. }) => true, - Some(OnelinerState::Ready { .. } | OnelinerState::Empty { .. }) => false, - }; - if !needs_retry { - continue; - } - if has_non_empty_transcript(&case_dir).await { - out.push((case_dir, slug.clone())); - } + for case_dir in cases_needing_oneliner_in(&user_path).await { + out.push((case_dir, slug.clone())); } } out.sort_by(|a, b| a.0.cmp(&b.0)); out } +/// Per-user variant of [`cases_needing_oneliner`]: scans only `user_root`'s +/// case dirs. Used both as the building block for the all-users scan and +/// from the page-load self-heal path. +pub(crate) async fn cases_needing_oneliner_in(user_root: &Path) -> Vec { + let mut out: Vec = Vec::new(); + let Ok(mut cases) = tokio::fs::read_dir(user_root).await else { + return out; + }; + while let Ok(Some(case_entry)) = cases.next_entry().await { + let case_dir = case_entry.path(); + if !case_dir.is_dir() || paths::is_deleted(&case_dir).await { + continue; + } + let (state, _) = paths::read_oneliner_state(&case_dir).await; + let needs_retry = match state { + None | Some(OnelinerState::Error { .. }) => true, + Some(OnelinerState::Ready { .. } | OnelinerState::Empty { .. }) => false, + }; + if !needs_retry { + continue; + } + if has_non_empty_transcript(&case_dir).await { + out.push(case_dir); + } + } + out.sort(); + out +} + async fn has_non_empty_transcript(case_dir: &Path) -> bool { let Ok(mut files) = tokio::fs::read_dir(case_dir).await else { return false; @@ -178,6 +190,27 @@ pub async fn regenerate_missing_oneliners( } } +/// Per-user variant of [`regenerate_missing_oneliners`]: fixes only the +/// given user's cases. Intended for the page-load self-heal path, so a +/// single missing oneliner is restored as soon as its owner opens a page +/// — no server restart needed. +pub async fn regenerate_missing_oneliners_for_user( + user_root: &Path, + slug: &str, + client: &reqwest::Client, + config: &Config, + vocab: &Gazetteer, + events_tx: &EventSender, +) { + let cases = cases_needing_oneliner_in(user_root).await; + if cases.is_empty() { + return; + } + for case_dir in cases { + super::worker::update_oneliner(&case_dir, slug, client, config, vocab, events_tx).await; + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/server/templates/case_recordings.html b/server/templates/case_recordings.html index db8de6b..09ab27e 100644 --- a/server/templates/case_recordings.html +++ b/server/templates/case_recordings.html @@ -14,7 +14,7 @@ header form { margin: 0; } .recording { margin: 1em 0; padding: 0.7em; border: 1px solid #ddd; border-radius: 4px; } .recording.failed-row { background: #fff7f7; border-left: 3px solid #e24a4a; } .recording-head { display: flex; align-items: center; gap: 1em; flex-wrap: wrap; } -.rec-time { font-family: monospace; font-size: 0.95em; min-width: 11em; color: #333; } +.rec-time { font-size: 0.95em; color: #666; } .filename { font-family: monospace; font-size: 0.8em; color: #999; } .transcript { margin: 0.6em 0 0; padding: 0.6em; background: #f6f6f6; border-radius: 4px; white-space: pre-wrap; font-family: serif; } .pending { color: #888; font-style: italic; margin-top: 0.5em; } @@ -61,6 +61,15 @@ header form { margin: 0; } .header-right { display: flex; align-items: center; gap: 0.8em; } .admin-toggle { font-size: 0.85em; color: #666; display: inline-flex; align-items: center; gap: 0.3em; cursor: pointer; user-select: none; } html.admin-view-off .admin-only { display: none !important; } + +.rec-delete-form { margin: 0; } +.rec-delete-form .delete-btn { + padding: 0.3em; background: transparent; border: none; color: #888; + cursor: pointer; display: inline-flex; align-items: center; + line-height: 0; border-radius: 4px; + transition: color 0.15s, background 0.15s; +} +.rec-delete-form .delete-btn:hover { color: #e24a4a; background: #fff0f0; } +