From 65bd607ff01de0e49c4767b7698cec8be1e7ad87 Mon Sep 17 00:00:00 2001 From: Richie McIlroy <33632126+richiemcilroy@users.noreply.github.com> Date: Sat, 24 Oct 2026 12:23:36 +0100 Subject: [PATCH 1/9] feat(web): record why the desktop app cleaned up an Instant start --- .../unit/desktop-video-create.test.ts | 73 +++++++++++++++++++ apps/web/app/api/desktop/[...route]/video.ts | 13 ++++ apps/web/lib/desktop-instant-cleanup.ts | 35 +++++++++ 3 files changed, 121 insertions(+) create mode 100644 apps/web/lib/desktop-instant-cleanup.ts diff --git a/apps/web/__tests__/unit/desktop-video-create.test.ts b/apps/web/__tests__/unit/desktop-video-create.test.ts index 795c0aaeb93..4ad18b6569f 100644 --- a/apps/web/__tests__/unit/desktop-video-create.test.ts +++ b/apps/web/__tests__/unit/desktop-video-create.test.ts @@ -124,6 +124,10 @@ vi.mock("@/lib/google-drive-storage-quota", () => ({ invalidateGoogleDriveStorageQuotaCache: vi.fn(), })); +vi.mock("@/lib/server-analytics", () => ({ + trackServerEvent: vi.fn(), +})); + // The live-transcription stack drags in the whole workflow graph // (server-only modules included); these tests only care that create works. vi.mock("@/lib/live-transcribe", () => ({ @@ -143,6 +147,7 @@ const { invalidateGoogleDriveStorageQuotaCache } = await import( "@/lib/google-drive-storage-quota" ); const { maybeStartLiveTranscription } = await import("@/lib/live-transcribe"); +const { trackServerEvent } = await import("@/lib/server-analytics"); function resetMockDb() { for (const key of Object.keys(mockDb)) { @@ -440,6 +445,74 @@ describe("DELETE /delete", () => { expect(deletion.deleteVideo).not.toHaveBeenCalled(); expect(mockDb.delete).not.toHaveBeenCalled(); }); + + it("reports why the desktop app cleaned up an Instant start after deleting it", async () => { + mockDb.where.mockResolvedValue([ + { video: { ...ownedVideo, source: { type: "desktopSegments" } } }, + ]); + const response = await app.request( + "https://cap.test/delete?videoId=video-1&reason=start-failed&cause=microphone", + { + method: "DELETE", + headers: { "X-Cap-Desktop-Features": "googleDriveUpload,gpui" }, + }, + ); + expect(response.status).toBe(200); + expect(await response.json()).toBe(true); + expect(trackServerEvent).toHaveBeenCalledExactlyOnceWith( + "user-1", + "desktop_instant_cleanup", + { + reason: "start-failed", + cause: "microphone", + app: "gpui", + sourceType: "desktopSegments", + }, + ); + }); + + it.each([ + "https://cap.test/delete?videoId=video-1", + "https://cap.test/delete?videoId=video-1&reason=unknown&cause=microphone", + ])( + "deletes without reporting when no known reason is sent: %s", + async (url) => { + const response = await app.request(url, { method: "DELETE" }); + expect(response.status).toBe(200); + expect(await response.json()).toBe(true); + expect(deletion.deleteVideo).toHaveBeenCalledExactlyOnceWith("video-1"); + expect(trackServerEvent).not.toHaveBeenCalled(); + }, + ); + + it("drops an unknown cause and labels other desktop apps", async () => { + const response = await app.request( + "https://cap.test/delete?videoId=video-1&reason=interrupted&cause=../../etc", + { method: "DELETE" }, + ); + expect(response.status).toBe(200); + expect(trackServerEvent).toHaveBeenCalledExactlyOnceWith( + "user-1", + "desktop_instant_cleanup", + { reason: "interrupted", cause: null, app: "desktop", sourceType: null }, + ); + }); + + it("reports nothing when the deletion itself fails", async () => { + deletion.deleteVideo.mockReturnValue( + Effect.fail( + new StorageDomain.StorageError({ + cause: new Error("Provider deletion failed"), + }), + ), + ); + const response = await app.request( + "https://cap.test/delete?videoId=video-1&reason=start-failed", + { method: "DELETE" }, + ); + expect(response.status).toBe(500); + expect(trackServerEvent).not.toHaveBeenCalled(); + }); }); describe("GET /create", () => { diff --git a/apps/web/app/api/desktop/[...route]/video.ts b/apps/web/app/api/desktop/[...route]/video.ts index 8565e429fd9..83ea3de6bca 100644 --- a/apps/web/app/api/desktop/[...route]/video.ts +++ b/apps/web/app/api/desktop/[...route]/video.ts @@ -21,9 +21,11 @@ import { Effect, Option } from "effect"; import { Hono } from "hono"; import { after } from "next/server"; import { z } from "zod"; +import { readInstantCleanupReport } from "@/lib/desktop-instant-cleanup"; import { invalidateGoogleDriveStorageQuotaCache } from "@/lib/google-drive-storage-quota"; import { maybeStartLiveTranscription } from "@/lib/live-transcribe"; import { runPromise } from "@/lib/server"; +import { trackServerEvent } from "@/lib/server-analytics"; import { GOOGLE_DRIVE_UPLOAD_FEATURE, hasDesktopFeature, @@ -443,6 +445,17 @@ app.delete( result.video.storageIntegrationId, ); + const cleanup = readInstantCleanupReport( + c.req.query("reason"), + c.req.query("cause"), + hasDesktopFeature(c.req, "gpui"), + ); + if (cleanup) + trackServerEvent(user.id, "desktop_instant_cleanup", { + ...cleanup, + sourceType: result.video.source?.type ?? null, + }); + return c.json(true); } catch (error) { console.error("Error in video delete endpoint:", error); diff --git a/apps/web/lib/desktop-instant-cleanup.ts b/apps/web/lib/desktop-instant-cleanup.ts new file mode 100644 index 00000000000..4591024e725 --- /dev/null +++ b/apps/web/lib/desktop-instant-cleanup.ts @@ -0,0 +1,35 @@ +const CLEANUP_REASONS = new Set([ + "start-failed", + "start-cancelled", + "interrupted", +]); + +const START_FAILURE_CAUSES = new Set([ + "permission", + "microphone", + "camera", + "capture", + "storage", + "account", + "network", + "other", +]); + +export type InstantCleanupReport = { + reason: string; + cause: string | null; + app: "gpui" | "desktop"; +}; + +export function readInstantCleanupReport( + reason: string | undefined, + cause: string | undefined, + isGpui: boolean, +): InstantCleanupReport | null { + if (!reason || !CLEANUP_REASONS.has(reason)) return null; + return { + reason, + cause: cause && START_FAILURE_CAUSES.has(cause) ? cause : null, + app: isGpui ? "gpui" : "desktop", + }; +} From 8bb3fa4e96830de189067868b6dba1e84965a6ce Mon Sep 17 00:00:00 2001 From: Richie McIlroy <33632126+richiemcilroy@users.noreply.github.com> Date: Sat, 24 Oct 2026 12:23:37 +0100 Subject: [PATCH 2/9] fix(desktop-gpui): retry Instant start cleanup until the empty server video is gone --- apps/desktop-gpui/src/auth.rs | 2 +- apps/desktop-gpui/src/main.rs | 1 + apps/desktop-gpui/src/recording.rs | 41 +- apps/desktop-gpui/src/session.rs | 22 +- apps/desktop-gpui/src/upload.rs | 14 +- .../src/upload/instant_cleanup.rs | 1136 +++++++++++++++++ apps/desktop-gpui/src/upload/queue.rs | 4 + 7 files changed, 1204 insertions(+), 16 deletions(-) create mode 100644 apps/desktop-gpui/src/upload/instant_cleanup.rs diff --git a/apps/desktop-gpui/src/auth.rs b/apps/desktop-gpui/src/auth.rs index 59a2018957e..635dd4635a9 100644 --- a/apps/desktop-gpui/src/auth.rs +++ b/apps/desktop-gpui/src/auth.rs @@ -246,7 +246,7 @@ pub fn sign_out() -> bool { pub fn apply_desktop_headers(mut request: reqwest::RequestBuilder) -> reqwest::RequestBuilder { request = request .header("X-Cap-Desktop-Version", env!("CARGO_PKG_VERSION")) - .header("X-Cap-Desktop-Features", "googleDriveUpload"); + .header("X-Cap-Desktop-Features", "googleDriveUpload,gpui"); if let Ok(secret) = std::env::var("VITE_VERCEL_AUTOMATION_BYPASS_SECRET") && !secret.is_empty() { diff --git a/apps/desktop-gpui/src/main.rs b/apps/desktop-gpui/src/main.rs index ecdc4701164..eba18495cd9 100644 --- a/apps/desktop-gpui/src/main.rs +++ b/apps/desktop-gpui/src/main.rs @@ -439,6 +439,7 @@ fn main() { updates::schedule_startup_check(cx); app_windows::schedule_launch_render_warmup(cx); upload::queue::init(cx); + upload::instant_cleanup::init(cx); // The app menu (and with it ⌘W/⌘M/⌘Q) and the status-bar item. Both // reach into the window registry, so they come after it -- and the menu diff --git a/apps/desktop-gpui/src/recording.rs b/apps/desktop-gpui/src/recording.rs index a1ec646769b..d933ed12302 100644 --- a/apps/desktop-gpui/src/recording.rs +++ b/apps/desktop-gpui/src/recording.rs @@ -96,6 +96,7 @@ pub struct ActiveRecording { start_gate: Option, instant_upload: Option, instant_share_link: Option, + instant_video_id: Option, #[cfg(target_os = "linux")] instant_completion: Option, #[cfg(target_os = "linux")] @@ -820,6 +821,10 @@ impl ActiveRecording { self.instant_share_link.as_deref() } + pub(crate) fn instant_video_id(&self) -> Option<&str> { + self.instant_video_id.as_deref() + } + /// Stop and finalize. Returns the project directory. /// /// Finalization mirrors the CLI's `finalize_completed`: studio projects get @@ -2117,15 +2122,15 @@ async fn start_internal( .map(str::to_string); let pre_created_video = if config.mode == RecordingMode::Instant { - Some( - crate::upload::prepare_instant_upload( - matches!(config.target, ScreenCaptureTarget::CameraOnly), - project_name, - organization_id, - ) - .await - .map_err(anyhow::Error::msg)?, + let video = crate::upload::prepare_instant_upload( + matches!(config.target, ScreenCaptureTarget::CameraOnly), + project_name, + organization_id, ) + .await + .map_err(anyhow::Error::msg)?; + crate::upload::instant_cleanup::begin(&video.id, &project_dir); + Some(video) } else { None }; @@ -2152,12 +2157,20 @@ async fn start_internal( ); } } - if result.is_err() + if let Err(start_error) = &result && let Some(video) = pre_created_video - && let Err(error) = crate::upload::delete_instant_video(&video.id).await { - tracing::error!(video_id = %video.id, "Failed to clean up instant recording: {error}"); - return Err(anyhow!("Failed to clean up instant recording: {error}")); + use crate::upload::instant_cleanup::{self, CleanupReason}; + let cause = instant_cleanup::classify_start_failure(&format!("{start_error:#}")); + let query = instant_cleanup::delete_query(CleanupReason::StartFailed, Some(cause)); + match crate::upload::delete_instant_video_with_query(&video.id, query.as_deref()).await { + Ok(()) => instant_cleanup::resolved(&video.id), + Err(error) => { + instant_cleanup::abandon(&video.id, CleanupReason::StartFailed, Some(cause)); + tracing::error!(video_id = %video.id, "Failed to clean up instant recording: {error}"); + return Err(anyhow!("Failed to clean up instant recording: {error}")); + } + } } result } @@ -2451,6 +2464,9 @@ async fn start_attempt_with_upload( let instant_share_link = instant_upload .as_ref() .map(|upload| upload.video().link.clone()); + let instant_video_id = instant_upload + .as_ref() + .map(|upload| upload.video().id.clone()); let instant_upload = instant_upload.map(|upload| Arc::new(tokio::sync::Mutex::new(Some(upload)))); Ok(ActiveRecording { @@ -2459,6 +2475,7 @@ async fn start_attempt_with_upload( start_gate: config.start_gate.clone(), instant_upload, instant_share_link, + instant_video_id, #[cfg(target_os = "linux")] instant_completion, #[cfg(target_os = "linux")] diff --git a/apps/desktop-gpui/src/session.rs b/apps/desktop-gpui/src/session.rs index 1763a0f74aa..894b61d546c 100644 --- a/apps/desktop-gpui/src/session.rs +++ b/apps/desktop-gpui/src/session.rs @@ -913,6 +913,12 @@ impl RecordingSession { fn activate(&mut self, active: ActiveRecording, cx: &mut Context) { tracing::info!(dir = %active.project_dir.display(), "recording started"); + if let Some(video_id) = active.instant_video_id().map(str::to_owned) { + cx.background_spawn(async move { + crate::upload::instant_cleanup::started(&video_id); + }) + .detach(); + } let project_dir = active.project_dir.clone(); let done = active.done_fut(); self.active = Some(active); @@ -945,7 +951,12 @@ impl RecordingSession { fn discard_stale_start(&mut self, active: ActiveRecording, cx: &mut Context) { let previous = self.discarding.take(); + let video_id = active.instant_video_id().map(str::to_owned); self.discarding = Some(gpui_tokio::Tokio::spawn(cx, async move { + use crate::upload::instant_cleanup::{self, CleanupReason}; + if let Some(video_id) = &video_id { + instant_cleanup::abandon(video_id, CleanupReason::StartCancelled, None); + } if let Some(previous) = previous { let _ = previous.await; } @@ -953,8 +964,15 @@ impl RecordingSession { let discarded = active.cancel_preserving().await; #[cfg(not(target_os = "linux"))] let discarded = active.cancel_and_delete().await; - if let Err(error) = discarded { - tracing::warn!(%error, "discarding a cancelled recording start failed"); + match discarded { + Ok(()) => { + if let Some(video_id) = &video_id { + instant_cleanup::resolved(video_id); + } + } + Err(error) => { + tracing::warn!(%error, "discarding a cancelled recording start failed"); + } } })); } diff --git a/apps/desktop-gpui/src/upload.rs b/apps/desktop-gpui/src/upload.rs index d8f11085955..6bbef37d11d 100644 --- a/apps/desktop-gpui/src/upload.rs +++ b/apps/desktop-gpui/src/upload.rs @@ -15,6 +15,7 @@ use serde_json::{Value, json}; use crate::auth::{self, AuthApiError}; +pub(crate) mod instant_cleanup; #[cfg(test)] mod multipart_tests; pub(crate) mod queue; @@ -492,10 +493,21 @@ impl InstantUpload { } pub async fn delete_instant_video(video_id: &str) -> Result<(), String> { - let path = format!( + delete_instant_video_with_query(video_id, None).await +} + +pub(crate) async fn delete_instant_video_with_query( + video_id: &str, + query: Option<&str>, +) -> Result<(), String> { + let mut path = format!( "/api/desktop/video/delete?videoId={}", urlencoding(video_id) ); + if let Some(query) = query { + path.push('&'); + path.push_str(query); + } let response = auth::authed_request(reqwest::Method::DELETE, &path, None) .await .map_err(|error| format!("Failed to delete instant recording: {error}"))?; diff --git a/apps/desktop-gpui/src/upload/instant_cleanup.rs b/apps/desktop-gpui/src/upload/instant_cleanup.rs new file mode 100644 index 00000000000..f4c22cc8542 --- /dev/null +++ b/apps/desktop-gpui/src/upload/instant_cleanup.rs @@ -0,0 +1,1136 @@ +use super::*; +use cap_project::{InstantRecordingMeta, RecordingMetaInner}; +use std::io::Write; + +const JOURNAL_FILE: &str = "gpui-instant-cleanup.json"; +const MAX_JOURNAL_BYTES: u64 = 256 * 1024; +const MAX_ENTRIES: usize = 256; +const ENTRY_LIFETIME_SECS: i64 = 14 * 24 * 60 * 60; +const RETRY_BASE_SECS: i64 = 5 * 60; +const RETRY_MAX_SECS: i64 = 6 * 60 * 60; +const INTERRUPTED_GRACE_SECS: i64 = 30 * 60; +const SWEEP_INTERVAL: Duration = Duration::from_secs(5 * 60); +const MEDIA_SCAN_DEPTH: usize = 3; + +static JOURNAL_LOCK: Mutex<()> = Mutex::new(()); +static SESSION: OnceLock = OnceLock::new(); + +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "kebab-case")] +pub(crate) enum CleanupReason { + StartFailed, + StartCancelled, + Interrupted, +} + +impl CleanupReason { + fn as_str(self) -> &'static str { + match self { + Self::StartFailed => "start-failed", + Self::StartCancelled => "start-cancelled", + Self::Interrupted => "interrupted", + } + } +} + +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "kebab-case")] +pub(crate) enum StartFailureCause { + Permission, + Microphone, + Camera, + Capture, + Storage, + Account, + Network, + Other, +} + +impl StartFailureCause { + fn as_str(self) -> &'static str { + match self { + Self::Permission => "permission", + Self::Microphone => "microphone", + Self::Camera => "camera", + Self::Capture => "capture", + Self::Storage => "storage", + Self::Account => "account", + Self::Network => "network", + Self::Other => "other", + } + } +} + +pub(crate) fn classify_start_failure(error: &str) -> StartFailureCause { + let error = error.to_ascii_lowercase(); + let has = |needles: &[&str]| needles.iter().any(|needle| error.contains(needle)); + if has(&["permission", "not authorized", "not permitted", "tcc"]) { + StartFailureCause::Permission + } else if has(&["microphone", "mic feed", "audio input"]) { + StartFailureCause::Microphone + } else if has(&["camera"]) { + StartFailureCause::Camera + } else if has(&["sign in", "authenticat", "session has expired"]) { + StartFailureCause::Account + } else if has(&["storage", "disk", "no space"]) { + StartFailureCause::Storage + } else if has(&["timed out", "timeout", "network", "connection", "dns"]) { + StartFailureCause::Network + } else if has(&["screen", "capture", "display", "window", "shareable"]) { + StartFailureCause::Capture + } else { + StartFailureCause::Other + } +} + +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "kebab-case")] +enum Stage { + Starting, + Abandoned, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[serde(deny_unknown_fields)] +struct Entry { + video_id: String, + server_url: String, + owner_id: String, + session: String, + stage: Stage, + reason: Option, + cause: Option, + created_at: i64, + attempts: u32, + next_attempt_at: i64, + #[serde(default)] + project_dir: Option, +} + +impl Entry { + fn effective_reason(&self) -> CleanupReason { + match self.stage { + Stage::Abandoned => self.reason.unwrap_or(CleanupReason::Interrupted), + Stage::Starting => CleanupReason::Interrupted, + } + } + + fn due(&self, session: &str, now: i64) -> bool { + let actionable = match self.stage { + Stage::Abandoned => true, + Stage::Starting => { + self.session != session + && now.saturating_sub(self.created_at) >= INTERRUPTED_GRACE_SECS + } + }; + actionable && self.next_attempt_at <= now + } + + fn expired(&self, now: i64) -> bool { + now.saturating_sub(self.created_at) > ENTRY_LIFETIME_SECS + } +} + +#[derive(Debug, Default, Serialize, Deserialize)] +struct Journal { + version: u32, + entries: Vec, +} + +fn now() -> i64 { + chrono::Utc::now().timestamp() +} + +fn session_id() -> &'static str { + SESSION.get_or_init(crate::store::new_uuid_v4) +} + +fn journal_path() -> PathBuf { + crate::store::app_data_dir().join(JOURNAL_FILE) +} + +fn current_owner_id() -> Option { + crate::store::store_section("auth") + .get("user_id") + .and_then(Value::as_str) + .filter(|id| !id.is_empty()) + .map(str::to_string) +} + +fn retry_delay(attempts: u32) -> i64 { + RETRY_BASE_SECS + .saturating_mul(1_i64 << attempts.min(12)) + .min(RETRY_MAX_SECS) +} + +fn read_journal(path: &Path) -> Journal { + let bytes = match std::fs::symlink_metadata(path) { + Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Journal::default(), + Err(error) => { + tracing::warn!(%error, "Instant cleanup journal is unreadable"); + return Journal::default(); + } + Ok(metadata) if !metadata.is_file() || metadata.len() > MAX_JOURNAL_BYTES => { + tracing::warn!("Instant cleanup journal is not a usable file"); + return Journal::default(); + } + Ok(_) => match std::fs::read(path) { + Ok(bytes) => bytes, + Err(error) => { + tracing::warn!(%error, "Instant cleanup journal is unreadable"); + return Journal::default(); + } + }, + }; + match serde_json::from_slice::(&bytes) { + Ok(journal) if journal.version == 1 => journal, + Ok(_) => Journal::default(), + Err(error) => { + tracing::warn!(%error, "Instant cleanup journal could not be parsed"); + Journal::default() + } + } +} + +fn write_journal(path: &Path, journal: &Journal) -> Result<(), String> { + let directory = path + .parent() + .ok_or("Instant cleanup journal has no directory")?; + std::fs::create_dir_all(directory).map_err(|error| error.to_string())?; + let bytes = serde_json::to_vec(journal).map_err(|error| error.to_string())?; + let temporary = directory.join(format!( + ".gpui-instant-cleanup-{}.tmp", + crate::store::new_uuid_v4() + )); + let result = (|| { + let mut file = std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(&temporary)?; + file.write_all(&bytes)?; + drop(file); + std::fs::rename(&temporary, path) + })(); + if result.is_err() { + let _ = std::fs::remove_file(&temporary); + } + result.map_err(|error| format!("Could not save the instant cleanup journal: {error}")) +} + +fn update_journal(path: &Path, change: impl FnOnce(&mut Vec)) -> Result<(), String> { + let _guard = JOURNAL_LOCK + .lock() + .unwrap_or_else(|error| error.into_inner()); + let mut journal = read_journal(path); + journal.version = 1; + change(&mut journal.entries); + if journal.entries.len() > MAX_ENTRIES { + journal.entries.sort_by_key(|entry| entry.created_at); + let excess = journal.entries.len() - MAX_ENTRIES; + journal.entries.drain(..excess); + } + write_journal(path, &journal) +} + +fn begin_at( + path: &Path, + video_id: &str, + project_dir: Option, + server_url: String, + owner_id: String, + now: i64, +) { + let entry = Entry { + video_id: video_id.to_string(), + server_url, + owner_id, + session: session_id().to_string(), + stage: Stage::Starting, + reason: None, + cause: None, + created_at: now, + attempts: 0, + next_attempt_at: now, + project_dir, + }; + if let Err(error) = update_journal(path, |entries| { + entries.retain(|existing| existing.video_id != video_id); + entries.push(entry); + }) { + tracing::warn!(%error, video_id, "Instant cleanup could not record a starting recording"); + } +} + +fn remove_at(path: &Path, video_id: &str) { + if let Err(error) = update_journal(path, |entries| { + entries.retain(|entry| entry.video_id != video_id); + }) { + tracing::warn!(%error, video_id, "Instant cleanup could not clear a journal entry"); + } +} + +fn abandon_at( + path: &Path, + video_id: &str, + reason: CleanupReason, + cause: Option, + fallback_identity: Option<(String, String)>, + now: i64, +) { + if let Err(error) = update_journal(path, |entries| { + if let Some(entry) = entries.iter_mut().find(|entry| entry.video_id == video_id) { + entry.stage = Stage::Abandoned; + entry.reason = Some(reason); + entry.cause = cause; + entry.next_attempt_at = now.saturating_add(RETRY_BASE_SECS); + } else if let Some((server_url, owner_id)) = fallback_identity { + entries.push(Entry { + video_id: video_id.to_string(), + server_url, + owner_id, + session: session_id().to_string(), + stage: Stage::Abandoned, + reason: Some(reason), + cause, + created_at: now, + attempts: 0, + next_attempt_at: now.saturating_add(RETRY_BASE_SECS), + project_dir: None, + }); + } + }) { + tracing::warn!(%error, video_id, "Instant cleanup could not record an abandoned recording"); + } +} + +/// Records a pre-created Instant video before capture starts, so a crash, +/// panic or aborted discard during startup still gets its server copy +/// deleted on the next sweep. Linux keeps its own startup cleanup contract. +pub(crate) fn begin(video_id: &str, project_dir: &Path) { + if cfg!(target_os = "linux") { + return; + } + let Some(owner_id) = current_owner_id() else { + return; + }; + begin_at( + &journal_path(), + video_id, + Some(project_dir.to_path_buf()), + auth::server_url(), + owner_id, + now(), + ); +} + +pub(crate) fn started(video_id: &str) { + if cfg!(target_os = "linux") { + return; + } + remove_at(&journal_path(), video_id); +} + +pub(crate) fn resolved(video_id: &str) { + if cfg!(target_os = "linux") { + return; + } + remove_at(&journal_path(), video_id); +} + +pub(crate) fn abandon(video_id: &str, reason: CleanupReason, cause: Option) { + if cfg!(target_os = "linux") { + return; + } + let identity = current_owner_id().map(|owner| (auth::server_url(), owner)); + abandon_at(&journal_path(), video_id, reason, cause, identity, now()); +} + +pub(crate) fn delete_query( + reason: CleanupReason, + cause: Option, +) -> Option { + if !crate::store::GeneralSettings::load().enable_telemetry { + return None; + } + Some(match cause { + Some(cause) => format!("reason={}&cause={}", reason.as_str(), cause.as_str()), + None => format!("reason={}", reason.as_str()), + }) +} + +fn upload_meta_video_id(meta: &RecordingMeta) -> Option<&str> { + match meta.upload.as_ref()? { + UploadMeta::MultipartUpload { video_id, .. } + | UploadMeta::SinglePartUpload { video_id, .. } + | UploadMeta::SegmentUpload { video_id, .. } => Some(video_id), + UploadMeta::Failed { .. } | UploadMeta::Complete => None, + } +} + +fn has_recorded_media(directory: &Path, depth: usize) -> bool { + let Ok(entries) = std::fs::read_dir(directory) else { + return false; + }; + entries.flatten().any(|entry| { + let path = entry.path(); + let Ok(metadata) = std::fs::symlink_metadata(&path) else { + return false; + }; + if metadata.is_dir() { + return depth > 0 && has_recorded_media(&path, depth - 1); + } + if !metadata.is_file() || metadata.len() == 0 { + return false; + } + let name = path + .file_name() + .and_then(|name| name.to_str()) + .unwrap_or_default() + .to_ascii_lowercase(); + name.ends_with(".m4s") || (name.ends_with(".mp4") && !name.starts_with("init")) + }) +} + +fn project_needs_video(project: &Path, meta: Option<&RecordingMeta>, interrupted: bool) -> bool { + let Some(meta) = meta else { + return true; + }; + !matches!( + meta.inner, + RecordingMetaInner::Instant(InstantRecordingMeta::InProgress { .. }) + ) || (interrupted && has_recorded_media(&project.join("content"), MEDIA_SCAN_DEPTH)) +} + +/// True when a local recording that must not lose its server copy points at +/// this video: anything past `InProgress`, an unreadable project that claims +/// the id, or (for interrupted starts) a project that already holds media. +/// An interrupted start whose own folder sits on storage that cannot be read +/// right now (an unmounted drive) is kept too: its media may still be there. +fn locally_protected( + recording_dirs: &[PathBuf], + video_id: &str, + interrupted: bool, + start_project: Option<&Path>, +) -> bool { + if interrupted && let Some(project) = start_project { + if !project + .parent() + .is_some_and(|parent| std::fs::read_dir(parent).is_ok()) + { + return true; + } + if project.exists() { + let meta = RecordingMeta::load_for_project(project).ok(); + if project_needs_video(project, meta.as_ref(), interrupted) { + return true; + } + } + } + for directory in recording_dirs { + let Ok(entries) = std::fs::read_dir(directory) else { + continue; + }; + for entry in entries.flatten() { + let project = entry.path(); + let state_matches = matches!( + queue::read_state(&project), + Ok(Some(state)) if state.video_id() == video_id + ); + let meta = RecordingMeta::load_for_project(&project).ok(); + let meta_matches = meta.as_ref().is_some_and(|meta| { + meta.sharing + .as_ref() + .is_some_and(|share| share.id == video_id) + || upload_meta_video_id(meta) == Some(video_id) + }); + if (state_matches || meta_matches) + && project_needs_video(&project, meta.as_ref(), interrupted) + { + return true; + } + } + } + false +} + +trait SweepBackend { + fn account_ready(&self, entry: &Entry) -> bool; + fn protected(&self, entry: &Entry) -> impl Future + Send; + fn delete(&self, entry: &Entry) -> impl Future> + Send; +} + +#[derive(Debug, Default, PartialEq)] +struct SweepSummary { + deleted: usize, + kept: usize, + expired: usize, + failed: usize, +} + +async fn sweep(path: &Path, session: &str, now: i64, backend: &impl SweepBackend) -> SweepSummary { + let mut summary = SweepSummary::default(); + let due: Vec = { + let _guard = JOURNAL_LOCK + .lock() + .unwrap_or_else(|error| error.into_inner()); + read_journal(path) + .entries + .into_iter() + .filter(|entry| entry.expired(now) || entry.due(session, now)) + .collect() + }; + let mut remove = Vec::new(); + let mut retry = Vec::new(); + for entry in due { + if entry.expired(now) { + tracing::warn!(video_id = %entry.video_id, "Instant cleanup gave up on an old entry"); + summary.expired += 1; + remove.push(entry.video_id); + continue; + } + if !backend.account_ready(&entry) { + continue; + } + if backend.protected(&entry).await { + tracing::warn!(video_id = %entry.video_id, "Instant cleanup kept a video a local recording uses"); + summary.kept += 1; + remove.push(entry.video_id); + continue; + } + match backend.delete(&entry).await { + Ok(()) => { + summary.deleted += 1; + remove.push(entry.video_id); + } + Err(error) => { + tracing::warn!(%error, video_id = %entry.video_id, "Instant cleanup delete will retry"); + summary.failed += 1; + retry.push(entry.video_id); + } + } + } + if remove.is_empty() && retry.is_empty() { + return summary; + } + if let Err(error) = update_journal(path, |entries| { + entries.retain(|entry| !remove.contains(&entry.video_id)); + for entry in entries.iter_mut() { + if retry.contains(&entry.video_id) { + entry.attempts = entry.attempts.saturating_add(1); + entry.next_attempt_at = now.saturating_add(retry_delay(entry.attempts)); + } + } + }) { + tracing::warn!(%error, "Instant cleanup could not save sweep results"); + } + summary +} + +struct LiveSweep; + +impl SweepBackend for LiveSweep { + fn account_ready(&self, entry: &Entry) -> bool { + auth::UploadRequestContext::for_upload(&entry.server_url, Some(&entry.owner_id)).is_ok() + } + + async fn protected(&self, entry: &Entry) -> bool { + let video_id = entry.video_id.clone(); + let interrupted = entry.effective_reason() == CleanupReason::Interrupted; + let start_project = entry.project_dir.clone(); + tokio::task::spawn_blocking(move || { + locally_protected( + &crate::library::known_recordings_dirs(), + &video_id, + interrupted, + start_project.as_deref(), + ) + }) + .await + .unwrap_or(true) + } + + async fn delete(&self, entry: &Entry) -> Result<(), String> { + let context = + auth::UploadRequestContext::for_upload(&entry.server_url, Some(&entry.owner_id)) + .map_err(|error| error.to_string())?; + let query = delete_query(entry.effective_reason(), entry.cause); + context + .run(delete_instant_video_with_query( + &entry.video_id, + query.as_deref(), + )) + .await + } +} + +async fn run() { + loop { + let summary = sweep(&journal_path(), session_id(), now(), &LiveSweep).await; + if summary != SweepSummary::default() { + tracing::info!(?summary, "Instant cleanup sweep finished"); + } + tokio::time::sleep(SWEEP_INTERVAL).await; + } +} + +pub(crate) fn init(cx: &mut gpui::App) { + if cfg!(target_os = "linux") { + return; + } + session_id(); + gpui_tokio::Tokio::spawn(cx, run()).detach(); +} + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::atomic::AtomicUsize; + + struct Fixture(tempfile::TempDir); + + impl Fixture { + fn new() -> Self { + Self(tempfile::tempdir().unwrap()) + } + + fn journal(&self) -> PathBuf { + self.0.path().join("state").join(JOURNAL_FILE) + } + + fn recordings(&self) -> PathBuf { + let path = self.0.path().join("recordings"); + std::fs::create_dir_all(&path).unwrap(); + path + } + + fn entries(&self) -> Vec { + read_journal(&self.journal()).entries + } + + fn seed(&self, entries: Vec) { + write_journal( + &self.journal(), + &Journal { + version: 1, + entries, + }, + ) + .unwrap(); + } + } + + fn entry(video_id: &str, session: &str, stage: Stage, created_at: i64) -> Entry { + Entry { + video_id: video_id.into(), + server_url: "https://cap.test".into(), + owner_id: "owner".into(), + session: session.into(), + stage, + reason: (stage == Stage::Abandoned).then_some(CleanupReason::StartFailed), + cause: None, + created_at, + attempts: 0, + next_attempt_at: created_at, + project_dir: None, + } + } + + #[derive(Default)] + struct FakeBackend { + account: bool, + protected: Vec, + failing: Vec, + deletes: Mutex>, + protected_checks: AtomicUsize, + } + + impl FakeBackend { + fn ready() -> Self { + Self { + account: true, + ..Self::default() + } + } + + fn deleted(&self) -> Vec { + self.deletes.lock().unwrap().clone() + } + } + + impl SweepBackend for FakeBackend { + fn account_ready(&self, _entry: &Entry) -> bool { + self.account + } + + async fn protected(&self, entry: &Entry) -> bool { + self.protected_checks.fetch_add(1, Ordering::SeqCst); + self.protected.contains(&entry.video_id) + } + + async fn delete(&self, entry: &Entry) -> Result<(), String> { + if self.failing.contains(&entry.video_id) { + return Err("server unavailable".into()); + } + self.deletes.lock().unwrap().push(entry.video_id.clone()); + Ok(()) + } + } + + #[test] + fn start_failures_are_classified_without_their_text() { + let cases = [ + ( + "Screen recording permission was denied", + StartFailureCause::Permission, + ), + ( + "Selected microphone 'Desk Mic' is not the connected microphone 'Other'. Please select it again.", + StartFailureCause::Microphone, + ), + ("Camera feed did not start", StartFailureCause::Camera), + ( + "Your session has expired. Please sign in again", + StartFailureCause::Account, + ), + ( + "Low storage: only 0.20 GB is available", + StartFailureCause::Storage, + ), + ("operation timed out", StartFailureCause::Network), + ("Failed to start screen capture", StartFailureCause::Capture), + ("something unexpected", StartFailureCause::Other), + ]; + for (error, cause) in cases { + assert_eq!(classify_start_failure(error), cause, "{error}"); + } + } + + #[test] + fn a_started_recording_leaves_nothing_to_clean() { + let fixture = Fixture::new(); + begin_at( + &fixture.journal(), + "video", + None, + "https://cap.test".into(), + "owner".into(), + 100, + ); + assert_eq!(fixture.entries().len(), 1); + assert_eq!(fixture.entries()[0].stage, Stage::Starting); + remove_at(&fixture.journal(), "video"); + assert!(fixture.entries().is_empty()); + } + + #[test] + fn abandoning_marks_the_entry_and_waits_before_the_first_retry() { + let fixture = Fixture::new(); + begin_at( + &fixture.journal(), + "video", + None, + "https://cap.test".into(), + "owner".into(), + 100, + ); + abandon_at( + &fixture.journal(), + "video", + CleanupReason::StartFailed, + Some(StartFailureCause::Microphone), + None, + 200, + ); + let entries = fixture.entries(); + assert_eq!(entries.len(), 1); + assert_eq!(entries[0].stage, Stage::Abandoned); + assert_eq!(entries[0].reason, Some(CleanupReason::StartFailed)); + assert_eq!(entries[0].cause, Some(StartFailureCause::Microphone)); + assert_eq!(entries[0].next_attempt_at, 200 + RETRY_BASE_SECS); + } + + #[test] + fn abandoning_without_a_journal_entry_needs_an_identity() { + let fixture = Fixture::new(); + abandon_at( + &fixture.journal(), + "video", + CleanupReason::StartCancelled, + None, + None, + 100, + ); + assert!(fixture.entries().is_empty()); + abandon_at( + &fixture.journal(), + "video", + CleanupReason::StartCancelled, + None, + Some(("https://cap.test".into(), "owner".into())), + 100, + ); + assert_eq!(fixture.entries().len(), 1); + assert_eq!(fixture.entries()[0].stage, Stage::Abandoned); + } + + #[test] + fn corrupt_or_oversized_journals_are_treated_as_empty() { + let fixture = Fixture::new(); + std::fs::create_dir_all(fixture.journal().parent().unwrap()).unwrap(); + std::fs::write(fixture.journal(), b"{not json").unwrap(); + assert!(fixture.entries().is_empty()); + std::fs::write( + fixture.journal(), + vec![b' '; MAX_JOURNAL_BYTES as usize + 1], + ) + .unwrap(); + assert!(fixture.entries().is_empty()); + begin_at( + &fixture.journal(), + "video", + None, + "https://cap.test".into(), + "owner".into(), + 100, + ); + assert_eq!(fixture.entries().len(), 1); + } + + #[test] + fn the_journal_keeps_only_the_newest_entries() { + let fixture = Fixture::new(); + fixture.seed( + (0..MAX_ENTRIES as i64) + .map(|index| entry(&format!("video-{index}"), "old", Stage::Starting, index)) + .collect(), + ); + begin_at( + &fixture.journal(), + "newest", + None, + "https://cap.test".into(), + "owner".into(), + 10_000, + ); + let entries = fixture.entries(); + assert_eq!(entries.len(), MAX_ENTRIES); + assert!(entries.iter().all(|entry| entry.video_id != "video-0")); + assert!(entries.iter().any(|entry| entry.video_id == "newest")); + } + + #[tokio::test] + async fn a_start_still_running_in_this_session_is_never_deleted() { + let fixture = Fixture::new(); + fixture.seed(vec![entry("in-flight", "current", Stage::Starting, 100)]); + let backend = FakeBackend::ready(); + let summary = sweep(&fixture.journal(), "current", 100 + 30 * 24 * 60, &backend).await; + assert_eq!(summary, SweepSummary::default()); + assert!(backend.deleted().is_empty()); + assert_eq!(fixture.entries().len(), 1); + } + + #[tokio::test] + async fn abandoned_and_interrupted_starts_are_deleted_once_due() { + let fixture = Fixture::new(); + let mut not_yet_due = entry("later", "current", Stage::Abandoned, 100); + not_yet_due.next_attempt_at = 100 + INTERRUPTED_GRACE_SECS + 1; + fixture.seed(vec![ + entry("abandoned", "current", Stage::Abandoned, 100), + entry("crashed", "previous", Stage::Starting, 100), + not_yet_due, + ]); + let backend = FakeBackend::ready(); + let now = 100 + INTERRUPTED_GRACE_SECS; + let summary = sweep(&fixture.journal(), "current", now, &backend).await; + assert_eq!(summary.deleted, 2); + assert_eq!( + backend.deleted(), + vec!["abandoned".to_string(), "crashed".to_string()] + ); + let remaining = fixture.entries(); + assert_eq!(remaining.len(), 1); + assert_eq!(remaining[0].video_id, "later"); + } + + #[tokio::test] + async fn another_running_app_gets_time_to_finish_its_start() { + let fixture = Fixture::new(); + fixture.seed(vec![entry("other-app", "other", Stage::Starting, 100)]); + let backend = FakeBackend::ready(); + let summary = sweep( + &fixture.journal(), + "current", + 100 + INTERRUPTED_GRACE_SECS - 1, + &backend, + ) + .await; + assert_eq!(summary, SweepSummary::default()); + assert!(backend.deleted().is_empty()); + assert_eq!(fixture.entries().len(), 1); + } + + #[tokio::test] + async fn another_account_leaves_the_entry_untouched() { + let fixture = Fixture::new(); + fixture.seed(vec![entry("video", "previous", Stage::Starting, 100)]); + let backend = FakeBackend::default(); + let summary = sweep( + &fixture.journal(), + "current", + 100 + INTERRUPTED_GRACE_SECS, + &backend, + ) + .await; + assert_eq!(summary, SweepSummary::default()); + assert_eq!(backend.protected_checks.load(Ordering::SeqCst), 0); + assert!(backend.deleted().is_empty()); + assert_eq!(fixture.entries().len(), 1); + } + + #[tokio::test] + async fn a_protected_video_is_forgotten_without_deleting_it() { + let fixture = Fixture::new(); + fixture.seed(vec![entry("kept", "previous", Stage::Starting, 100)]); + let backend = FakeBackend { + protected: vec!["kept".into()], + ..FakeBackend::ready() + }; + let summary = sweep( + &fixture.journal(), + "current", + 100 + INTERRUPTED_GRACE_SECS, + &backend, + ) + .await; + assert_eq!(summary.kept, 1); + assert!(backend.deleted().is_empty()); + assert!(fixture.entries().is_empty()); + } + + #[tokio::test] + async fn failed_deletes_back_off_and_old_entries_expire() { + let fixture = Fixture::new(); + let now = ENTRY_LIFETIME_SECS + 1_000; + fixture.seed(vec![ + entry("flaky", "current", Stage::Abandoned, now - 10), + entry("ancient", "previous", Stage::Starting, 0), + ]); + let backend = FakeBackend { + failing: vec!["flaky".into()], + ..FakeBackend::ready() + }; + let summary = sweep(&fixture.journal(), "current", now, &backend).await; + assert_eq!(summary.failed, 1); + assert_eq!(summary.expired, 1); + let remaining = fixture.entries(); + assert_eq!(remaining.len(), 1); + assert_eq!(remaining[0].attempts, 1); + assert_eq!(remaining[0].next_attempt_at, now + retry_delay(1)); + assert!(retry_delay(40) <= RETRY_MAX_SECS); + } + + #[tokio::test] + async fn entries_cleared_during_a_sweep_are_not_brought_back() { + let fixture = Fixture::new(); + fixture.seed(vec![entry("flaky", "current", Stage::Abandoned, 100)]); + struct ClearsDuringDelete<'a>(&'a Path); + impl SweepBackend for ClearsDuringDelete<'_> { + fn account_ready(&self, _entry: &Entry) -> bool { + true + } + async fn protected(&self, _entry: &Entry) -> bool { + false + } + async fn delete(&self, entry: &Entry) -> Result<(), String> { + remove_at(self.0, &entry.video_id); + Err("server unavailable".into()) + } + } + let journal = fixture.journal(); + sweep(&journal, "current", 500, &ClearsDuringDelete(&journal)).await; + assert!(fixture.entries().is_empty()); + } + + fn write_project( + root: &Path, + name: &str, + video_id: &str, + inner: RecordingMetaInner, + ) -> PathBuf { + let project = root.join(name); + std::fs::create_dir_all(project.join("content/display")).unwrap(); + let video = VideoUploadInfo { + id: video_id.into(), + link: format!("https://cap.test/s/{video_id}"), + config: S3UploadMeta { + id: video_id.into(), + }, + }; + RecordingMeta { + platform: None, + project_path: project.clone(), + pretty_name: name.into(), + sharing: None, + inner, + upload: Some(UploadMeta::SegmentUpload { + video_id: video_id.into(), + pre_created_video: video, + recording_dir: project.clone(), + }), + } + .save_for_project() + .unwrap(); + project + } + + fn in_progress() -> RecordingMetaInner { + RecordingMetaInner::Instant(InstantRecordingMeta::InProgress { recording: true }) + } + + #[test] + fn a_finished_local_recording_always_protects_its_video() { + let fixture = Fixture::new(); + let recordings = fixture.recordings(); + write_project( + &recordings, + "done.cap", + "video", + RecordingMetaInner::Instant(InstantRecordingMeta::Complete { + fps: 30, + sample_rate: None, + }), + ); + assert!(locally_protected( + std::slice::from_ref(&recordings), + "video", + false, + None, + )); + assert!(locally_protected(&[recordings], "video", true, None)); + } + + #[test] + fn a_failed_start_without_media_does_not_protect_its_video() { + let fixture = Fixture::new(); + let recordings = fixture.recordings(); + let project = write_project(&recordings, "failed.cap", "video", in_progress()); + std::fs::write(project.join("content/display/init.mp4"), b"header").unwrap(); + assert!(!locally_protected( + std::slice::from_ref(&recordings), + "video", + false, + None, + )); + assert!(!locally_protected( + std::slice::from_ref(&recordings), + "video", + true, + None, + )); + assert!(!locally_protected(&[recordings], "other-video", true, None)); + } + + #[test] + fn recorded_media_protects_an_interrupted_start_only() { + let fixture = Fixture::new(); + let recordings = fixture.recordings(); + let project = write_project(&recordings, "crashed.cap", "video", in_progress()); + std::fs::write(project.join("content/display/segment_001.m4s"), b"media").unwrap(); + assert!(locally_protected( + std::slice::from_ref(&recordings), + "video", + true, + None, + )); + assert!(!locally_protected(&[recordings], "video", false, None)); + } + + #[test] + fn an_unreadable_project_that_claims_the_video_protects_it() { + let fixture = Fixture::new(); + let recordings = fixture.recordings(); + let project = write_project(&recordings, "broken.cap", "video", in_progress()); + queue::record_capture( + &project, + &VideoUploadInfo { + id: "video".into(), + link: "https://cap.test/s/video".into(), + config: S3UploadMeta { id: "video".into() }, + }, + true, + true, + ) + .unwrap(); + std::fs::write(project.join("recording-meta.json"), b"{broken").unwrap(); + assert!(locally_protected(&[recordings], "video", false, None)); + } + + #[test] + fn an_interrupted_start_on_unreadable_storage_is_kept() { + let fixture = Fixture::new(); + let recordings = fixture.recordings(); + let unmounted = fixture.0.path().join("unmounted-drive").join("start.cap"); + assert!(locally_protected( + std::slice::from_ref(&recordings), + "video", + true, + Some(&unmounted), + )); + assert!(!locally_protected( + std::slice::from_ref(&recordings), + "video", + false, + Some(&unmounted), + )); + } + + #[test] + fn an_interrupted_start_folder_is_checked_even_without_a_reference() { + let fixture = Fixture::new(); + let recordings = fixture.recordings(); + let project = recordings.join("orphan.cap"); + std::fs::create_dir_all(project.join("content/display")).unwrap(); + assert!(locally_protected( + std::slice::from_ref(&recordings), + "video", + true, + Some(&project), + )); + let deleted = recordings.join("deleted.cap"); + assert!(!locally_protected( + std::slice::from_ref(&recordings), + "video", + true, + Some(&deleted), + )); + let failed = write_project(&recordings, "failed.cap", "other", in_progress()); + assert!(!locally_protected( + std::slice::from_ref(&recordings), + "video", + true, + Some(&failed), + )); + std::fs::write(failed.join("content/display/segment_001.m4s"), b"media").unwrap(); + assert!(locally_protected( + &[recordings], + "video", + true, + Some(&failed) + )); + } + + #[test] + fn begin_records_the_start_folder() { + let fixture = Fixture::new(); + let project = fixture.recordings().join("start.cap"); + begin_at( + &fixture.journal(), + "video", + Some(project.clone()), + "https://cap.test".into(), + "owner".into(), + 100, + ); + assert_eq!(fixture.entries()[0].project_dir, Some(project)); + } +} diff --git a/apps/desktop-gpui/src/upload/queue.rs b/apps/desktop-gpui/src/upload/queue.rs index c32fe8c917a..41d726c2e3b 100644 --- a/apps/desktop-gpui/src/upload/queue.rs +++ b/apps/desktop-gpui/src/upload/queue.rs @@ -55,6 +55,10 @@ pub(crate) struct UploadState { } impl UploadState { + pub(crate) fn video_id(&self) -> &str { + &self.video_id + } + pub(super) fn request_context(&self) -> Result { auth::UploadRequestContext::for_upload(&self.server_url, self.owner_id.as_deref()) .map_err(|error| error.to_string()) From e9e5dc5247dac1056a478969aeac584e19f3aeae Mon Sep 17 00:00:00 2001 From: Richie McIlroy <33632126+richiemcilroy@users.noreply.github.com> Date: Sat, 24 Oct 2026 12:35:48 +0100 Subject: [PATCH 3/9] fix(desktop-gpui): keep an interrupted start's video when its media cannot be read --- .../src/upload/instant_cleanup.rs | 35 ++++++++++++++++--- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/apps/desktop-gpui/src/upload/instant_cleanup.rs b/apps/desktop-gpui/src/upload/instant_cleanup.rs index f4c22cc8542..2b209630cb0 100644 --- a/apps/desktop-gpui/src/upload/instant_cleanup.rs +++ b/apps/desktop-gpui/src/upload/instant_cleanup.rs @@ -367,14 +367,21 @@ fn upload_meta_video_id(meta: &RecordingMeta) -> Option<&str> { } } +/// True when the folder holds recorded media, or when part of it cannot be +/// read: media that cannot be checked counts as present. Only a missing folder +/// counts as empty. fn has_recorded_media(directory: &Path, depth: usize) -> bool { - let Ok(entries) = std::fs::read_dir(directory) else { - return false; + let entries = match std::fs::read_dir(directory) { + Ok(entries) => entries, + Err(error) => return error.kind() != std::io::ErrorKind::NotFound, }; - entries.flatten().any(|entry| { + entries.into_iter().any(|entry| { + let Ok(entry) = entry else { + return true; + }; let path = entry.path(); let Ok(metadata) = std::fs::symlink_metadata(&path) else { - return false; + return true; }; if metadata.is_dir() { return depth > 0 && has_recorded_media(&path, depth - 1); @@ -1045,6 +1052,26 @@ mod tests { assert!(!locally_protected(&[recordings], "video", false, None)); } + #[cfg(unix)] + #[test] + fn media_that_cannot_be_read_protects_an_interrupted_start() { + use std::os::unix::fs::PermissionsExt; + let fixture = Fixture::new(); + let recordings = fixture.recordings(); + let project = write_project(&recordings, "locked.cap", "video", in_progress()); + let display = project.join("content/display"); + std::fs::set_permissions(&display, std::fs::Permissions::from_mode(0o000)).unwrap(); + let readable = std::fs::read_dir(&display).is_ok(); + let protected = locally_protected(std::slice::from_ref(&recordings), "video", true, None); + let normal = locally_protected(&[recordings], "video", false, None); + std::fs::set_permissions(&display, std::fs::Permissions::from_mode(0o755)).unwrap(); + if readable { + return; + } + assert!(protected); + assert!(!normal); + } + #[test] fn an_unreadable_project_that_claims_the_video_protects_it() { let fixture = Fixture::new(); From 9793ad9a6bf32d6209128909abf5b75a4c11862a Mon Sep 17 00:00:00 2001 From: Richie McIlroy <33632126+richiemcilroy@users.noreply.github.com> Date: Sat, 24 Oct 2026 12:35:48 +0100 Subject: [PATCH 4/9] fix(desktop-gpui): report start-cancelled when a superseded start is discarded --- apps/desktop-gpui/src/recording.rs | 17 ++++++++++++++++- apps/desktop-gpui/src/session.rs | 2 +- apps/desktop-gpui/src/upload.rs | 9 +++------ 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/apps/desktop-gpui/src/recording.rs b/apps/desktop-gpui/src/recording.rs index d933ed12302..7c91d53e270 100644 --- a/apps/desktop-gpui/src/recording.rs +++ b/apps/desktop-gpui/src/recording.rs @@ -1123,6 +1123,21 @@ impl ActiveRecording { /// delete and restart flows. Deleting a directory this app just created is /// app behavior, same as the Tauri delete button. pub async fn cancel_and_delete(self) -> anyhow::Result<()> { + self.cancel_and_delete_with_query(None).await + } + + /// `cancel_and_delete` for a start that was superseded before it began, + /// telling the server why its Instant video is deleted. + #[cfg(not(target_os = "linux"))] + pub(crate) async fn discard_cancelled_start(self) -> anyhow::Result<()> { + let query = crate::upload::instant_cleanup::delete_query( + crate::upload::instant_cleanup::CleanupReason::StartCancelled, + None, + ); + self.cancel_and_delete_with_query(query).await + } + + async fn cancel_and_delete_with_query(self, query: Option) -> anyhow::Result<()> { if let Some(gate) = &self.start_gate { gate.arm(); } @@ -1175,7 +1190,7 @@ impl ActiveRecording { None => None, }; let remote_result = if let Some(upload) = upload.as_mut() { - upload.cancel().await + upload.cancel(query.as_deref()).await } else { Ok(()) }; diff --git a/apps/desktop-gpui/src/session.rs b/apps/desktop-gpui/src/session.rs index 894b61d546c..b964bbead50 100644 --- a/apps/desktop-gpui/src/session.rs +++ b/apps/desktop-gpui/src/session.rs @@ -963,7 +963,7 @@ impl RecordingSession { #[cfg(target_os = "linux")] let discarded = active.cancel_preserving().await; #[cfg(not(target_os = "linux"))] - let discarded = active.cancel_and_delete().await; + let discarded = active.discard_cancelled_start().await; match discarded { Ok(()) => { if let Some(video_id) = &video_id { diff --git a/apps/desktop-gpui/src/upload.rs b/apps/desktop-gpui/src/upload.rs index 6bbef37d11d..29c8c831a06 100644 --- a/apps/desktop-gpui/src/upload.rs +++ b/apps/desktop-gpui/src/upload.rs @@ -462,7 +462,8 @@ impl InstantUpload { result.map_err(|error| format!("Instant recording thumbnail upload failed: {error}")) } - pub async fn cancel(&mut self) -> Result<(), String> { + /// `query` adds parameters to the server delete. + pub async fn cancel(&mut self, query: Option<&str>) -> Result<(), String> { self.abort_segments().await; if let Some(ownership) = &self.ownership { queue::record_cancelled(ownership.project_path())?; @@ -470,7 +471,7 @@ impl InstantUpload { with_upload_context( self.request_context.clone(), self.reads.clone(), - delete_instant_video(&self.video.id), + delete_instant_video_with_query(&self.video.id, query), ) .await } @@ -492,10 +493,6 @@ impl InstantUpload { } } -pub async fn delete_instant_video(video_id: &str) -> Result<(), String> { - delete_instant_video_with_query(video_id, None).await -} - pub(crate) async fn delete_instant_video_with_query( video_id: &str, query: Option<&str>, From f379ce325d94d0e1a020a95ce9fea224f2195d35 Mon Sep 17 00:00:00 2001 From: Richie McIlroy <33632126+richiemcilroy@users.noreply.github.com> Date: Sat, 24 Oct 2026 12:40:38 +0100 Subject: [PATCH 5/9] chore(desktop-gpui): drop a comment that restates cancel's parameter --- apps/desktop-gpui/src/upload.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/apps/desktop-gpui/src/upload.rs b/apps/desktop-gpui/src/upload.rs index 29c8c831a06..d952cb86b08 100644 --- a/apps/desktop-gpui/src/upload.rs +++ b/apps/desktop-gpui/src/upload.rs @@ -462,7 +462,6 @@ impl InstantUpload { result.map_err(|error| format!("Instant recording thumbnail upload failed: {error}")) } - /// `query` adds parameters to the server delete. pub async fn cancel(&mut self, query: Option<&str>) -> Result<(), String> { self.abort_segments().await; if let Some(ownership) = &self.ownership { From 041d065179fb07fbc6d96e1cde13c55129be5541 Mon Sep 17 00:00:00 2001 From: Richie McIlroy <33632126+richiemcilroy@users.noreply.github.com> Date: Sat, 24 Oct 2026 13:25:03 +0100 Subject: [PATCH 6/9] fix(desktop-gpui): keep an interrupted start's video when its folder is missing --- .../src/upload/instant_cleanup.rs | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/apps/desktop-gpui/src/upload/instant_cleanup.rs b/apps/desktop-gpui/src/upload/instant_cleanup.rs index 2b209630cb0..f2aef53ae95 100644 --- a/apps/desktop-gpui/src/upload/instant_cleanup.rs +++ b/apps/desktop-gpui/src/upload/instant_cleanup.rs @@ -411,8 +411,9 @@ fn project_needs_video(project: &Path, meta: Option<&RecordingMeta>, interrupted /// True when a local recording that must not lose its server copy points at /// this video: anything past `InProgress`, an unreadable project that claims /// the id, or (for interrupted starts) a project that already holds media. -/// An interrupted start whose own folder sits on storage that cannot be read -/// right now (an unmounted drive) is kept too: its media may still be there. +/// An interrupted start is kept unless its own folder can be read: the folder +/// is created before the server video, so a missing one was moved, deleted or +/// sits on a drive that is not mounted, and its media may still exist. fn locally_protected( recording_dirs: &[PathBuf], video_id: &str, @@ -420,17 +421,12 @@ fn locally_protected( start_project: Option<&Path>, ) -> bool { if interrupted && let Some(project) = start_project { - if !project - .parent() - .is_some_and(|parent| std::fs::read_dir(parent).is_ok()) - { + if !matches!(project.try_exists(), Ok(true)) { return true; } - if project.exists() { - let meta = RecordingMeta::load_for_project(project).ok(); - if project_needs_video(project, meta.as_ref(), interrupted) { - return true; - } + let meta = RecordingMeta::load_for_project(project).ok(); + if project_needs_video(project, meta.as_ref(), interrupted) { + return true; } } for directory in recording_dirs { @@ -1124,7 +1120,7 @@ mod tests { Some(&project), )); let deleted = recordings.join("deleted.cap"); - assert!(!locally_protected( + assert!(locally_protected( std::slice::from_ref(&recordings), "video", true, From dccf55420210932eb3849f283826d4ae1b86d50e Mon Sep 17 00:00:00 2001 From: Richie McIlroy <33632126+richiemcilroy@users.noreply.github.com> Date: Sat, 24 Oct 2026 14:03:03 +0100 Subject: [PATCH 7/9] fix(desktop-gpui): delete a crashed start's video when its folder was never created, and recheck the journal before each delete --- .../src/upload/instant_cleanup.rs | 124 ++++++++++++++++-- 1 file changed, 116 insertions(+), 8 deletions(-) diff --git a/apps/desktop-gpui/src/upload/instant_cleanup.rs b/apps/desktop-gpui/src/upload/instant_cleanup.rs index f2aef53ae95..46453e3fd3b 100644 --- a/apps/desktop-gpui/src/upload/instant_cleanup.rs +++ b/apps/desktop-gpui/src/upload/instant_cleanup.rs @@ -261,6 +261,38 @@ fn begin_at( } } +fn forget_at(path: &Path, video_id: &str) -> bool { + let _guard = JOURNAL_LOCK + .lock() + .unwrap_or_else(|error| error.into_inner()); + let mut journal = read_journal(path); + if !journal + .entries + .iter() + .any(|entry| entry.video_id == video_id) + { + return true; + } + journal.entries.retain(|entry| entry.video_id != video_id); + match write_journal(path, &journal) { + Ok(()) => true, + Err(error) => { + tracing::warn!(%error, video_id, "Instant cleanup could not clear a verified video"); + false + } + } +} + +fn still_journaled(path: &Path, video_id: &str) -> bool { + let _guard = JOURNAL_LOCK + .lock() + .unwrap_or_else(|error| error.into_inner()); + read_journal(path) + .entries + .iter() + .any(|entry| entry.video_id == video_id) +} + fn remove_at(path: &Path, video_id: &str) { if let Err(error) = update_journal(path, |entries| { entries.retain(|entry| entry.video_id != video_id); @@ -337,6 +369,17 @@ pub(crate) fn resolved(video_id: &str) { remove_at(&journal_path(), video_id); } +/// Called once the server has verified an upload. If `started` could not clear +/// the entry, a later sweep would otherwise find a start whose folder was +/// removed and delete the uploaded video. False when the entry is still there, +/// so the caller keeps the local copy. +pub(crate) fn forget_verified(video_id: &str) -> bool { + if cfg!(target_os = "linux") { + return true; + } + forget_at(&journal_path(), video_id) +} + pub(crate) fn abandon(video_id: &str, reason: CleanupReason, cause: Option) { if cfg!(target_os = "linux") { return; @@ -411,9 +454,10 @@ fn project_needs_video(project: &Path, meta: Option<&RecordingMeta>, interrupted /// True when a local recording that must not lose its server copy points at /// this video: anything past `InProgress`, an unreadable project that claims /// the id, or (for interrupted starts) a project that already holds media. -/// An interrupted start is kept unless its own folder can be read: the folder -/// is created before the server video, so a missing one was moved, deleted or -/// sits on a drive that is not mounted, and its media may still exist. +/// An interrupted start whose own folder sits on storage that cannot be read +/// right now (an unmounted drive) is kept too: its media may still be there. +/// A missing folder on readable storage is not protected, because capture +/// creates the folder only once it starts. fn locally_protected( recording_dirs: &[PathBuf], video_id: &str, @@ -421,12 +465,21 @@ fn locally_protected( start_project: Option<&Path>, ) -> bool { if interrupted && let Some(project) = start_project { - if !matches!(project.try_exists(), Ok(true)) { + if !project + .parent() + .is_some_and(|parent| std::fs::read_dir(parent).is_ok()) + { return true; } - let meta = RecordingMeta::load_for_project(project).ok(); - if project_needs_video(project, meta.as_ref(), interrupted) { - return true; + match project.try_exists() { + Ok(true) => { + let meta = RecordingMeta::load_for_project(project).ok(); + if project_needs_video(project, meta.as_ref(), interrupted) { + return true; + } + } + Ok(false) => {} + Err(_) => return true, } } for directory in recording_dirs { @@ -500,6 +553,9 @@ async fn sweep(path: &Path, session: &str, now: i64, backend: &impl SweepBackend remove.push(entry.video_id); continue; } + if !still_journaled(path, &entry.video_id) { + continue; + } match backend.delete(&entry).await { Ok(()) => { summary.deleted += 1; @@ -954,6 +1010,58 @@ mod tests { assert!(fixture.entries().is_empty()); } + #[tokio::test] + async fn a_video_verified_during_a_sweep_is_never_deleted() { + let fixture = Fixture::new(); + fixture.seed(vec![entry("uploaded", "earlier", Stage::Starting, 100)]); + struct VerifiedDuringCheck<'a> { + journal: &'a Path, + deletes: AtomicUsize, + } + impl SweepBackend for VerifiedDuringCheck<'_> { + fn account_ready(&self, _entry: &Entry) -> bool { + true + } + async fn protected(&self, entry: &Entry) -> bool { + assert!(forget_at(self.journal, &entry.video_id)); + false + } + async fn delete(&self, _entry: &Entry) -> Result<(), String> { + self.deletes.fetch_add(1, Ordering::SeqCst); + Ok(()) + } + } + let journal = fixture.journal(); + let backend = VerifiedDuringCheck { + journal: &journal, + deletes: AtomicUsize::new(0), + }; + sweep( + &journal, + "current", + 100 + INTERRUPTED_GRACE_SECS + 1, + &backend, + ) + .await; + assert_eq!(backend.deletes.load(Ordering::SeqCst), 0); + assert!(fixture.entries().is_empty()); + } + + #[test] + fn forgetting_a_verified_video_clears_only_that_entry() { + let fixture = Fixture::new(); + let journal = fixture.journal(); + assert!(forget_at(&journal, "absent")); + assert!(!journal.exists()); + fixture.seed(vec![ + entry("uploaded", "earlier", Stage::Starting, 100), + entry("other", "earlier", Stage::Starting, 100), + ]); + assert!(forget_at(&journal, "uploaded")); + let remaining: Vec<_> = fixture.entries().into_iter().map(|e| e.video_id).collect(); + assert_eq!(remaining, vec!["other".to_string()]); + } + fn write_project( root: &Path, name: &str, @@ -1120,7 +1228,7 @@ mod tests { Some(&project), )); let deleted = recordings.join("deleted.cap"); - assert!(locally_protected( + assert!(!locally_protected( std::slice::from_ref(&recordings), "video", true, From 2f14141e374ebad020ce0f8c156da9b491797a66 Mon Sep 17 00:00:00 2001 From: Richie McIlroy <33632126+richiemcilroy@users.noreply.github.com> Date: Sat, 24 Oct 2026 14:03:03 +0100 Subject: [PATCH 8/9] fix(desktop-gpui): clear a verified upload's start cleanup entry and keep the local copy if that fails --- apps/desktop-gpui/src/upload/queue.rs | 32 ++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/apps/desktop-gpui/src/upload/queue.rs b/apps/desktop-gpui/src/upload/queue.rs index 41d726c2e3b..9d8495c1e4e 100644 --- a/apps/desktop-gpui/src/upload/queue.rs +++ b/apps/desktop-gpui/src/upload/queue.rs @@ -376,6 +376,7 @@ trait UploadBackend: Send + Sync + 'static { verification: &UploadVerification, ) -> Result<(), String>; fn delete_after_upload(&self) -> bool; + fn forget_start_cleanup(&self, video_id: &str) -> bool; fn notify_failure(&self); } @@ -523,6 +524,10 @@ impl UploadBackend for LiveBackend { crate::store::GeneralSettings::load().delete_instant_recordings_after_upload } + fn forget_start_cleanup(&self, video_id: &str) -> bool { + super::instant_cleanup::forget_verified(video_id) + } + fn notify_failure(&self) { crate::app_sounds::play_notification(); } @@ -826,9 +831,12 @@ impl Manager { Some(&upload.cancel), ) .map_err(|error| error.to_string())?; + let cleanup_cleared = self.backend.forget_start_cleanup(&state.video_id); if !state.preserve_local && self.backend.delete_after_upload() { check_segment_cancelled(&upload.cancel)?; - if let Err(error) = std::fs::remove_dir_all(project) { + if !cleanup_cleared { + tracing::warn!(path = %project.display(), "Verified upload kept locally because its cleanup entry could not be cleared"); + } else if let Err(error) = std::fs::remove_dir_all(project) { tracing::warn!(path = %project.display(), %error, "Verified upload kept locally because cleanup failed"); } } @@ -1348,6 +1356,7 @@ mod tests { stall_confirmation: AtomicBool, reupload_required: AtomicBool, delete: AtomicBool, + keep_cleanup_entry: AtomicBool, transfers: AtomicUsize, confirmations: AtomicUsize, failure_notifications: AtomicUsize, @@ -1439,6 +1448,10 @@ mod tests { self.delete.load(Ordering::Acquire) } + fn forget_start_cleanup(&self, _video_id: &str) -> bool { + !self.keep_cleanup_entry.load(Ordering::Acquire) + } + fn notify_failure(&self) { self.failure_notifications.fetch_add(1, Ordering::AcqRel); } @@ -1867,6 +1880,23 @@ mod tests { } } + #[tokio::test] + async fn verified_upload_stays_local_when_its_cleanup_entry_cannot_be_cleared() { + let fixture = Fixture::new(); + let backend = FakeBackend::default(); + backend.verified.store(true, Ordering::Release); + backend.delete.store(true, Ordering::Release); + backend.keep_cleanup_entry.store(true, Ordering::Release); + let manager = Arc::new(Manager::new(backend, Duration::from_secs(1))); + manager + .admit(fixture.project(), fixture.upload(), false) + .await + .unwrap(); + joined(&manager).await; + assert_eq!(fixture.state().phase, UploadPhase::Verified); + assert!(fixture.project().join("content/output.mp4").is_file()); + } + #[tokio::test] async fn cancelled_worker_cannot_complete_even_after_network_gate_opens() { let fixture = Fixture::new(); From 714831eb84341e45dfb01a768339251135763d4d Mon Sep 17 00:00:00 2001 From: Richie McIlroy <33632126+richiemcilroy@users.noreply.github.com> Date: Sat, 24 Oct 2026 14:21:03 +0100 Subject: [PATCH 9/9] fix(desktop-gpui): keep a crashed start's video unless its own folder can be read, and never treat an unreadable journal as empty --- .../src/upload/instant_cleanup.rs | 118 ++++++++++++------ 1 file changed, 80 insertions(+), 38 deletions(-) diff --git a/apps/desktop-gpui/src/upload/instant_cleanup.rs b/apps/desktop-gpui/src/upload/instant_cleanup.rs index 46453e3fd3b..738f09c4849 100644 --- a/apps/desktop-gpui/src/upload/instant_cleanup.rs +++ b/apps/desktop-gpui/src/upload/instant_cleanup.rs @@ -163,33 +163,36 @@ fn retry_delay(attempts: u32) -> i64 { .min(RETRY_MAX_SECS) } -fn read_journal(path: &Path) -> Journal { +/// A missing, oversized or unparsable journal reads as empty and is replaced +/// by the next write. A journal that exists but cannot be read right now is an +/// error, so no caller mistakes it for one without entries. +fn read_journal_checked(path: &Path) -> Result { let bytes = match std::fs::symlink_metadata(path) { - Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Journal::default(), - Err(error) => { - tracing::warn!(%error, "Instant cleanup journal is unreadable"); - return Journal::default(); + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + return Ok(Journal::default()); } + Err(error) => return Err(error.to_string()), Ok(metadata) if !metadata.is_file() || metadata.len() > MAX_JOURNAL_BYTES => { tracing::warn!("Instant cleanup journal is not a usable file"); - return Journal::default(); + return Ok(Journal::default()); } - Ok(_) => match std::fs::read(path) { - Ok(bytes) => bytes, - Err(error) => { - tracing::warn!(%error, "Instant cleanup journal is unreadable"); - return Journal::default(); - } - }, + Ok(_) => std::fs::read(path).map_err(|error| error.to_string())?, }; - match serde_json::from_slice::(&bytes) { + Ok(match serde_json::from_slice::(&bytes) { Ok(journal) if journal.version == 1 => journal, Ok(_) => Journal::default(), Err(error) => { tracing::warn!(%error, "Instant cleanup journal could not be parsed"); Journal::default() } - } + }) +} + +fn read_journal(path: &Path) -> Journal { + read_journal_checked(path).unwrap_or_else(|error| { + tracing::warn!(%error, "Instant cleanup journal is unreadable"); + Journal::default() + }) } fn write_journal(path: &Path, journal: &Journal) -> Result<(), String> { @@ -221,7 +224,7 @@ fn update_journal(path: &Path, change: impl FnOnce(&mut Vec)) -> Result<( let _guard = JOURNAL_LOCK .lock() .unwrap_or_else(|error| error.into_inner()); - let mut journal = read_journal(path); + let mut journal = read_journal_checked(path)?; journal.version = 1; change(&mut journal.entries); if journal.entries.len() > MAX_ENTRIES { @@ -265,7 +268,13 @@ fn forget_at(path: &Path, video_id: &str) -> bool { let _guard = JOURNAL_LOCK .lock() .unwrap_or_else(|error| error.into_inner()); - let mut journal = read_journal(path); + let mut journal = match read_journal_checked(path) { + Ok(journal) => journal, + Err(error) => { + tracing::warn!(%error, video_id, "Instant cleanup could not read the journal to clear a verified video"); + return false; + } + }; if !journal .entries .iter() @@ -410,6 +419,13 @@ fn upload_meta_video_id(meta: &RecordingMeta) -> Option<&str> { } } +fn meta_claims(meta: &RecordingMeta, video_id: &str) -> bool { + meta.sharing + .as_ref() + .is_some_and(|share| share.id == video_id) + || upload_meta_video_id(meta) == Some(video_id) +} + /// True when the folder holds recorded media, or when part of it cannot be /// read: media that cannot be checked counts as present. Only a missing folder /// counts as empty. @@ -454,10 +470,13 @@ fn project_needs_video(project: &Path, meta: Option<&RecordingMeta>, interrupted /// True when a local recording that must not lose its server copy points at /// this video: anything past `InProgress`, an unreadable project that claims /// the id, or (for interrupted starts) a project that already holds media. -/// An interrupted start whose own folder sits on storage that cannot be read -/// right now (an unmounted drive) is kept too: its media may still be there. -/// A missing folder on readable storage is not protected, because capture -/// creates the folder only once it starts. +/// An interrupted start is kept unless its own folder can be read and checked: +/// a folder on storage that cannot be read right now (an unmounted drive) may +/// still hold media, and a missing folder cannot be told apart from one moved +/// elsewhere with its media, even though capture usually creates it moments +/// after the server video. The folder name is only reserved before capture +/// starts, so a later recording can reuse it; a folder that belongs to another +/// video counts as missing. fn locally_protected( recording_dirs: &[PathBuf], video_id: &str, @@ -465,21 +484,18 @@ fn locally_protected( start_project: Option<&Path>, ) -> bool { if interrupted && let Some(project) = start_project { - if !project - .parent() - .is_some_and(|parent| std::fs::read_dir(parent).is_ok()) - { - return true; - } match project.try_exists() { Ok(true) => { let meta = RecordingMeta::load_for_project(project).ok(); - if project_needs_video(project, meta.as_ref(), interrupted) { + if meta + .as_ref() + .is_some_and(|meta| !meta_claims(meta, video_id)) + || project_needs_video(project, meta.as_ref(), interrupted) + { return true; } } - Ok(false) => {} - Err(_) => return true, + Ok(false) | Err(_) => return true, } } for directory in recording_dirs { @@ -493,12 +509,9 @@ fn locally_protected( Ok(Some(state)) if state.video_id() == video_id ); let meta = RecordingMeta::load_for_project(&project).ok(); - let meta_matches = meta.as_ref().is_some_and(|meta| { - meta.sharing - .as_ref() - .is_some_and(|share| share.id == video_id) - || upload_meta_video_id(meta) == Some(video_id) - }); + let meta_matches = meta + .as_ref() + .is_some_and(|meta| meta_claims(meta, video_id)); if (state_matches || meta_matches) && project_needs_video(&project, meta.as_ref(), interrupted) { @@ -1047,6 +1060,28 @@ mod tests { assert!(fixture.entries().is_empty()); } + #[cfg(unix)] + #[test] + fn an_unreadable_journal_is_never_treated_as_empty() { + use std::os::unix::fs::PermissionsExt; + let fixture = Fixture::new(); + let journal = fixture.journal(); + fixture.seed(vec![entry("uploaded", "earlier", Stage::Starting, 100)]); + std::fs::set_permissions(&journal, std::fs::Permissions::from_mode(0o000)).unwrap(); + let readable = std::fs::read(&journal).is_ok(); + let forgotten = forget_at(&journal, "uploaded"); + let still_there = still_journaled(&journal, "uploaded"); + let written = update_journal(&journal, |entries| entries.clear()); + std::fs::set_permissions(&journal, std::fs::Permissions::from_mode(0o644)).unwrap(); + if readable { + return; + } + assert!(!forgotten); + assert!(!still_there); + assert!(written.is_err()); + assert_eq!(fixture.entries().len(), 1); + } + #[test] fn forgetting_a_verified_video_clears_only_that_entry() { let fixture = Fixture::new(); @@ -1228,19 +1263,26 @@ mod tests { Some(&project), )); let deleted = recordings.join("deleted.cap"); - assert!(!locally_protected( + assert!(locally_protected( std::slice::from_ref(&recordings), "video", true, Some(&deleted), )); - let failed = write_project(&recordings, "failed.cap", "other", in_progress()); + let failed = write_project(&recordings, "failed.cap", "video", in_progress()); assert!(!locally_protected( std::slice::from_ref(&recordings), "video", true, Some(&failed), )); + let reused = write_project(&recordings, "reused.cap", "next-video", in_progress()); + assert!(locally_protected( + std::slice::from_ref(&recordings), + "video", + true, + Some(&reused), + )); std::fs::write(failed.join("content/display/segment_001.m4s"), b"media").unwrap(); assert!(locally_protected( &[recordings],