diff --git a/templates/clips/actions/finalize-recording.test.ts b/templates/clips/actions/finalize-recording.test.ts index 85a255688a..460378f4f3 100644 --- a/templates/clips/actions/finalize-recording.test.ts +++ b/templates/clips/actions/finalize-recording.test.ts @@ -12,6 +12,7 @@ const mockState = vi.hoisted(() => ({ hasAudio: true, hasCamera: false, title: "Test recording", + uploadGenerationId: null as string | null, }, uploadState: null as Record | null, chunkRows: [] as Array<{ key: string }>, @@ -106,6 +107,7 @@ vi.mock("../server/db/index.js", () => ({ id: "recordings.id", ownerEmail: "recordings.ownerEmail", status: "recordings.status", + uploadGenerationId: "recordings.uploadGenerationId", videoUrl: "recordings.videoUrl", trashedAt: "recordings.trashedAt", }, @@ -188,6 +190,8 @@ describe("finalize-recording chunk completeness", () => { }; mockState.chunkRows = []; mockState.selectRows = []; + mockState.existingRecording.status = "uploading"; + mockState.existingRecording.uploadGenerationId = null; mockReadAppState.mockImplementation(async (key: string) => { if (key === "recording-upload-rec_1") return mockState.uploadState; return null; @@ -198,6 +202,36 @@ describe("finalize-recording chunk completeness", () => { })); }); + it("does not finalize after reset wins the generation claim race", async () => { + mockState.existingRecording = { + ...mockState.existingRecording, + status: "uploading", + uploadGenerationId: "generation-a", + }; + // The generation/status CAS returns no row: reset installed generation B + // after finalize read A but before it could claim processing. + mockUpdateReturning.mockResolvedValueOnce([]); + + await expect( + finalizeRecording.run({ + id: "rec_1", + uploadGenerationId: "generation-a", + }), + ).rejects.toThrow("Upload changed before finalization could claim it"); + + expect(mockUploadFile).not.toHaveBeenCalled(); + }); + + it("rejects an unfenced finalizer after reset installs a generation", async () => { + mockState.existingRecording.uploadGenerationId = "generation-b"; + + await expect(finalizeRecording.run({ id: "rec_1" })).rejects.toThrow( + "Upload generation changed before finalization", + ); + + expect(mockUploadFile).not.toHaveBeenCalled(); + }); + it("fails before upload when persisted chunk indices have a gap", async () => { mockState.chunkRows = [ { key: "recording-chunks-rec_1-000000" }, @@ -786,6 +820,6 @@ describe("finalize-recording resumable recovery", () => { videoUrl: "https://cdn.example.com/rec_1", }), ); - expect(deleteResumableSession).toHaveBeenCalledWith("rec_1"); + expect(deleteResumableSession).toHaveBeenCalledWith("rec_1", null); }); }); diff --git a/templates/clips/actions/finalize-recording.ts b/templates/clips/actions/finalize-recording.ts index 147e1a98f4..e5c4d5a303 100644 --- a/templates/clips/actions/finalize-recording.ts +++ b/templates/clips/actions/finalize-recording.ts @@ -902,6 +902,12 @@ export default defineAction({ "Whether the uploaded video bytes were already locally transcoded/compressed before upload", ), mediaVerificationRetryAttempt: z.number().int().min(1).max(10).optional(), + uploadGenerationId: z + .string() + .min(1) + .max(128) + .optional() + .describe("Upload generation that owns the scratch data being finalized"), }), run: async (args) => { const db = getDb(); @@ -921,7 +927,7 @@ export default defineAction({ // never retain scratch video blobs. let chunkKeysToPurge: string[] = []; try { - const [existing] = await db + let [existing] = await db .select() .from(schema.recordings) .where( @@ -938,6 +944,32 @@ export default defineAction({ throw new Error(`Recording not found: ${id}`); } + const generationId = args.uploadGenerationId ?? null; + if ((existing.uploadGenerationId ?? null) !== generationId) { + throw new Error("Upload generation changed before finalization"); + } + // Claim finalization before touching provider/scratch state. Reset only + // admits uploading/failed rows, so once this CAS succeeds it cannot + // replace the generation underneath a delayed final chunk. + if (generationId !== null && existing.status === "uploading") { + const claimed = await db + .update(schema.recordings) + .set({ status: "processing", updatedAt: new Date().toISOString() }) + .where( + and( + eq(schema.recordings.id, id), + ownerEmailMatches(schema.recordings.ownerEmail, ownerEmail), + eq(schema.recordings.status, "uploading"), + eq(schema.recordings.uploadGenerationId, generationId), + ), + ) + .returning(); + if (claimed.length !== 1) { + throw new Error("Upload changed before finalization could claim it"); + } + existing = claimed[0]!; + } + // Idempotency guard: finalize can be re-invoked when a client retries the // final chunk after a lost response. If already 'ready' return the existing // result instead of re-running the complete/assembly path (session and @@ -947,7 +979,7 @@ export default defineAction({ // A prior attempt may have persisted the ready row and then failed // before deleting its resumable-session handle. The provider upload is // complete at this point, so retire only the local retry state. - await deleteResumableSession(id).catch((err) => + await deleteResumableSession(id, generationId).catch((err) => console.warn("[finalize] failed to delete resumable session:", err), ); await deleteAppState(mediaVerificationStateKey(id)).catch(() => {}); @@ -1038,7 +1070,7 @@ export default defineAction({ // Resumable path: create-recording initialized a session and chunk.post.ts // forwarded all chunks to the provider. Complete the session to get the CDN URL. - const resumableSession = await getResumableSession(id); + const resumableSession = await getResumableSession(id, generationId); if (resumableSession && isStreamingUploadDisabled()) { console.warn( `[finalize] streaming uploads are disabled, but completing existing resumable session for in-flight recording: ${id}`, @@ -1147,7 +1179,7 @@ export default defineAction({ locallyTranscoded: args.locallyTranscoded === true, }, }); - await deleteResumableSession(id).catch((deleteErr) => + await deleteResumableSession(id, generationId).catch((deleteErr) => console.warn( "[finalize] failed to retire pending resumable session:", deleteErr, @@ -1178,7 +1210,7 @@ export default defineAction({ } // Delete only after durable state is written — so a retry before // this point can still find the session and re-enter this path. - deleteResumableSession(id).catch((err) => + deleteResumableSession(id, generationId).catch((err) => console.warn("[finalize] failed to delete resumable session:", err), ); return result; @@ -1262,7 +1294,11 @@ export default defineAction({ // Pull chunk keys first, then fetch values one at a time. A single // SELECT key,value over many base64 chunks can exceed Neon's 8s op // timeout before we even start assembling the recording. - const chunkKeys = await listRecordingChunkKeys(ownerEmail, id); + const chunkKeys = await listRecordingChunkKeys( + ownerEmail, + id, + generationId, + ); const expectedDataChunks = stateNumber(uploadState, "expectedDataChunks"); debugLog("[finalize] chunks found", { id, diff --git a/templates/clips/app/components/recorder/recorder-engine.ts b/templates/clips/app/components/recorder/recorder-engine.ts index 52358877c5..91753f9c56 100644 --- a/templates/clips/app/components/recorder/recorder-engine.ts +++ b/templates/clips/app/components/recorder/recorder-engine.ts @@ -540,6 +540,7 @@ export class RecorderEngine { */ private uploadAbort: AbortController | null = null; private uploadMode: UploadMode = "buffered"; + private uploadGenerationId: string | null = null; /** * Streaming-path buffer. MediaRecorder blobs accumulate here until at least * STREAM_CHUNK_BYTES is available, then a 256 KiB-aligned slice is PUT to API @@ -1102,6 +1103,7 @@ export class RecorderEngine { this.opts.uploadUrl = target.uploadUrl; this.opts.abortUrl = target.abortUrl; this.opts.uploadMode = target.uploadMode ?? "buffered"; + this.uploadGenerationId = null; } // ------------------------------------------------------------------------- @@ -1178,6 +1180,7 @@ export class RecorderEngine { this.lastFinalizeMeta = null; this.uploadAbort = new AbortController(); this.uploadMode = this.opts.uploadMode ?? "buffered"; + this.uploadGenerationId = null; this.pendingStreamBlobs = []; this.pendingStreamBytes = 0; this.cameraDisconnectNotified = false; @@ -1540,6 +1543,10 @@ export class RecorderEngine { compression, requestStreaming: this.uploadMode === "streaming", mimeType: uploadMimeType, + useGenerationFence: true, + ...(this.uploadGenerationId + ? { uploadGenerationId: this.uploadGenerationId } + : {}), }), signal, }); @@ -1566,6 +1573,7 @@ export class RecorderEngine { } const reset = (await resetRes.json().catch(() => null)) as { uploadMode?: unknown; + uploadGenerationId?: unknown; } | null; if (reset?.uploadMode !== "streaming" && reset?.uploadMode !== "buffered") { throw new Error( @@ -1574,6 +1582,11 @@ export class RecorderEngine { } const uploadMode = reset.uploadMode; this.uploadMode = uploadMode; + this.uploadGenerationId = + typeof reset.uploadGenerationId === "string" && + reset.uploadGenerationId.length > 0 + ? reset.uploadGenerationId + : null; return uploadMode; } @@ -1752,8 +1765,10 @@ export class RecorderEngine { this.uploadAbort = null; } this.cleanupTracks(); + const uploadGenerationId = this.uploadGenerationId; this.chunkIndex = 0; this.uploadFailure = null; + this.uploadGenerationId = null; this.startedAtMs = null; this.pausedAccumMs = 0; this.pausedStartedMs = null; @@ -1764,7 +1779,13 @@ export class RecorderEngine { if (this.opts.abortUrl) { try { - await fetch(this.opts.abortUrl, { method: "POST" }); + await fetch(this.opts.abortUrl, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + ...(uploadGenerationId ? { uploadGenerationId } : {}), + }), + }); } catch { // ignore — best effort } @@ -2136,6 +2157,7 @@ export class RecorderEngine { height: extra.height, hasAudio: extra.hasAudio, hasCamera: extra.hasCamera, + uploadGenerationId: this.uploadGenerationId ?? undefined, }); const body = await blob.arrayBuffer(); diff --git a/templates/clips/app/components/recorder/recorder-engine.upload-generation.test.ts b/templates/clips/app/components/recorder/recorder-engine.upload-generation.test.ts new file mode 100644 index 0000000000..0b4603521e --- /dev/null +++ b/templates/clips/app/components/recorder/recorder-engine.upload-generation.test.ts @@ -0,0 +1,67 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { RecorderEngine } from "./recorder-engine"; + +describe("RecorderEngine upload generation fencing", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("carries each reset generation through chunks and the next reset", async () => { + vi.stubGlobal("window", { + setTimeout, + clearTimeout, + location: { pathname: "/" }, + }); + const requests: Array<{ url: string; body: unknown }> = []; + let resetCount = 0; + vi.stubGlobal( + "fetch", + vi.fn(async (input: string | URL | Request, init?: RequestInit) => { + const url = String(input); + if (url.endsWith("/reset-chunks")) { + resetCount += 1; + requests.push({ + url, + body: JSON.parse(String(init?.body)), + }); + return Response.json({ + uploadMode: "buffered", + uploadGenerationId: `generation-${resetCount}`, + }); + } + requests.push({ url, body: init?.body }); + return Response.json({ ok: true }); + }), + ); + + const engine = new RecorderEngine({ + recordingId: "rec-1", + mode: "screen", + uploadUrl: "/api/uploads/rec-1/chunk", + abortUrl: "/api/uploads/rec-1/abort", + }); + const internals = engine as unknown as { + resetUploadedChunks: (compression: null) => Promise<"buffered">; + uploadChunk: (blob: Blob, index: number) => Promise; + }; + + await internals.resetUploadedChunks(null); + await internals.uploadChunk(new Blob(["first"]), 0); + await internals.resetUploadedChunks(null); + await internals.uploadChunk(new Blob(["second"]), 0); + await engine.cancel(); + + expect(requests[0]?.body).toMatchObject({ useGenerationFence: true }); + expect(requests[1]?.url).toContain("uploadGenerationId=generation-1"); + expect(requests[2]?.body).toMatchObject({ + useGenerationFence: true, + uploadGenerationId: "generation-1", + }); + expect(requests[3]?.url).toContain("uploadGenerationId=generation-2"); + expect(requests[4]).toMatchObject({ + url: "/api/uploads/rec-1/abort", + body: JSON.stringify({ uploadGenerationId: "generation-2" }), + }); + }); +}); diff --git a/templates/clips/changelog/2026-07-27-saved-clips-now-resume-from-the-last-uploaded-byte-after-an-.md b/templates/clips/changelog/2026-07-27-saved-clips-now-resume-from-the-last-uploaded-byte-after-an-.md new file mode 100644 index 0000000000..41a30a5294 --- /dev/null +++ b/templates/clips/changelog/2026-07-27-saved-clips-now-resume-from-the-last-uploaded-byte-after-an-.md @@ -0,0 +1,6 @@ +--- +type: fixed +date: 2026-07-27 +--- + +Saved Clips now resume from the last uploaded byte after an interruption and promptly show when another retry is needed. diff --git a/templates/clips/desktop/src-tauri/src/native_screen.rs b/templates/clips/desktop/src-tauri/src/native_screen.rs index 1bf17be250..886f8b6057 100644 --- a/templates/clips/desktop/src-tauri/src/native_screen.rs +++ b/templates/clips/desktop/src-tauri/src/native_screen.rs @@ -155,6 +155,10 @@ const FFMPEG_CANDIDATE_PATHS: &[&str] = &[ ]; const PENDING_UPLOADS_DIR: &str = "pending-recording-uploads"; const CLIP_DRAFTS_DIR: &str = "Drafts"; +const NATIVE_UPLOAD_RESTART_REQUIRED: &str = "native upload requires a one-time restart"; +const NATIVE_UPLOAD_UNFENCED_RESTART_REQUIRED: &str = + "native upload requires an unfenced one-time restart"; +static NATIVE_RETRY_CLAIM_COUNTER: AtomicU64 = AtomicU64::new(1); // Minimum free space required to start recording; below this we hard-block. pub(crate) const DISK_SPACE_BLOCK_BYTES: u64 = 500 * 1024 * 1024; // Free space below this at start time is logged as a warning but not blocked. @@ -172,6 +176,51 @@ pub(crate) enum NativeUploadMode { Streaming, } +#[derive(Debug, Clone)] +struct NativeStreamingResume { + bytes_received: u64, + next_chunk_index: usize, + attempt_id: Option, + upload_generation_id: Option, +} + +#[derive(Debug, Clone)] +enum NativeRetryUploadPlan { + Resume(NativeStreamingResume), + Restart { + attempt_id: Option, + upload_generation_id: Option, + }, + Reconcile, +} + +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +struct NativeUploadResumeResponse { + resumable: bool, + #[serde(default)] + recovery_enabled: bool, + status: Option, + upload_mode: Option, + bytes_received: Option, + next_chunk_index: Option, + attempt_id: Option, + upload_generation_id: Option, +} + +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +struct NativeUploadResetResponse { + upload_mode: Option, + upload_generation_id: Option, +} + +impl NativeUploadResetResponse { + fn mode(&self) -> NativeUploadMode { + NativeUploadMode::from_option(self.upload_mode.clone()) + } +} + impl NativeUploadMode { pub(crate) fn from_option(value: Option) -> Self { match value.as_deref() { @@ -187,6 +236,7 @@ impl NativeUploadMode { } } + #[cfg(test)] fn from_reset_response(body: &str) -> Self { let value = serde_json::from_str::(body).ok(); let upload_mode = value @@ -526,6 +576,8 @@ impl SharedClipSink { &server_url, &recording_id, MP4_RECORDING_MIME_TYPE, + None, + None, &auth_token, &cookie, ) @@ -964,6 +1016,8 @@ struct SavedNativeRecording { last_error: Option, retry_count: u32, #[serde(default)] + retry_attempt_id: Option, + #[serde(default)] custom_pipeline: bool, /// True when the SCK finalization callback reported an error, meaning the /// MP4 is missing its moov atom and cannot be recovered by retrying. @@ -1871,6 +1925,8 @@ pub async fn native_fullscreen_recording_stop_and_upload( &server_url, &recording_id, session.mime_type, + None, + None, &auth_token, &cookie, ) @@ -2965,6 +3021,7 @@ pub async fn native_fullscreen_recording_retry_upload( saved.server_url = server_url.trim_end_matches('/').to_string(); saved.last_attempt_at = Some(now_iso()); saved.last_error = None; + let claimed_attempt_id = saved_native_retry_attempt_id(&mut saved.retry_attempt_id); write_saved_recording_metadata(&app, &saved)?; // Preparation can normalize or transcode the saved recording. The @@ -2972,37 +3029,208 @@ pub async fn native_fullscreen_recording_retry_upload( // upload, not the source file's potentially stale MIME type. let result = async { let (prepared, retry_combined_path) = prepare_saved_recording_file(&app, &saved)?; - let upload_mode = match reset_upload_chunks( + let auth_token = auth_token.unwrap_or_default(); + let cookie = cookie.unwrap_or_default(); + // A resume offset only applies to the exact byte stream previously + // uploaded. Any retry-time concat/transcode creates different bytes, + // so take the explicit reset path instead of skipping an unsafe prefix. + let can_resume_exact_stream = !prepared.temporary && retry_combined_path.is_none(); + // This saved claim survives response loss and later user retries, so a + // second click cannot steal an upload session already owned by this + // local recording. + let retry_plan = match get_native_retry_upload_plan( &saved.server_url, &saved.recording_id, - &prepared.mime_type, - auth_token.as_deref().unwrap_or(""), - cookie.as_deref().unwrap_or(""), + prepared.bytes, + can_resume_exact_stream, + &claimed_attempt_id, + &auth_token, + &cookie, ) .await { - Ok(upload_mode) => upload_mode, + Ok(plan) => plan, Err(err) => { + interrupt_native_retry_upload( + &saved.server_url, + &saved.recording_id, + &err, + Some(&claimed_attempt_id), + None, + &auth_token, + &cookie, + ) + .await; cleanup_prepared_saved_recording_files(&prepared, retry_combined_path); return Err(err); } }; + let active_attempt_id = match &retry_plan { + NativeRetryUploadPlan::Resume(resume) => resume.attempt_id.clone(), + NativeRetryUploadPlan::Restart { attempt_id, .. } => attempt_id.clone(), + NativeRetryUploadPlan::Reconcile => None, + }; + let active_upload_generation_id = match &retry_plan { + NativeRetryUploadPlan::Resume(resume) => resume.upload_generation_id.clone(), + NativeRetryUploadPlan::Restart { + upload_generation_id, + .. + } => upload_generation_id.clone(), + NativeRetryUploadPlan::Reconcile => None, + }; + + if let NativeRetryUploadPlan::Reconcile = retry_plan { + cleanup_prepared_saved_recording_files(&prepared, retry_combined_path); + return Ok(NativeFullscreenUploadResult { + recording_id: saved.recording_id.clone(), + duration_ms: saved.duration_ms, + width: saved.width, + height: saved.height, + bytes: prepared.bytes, + // Keep the backup until the ordinary owner-status reconciliation + // proves the accepted media is ready and complete. + verification_pending: true, + }); + } + + let (upload_mode, streaming_resume, upload_generation_id) = match retry_plan { + NativeRetryUploadPlan::Resume(resume) => { + emit_native_upload_progress( + &app, + "uploading", + "Resuming upload", + None, + Some(resume.bytes_received as f32 / prepared.bytes as f32), + ); + let upload_generation_id = resume.upload_generation_id.clone(); + ( + NativeUploadMode::Streaming, + Some(resume), + upload_generation_id, + ) + } + NativeRetryUploadPlan::Restart { + attempt_id, + upload_generation_id, + } => { + emit_native_upload_progress( + &app, + "uploading", + "Restarting upload", + None, + Some(0.0), + ); + let reset = match reset_upload_chunks( + &saved.server_url, + &saved.recording_id, + &prepared.mime_type, + attempt_id.as_deref(), + upload_generation_id.as_deref(), + &auth_token, + &cookie, + ) + .await + { + Ok(reset) => reset, + Err(err) => { + interrupt_native_retry_upload( + &saved.server_url, + &saved.recording_id, + &err, + active_attempt_id.as_deref(), + active_upload_generation_id.as_deref(), + &auth_token, + &cookie, + ) + .await; + return Err(err); + } + }; + (reset.mode(), None, reset.upload_generation_id) + } + NativeRetryUploadPlan::Reconcile => unreachable!("handled above"), + }; + let upload_result = upload_prepared_recording_file( &app, &prepared, saved.server_url.clone(), saved.recording_id.clone(), - auth_token.unwrap_or_default(), - cookie.unwrap_or_default(), + auth_token.clone(), + cookie.clone(), upload_mode, saved.duration_ms, saved.width, saved.height, saved.has_audio, saved.has_camera, + active_attempt_id.clone(), + upload_generation_id.clone(), + streaming_resume, ) .await; + let replay_attempt_id = native_replay_attempt_id( + &upload_result, + active_attempt_id.as_deref(), + ); + let replay_upload_generation_id = replay_attempt_id + .as_ref() + .and_then(|_| upload_generation_id.clone()); + let mut interruption_upload_generation_id = upload_generation_id.clone(); + let upload_result = if native_upload_restart_required(&upload_result) { + eprintln!( + "[clips-tray] native retry replaying from byte zero after the provider requested a restart" + ); + match reset_upload_chunks( + &saved.server_url, + &saved.recording_id, + &prepared.mime_type, + replay_attempt_id.as_deref(), + replay_upload_generation_id.as_deref(), + &auth_token, + &cookie, + ) + .await + { + Ok(reset) => { + interruption_upload_generation_id = reset.upload_generation_id.clone(); + upload_prepared_recording_file( + &app, + &prepared, + saved.server_url.clone(), + saved.recording_id.clone(), + auth_token.clone(), + cookie.clone(), + reset.mode(), + saved.duration_ms, + saved.width, + saved.height, + saved.has_audio, + saved.has_camera, + replay_attempt_id.clone(), + reset.upload_generation_id, + None, + ) + .await + } + Err(err) => Err(err), + } + } else { + upload_result + }; + if let Err(err) = &upload_result { + interrupt_native_retry_upload( + &saved.server_url, + &saved.recording_id, + err, + replay_attempt_id.as_deref(), + interruption_upload_generation_id.as_deref(), + &auth_token, + &cookie, + ) + .await; + } cleanup_prepared_saved_recording_files(&prepared, retry_combined_path); upload_result } @@ -3532,6 +3760,7 @@ fn saved_recording_from_path( last_attempt_at: None, last_error: None, retry_count: 0, + retry_attempt_id: None, custom_pipeline: session.custom_pipeline, corrupt: false, }) @@ -3643,6 +3872,7 @@ pub(crate) fn persist_shared_clip_recording( last_attempt_at: error.map(|_| now_iso()), last_error: error.map(ToString::to_string), retry_count: u32::from(error.is_some()), + retry_attempt_id: None, custom_pipeline: true, corrupt: false, }; @@ -4234,6 +4464,9 @@ pub(crate) async fn upload_finalized_native_artifact( artifact.height, has_audio, has_camera, + None, + None, + None, ) .await; if prepared.temporary { @@ -4390,6 +4623,9 @@ async fn upload_prepared_recording_file( height: Option, has_audio: bool, has_camera: bool, + upload_attempt_id: Option, + upload_generation_id: Option, + streaming_resume: Option, ) -> Result { #[cfg(target_os = "macos")] let verified_local_duration_ms = { @@ -4415,7 +4651,21 @@ async fn upload_prepared_recording_file( } else { total_chunks + 1 }; - emit_native_upload_progress(app, "uploading", "Uploading clip", None, Some(0.0)); + let resumed_bytes = streaming_resume + .as_ref() + .map(|resume| resume.bytes_received) + .unwrap_or(0); + emit_native_upload_progress( + app, + "uploading", + if streaming_resume.is_some() { + "Resuming upload" + } else { + "Uploading clip" + }, + None, + Some(resumed_bytes as f32 / total_bytes as f32), + ); eprintln!( "[clips-tray] native upload starting recording={recording_id} mode={} bytes={total_bytes} posts={total_posts}", upload_mode.label() @@ -4427,16 +4677,26 @@ async fn upload_prepared_recording_file( let mut file = File::open(&prepared.path).map_err(|e| format!("native recording open failed: {e}"))?; let verification_pending; + let upload_attempt_id = upload_attempt_id.as_deref(); + let upload_generation_id = upload_generation_id.as_deref(); if upload_mode == NativeUploadMode::Streaming { // Resumable providers require every non-final body to be aligned. The // final body may be the unaligned tail; if the file is exactly aligned, // an empty final request closes the session. - for index in 0..streaming_full_chunks { + let resume = streaming_resume.unwrap_or(NativeStreamingResume { + bytes_received: 0, + next_chunk_index: 0, + attempt_id: None, + upload_generation_id: None, + }); + file.seek(SeekFrom::Start(resume.bytes_received)) + .map_err(|e| format!("native recording seek failed: {e}"))?; + for index in resume.next_chunk_index..streaming_full_chunks { let mut buffer = vec![0_u8; UPLOAD_CHUNK_BYTES]; file.read_exact(&mut buffer) .map_err(|e| format!("native recording read failed: {e}"))?; - send_upload_post( + send_upload_post_with_attempt( &client, &server_url, &recording_id, @@ -4454,6 +4714,8 @@ async fn upload_prepared_recording_file( upload_mode, false, None, + upload_attempt_id, + upload_generation_id, buffer, ) .await?; @@ -4479,7 +4741,7 @@ async fn upload_prepared_recording_file( None, Some(streaming_full_chunks as f32 / total_posts as f32), ); - verification_pending = send_upload_post( + verification_pending = send_upload_post_with_attempt( &client, &server_url, &recording_id, @@ -4497,6 +4759,8 @@ async fn upload_prepared_recording_file( upload_mode, prepared.locally_transcoded, Some(total_bytes), + upload_attempt_id, + upload_generation_id, final_body, ) .await?; @@ -4510,7 +4774,7 @@ async fn upload_prepared_recording_file( return Err("Native recording ended before all chunks were read.".into()); } buffer.truncate(read); - send_upload_post( + send_upload_post_with_attempt( &client, &server_url, &recording_id, @@ -4528,6 +4792,8 @@ async fn upload_prepared_recording_file( upload_mode, false, None, + upload_attempt_id, + upload_generation_id, buffer, ) .await?; @@ -4547,7 +4813,7 @@ async fn upload_prepared_recording_file( None, Some(total_chunks as f32 / total_posts as f32), ); - verification_pending = send_upload_post( + verification_pending = send_upload_post_with_attempt( &client, &server_url, &recording_id, @@ -4565,6 +4831,8 @@ async fn upload_prepared_recording_file( upload_mode, prepared.locally_transcoded, Some(total_bytes), + upload_attempt_id, + upload_generation_id, Vec::new(), ) .await?; @@ -4581,13 +4849,470 @@ async fn upload_prepared_recording_file( }) } +async fn get_native_retry_upload_plan( + server_url: &str, + recording_id: &str, + local_bytes: u64, + exact_local_stream: bool, + claimed_attempt_id: &str, + auth_token: &str, + cookie: &str, +) -> Result { + let base = server_url.trim_end_matches('/'); + let mut url = url::Url::parse(&format!("{base}/api/uploads/{recording_id}/resume")) + .map_err(|e| format!("invalid upload resume URL: {e}"))?; + url.query_pairs_mut() + .append_pair("attemptId", claimed_attempt_id); + let client = reqwest::Client::builder() + .timeout(Duration::from_secs(30)) + .build() + .map_err(|e| format!("upload resume client failed: {e}"))?; + let mut request = client + .get(url) + .header("X-Request-Source", "clips-desktop") + .header("Accept", "application/json"); + if !auth_token.trim().is_empty() { + request = request.bearer_auth(auth_token.trim()); + } + if !cookie.trim().is_empty() { + request = request.header("Cookie", cookie.trim()); + } + let response = request + .send() + .await + .map_err(|e| format!("native recording resume check failed: {e}"))?; + let status = response.status(); + let body = response.text().await.unwrap_or_default(); + if !status.is_success() { + return Err(format!( + "native recording resume check returned {status}: {}", + body.chars().take(400).collect::() + )); + } + let response: NativeUploadResumeResponse = serde_json::from_str(&body) + .map_err(|_| "native recording resume check returned an unreadable response".to_string())?; + if response.resumable && response.attempt_id.as_deref() != Some(claimed_attempt_id) { + return Err( + "native recording resume check did not acknowledge its attempt claim".to_string(), + ); + } + Ok(plan_native_retry_upload( + response, + local_bytes, + exact_local_stream, + )) +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct NativeUploadErrorReceipt { + restart_required: Option, + recovery_enabled: Option, +} + +fn is_native_upload_restart_required(status: reqwest::StatusCode, body: &str) -> bool { + status == reqwest::StatusCode::CONFLICT + && serde_json::from_str::(body) + .ok() + .and_then(|receipt| receipt.restart_required) + == Some(true) +} + +fn is_native_upload_unfenced_restart_required(status: reqwest::StatusCode, body: &str) -> bool { + status == reqwest::StatusCode::CONFLICT + && serde_json::from_str::(body) + .ok() + .is_some_and(|receipt| { + receipt.restart_required == Some(true) && receipt.recovery_enabled == Some(false) + }) +} + +fn native_upload_restart_required(result: &Result) -> bool { + matches!(result, Err(error) if error == NATIVE_UPLOAD_RESTART_REQUIRED || error == NATIVE_UPLOAD_UNFENCED_RESTART_REQUIRED) +} + +fn native_replay_attempt_id( + result: &Result, + active_attempt_id: Option<&str>, +) -> Option { + if matches!(result, Err(error) if error == NATIVE_UPLOAD_UNFENCED_RESTART_REQUIRED) { + None + } else { + active_attempt_id.map(str::to_string) + } +} + +fn native_retry_attempt_id() -> String { + let counter = NATIVE_RETRY_CLAIM_COUNTER.fetch_add(1, Ordering::Relaxed); + format!("native-{:x}-{:x}", Utc::now().timestamp_millis(), counter) +} + +fn saved_native_retry_attempt_id(attempt_id: &mut Option) -> String { + if let Some(existing) = attempt_id + .as_deref() + .filter(|existing| !existing.trim().is_empty()) + { + return existing.to_string(); + } + let claimed_attempt_id = native_retry_attempt_id(); + *attempt_id = Some(claimed_attempt_id.clone()); + claimed_attempt_id +} + +fn plan_native_retry_upload( + response: NativeUploadResumeResponse, + local_bytes: u64, + exact_local_stream: bool, +) -> NativeRetryUploadPlan { + if !response.recovery_enabled { + return NativeRetryUploadPlan::Restart { + attempt_id: None, + upload_generation_id: None, + }; + } + + match response.status.as_deref() { + Some("ready") | Some("processing") => { + return NativeRetryUploadPlan::Reconcile; + } + _ => {} + } + + let Some(bytes_received) = response.bytes_received else { + return NativeRetryUploadPlan::Restart { + attempt_id: response.attempt_id, + upload_generation_id: response.upload_generation_id, + }; + }; + let Some(next_chunk_index) = response.next_chunk_index else { + return NativeRetryUploadPlan::Restart { + attempt_id: response.attempt_id, + upload_generation_id: response.upload_generation_id, + }; + }; + let Ok(next_chunk_index) = usize::try_from(next_chunk_index) else { + return NativeRetryUploadPlan::Restart { + attempt_id: response.attempt_id, + upload_generation_id: response.upload_generation_id, + }; + }; + let aligned = bytes_received % UPLOAD_CHUNK_BYTES as u64 == 0; + let consistent_index = next_chunk_index as u64 == bytes_received / UPLOAD_CHUNK_BYTES as u64; + if response.resumable + && response.upload_mode.as_deref() == Some("streaming") + && exact_local_stream + && bytes_received <= local_bytes + && aligned + && consistent_index + { + return NativeRetryUploadPlan::Resume(NativeStreamingResume { + bytes_received, + next_chunk_index, + attempt_id: response.attempt_id, + upload_generation_id: response.upload_generation_id, + }); + } + + NativeRetryUploadPlan::Restart { + attempt_id: response.attempt_id, + upload_generation_id: response.upload_generation_id, + } +} + +async fn interrupt_native_retry_upload( + server_url: &str, + recording_id: &str, + detail: &str, + attempt_id: Option<&str>, + upload_generation_id: Option<&str>, + auth_token: &str, + cookie: &str, +) { + let base = server_url.trim_end_matches('/'); + let Ok(url) = url::Url::parse(&format!("{base}/api/uploads/{recording_id}/interrupt")) else { + return; + }; + let Ok(client) = reqwest::Client::builder() + .timeout(Duration::from_secs(30)) + .build() + else { + return; + }; + let mut request = client + .post(url) + .header("Content-Type", "application/json") + .header("X-Request-Source", "clips-desktop") + .json(&native_retry_interruption_payload( + detail, + attempt_id, + upload_generation_id, + )); + if !auth_token.trim().is_empty() { + request = request.bearer_auth(auth_token.trim()); + } + if !cookie.trim().is_empty() { + request = request.header("Cookie", cookie.trim()); + } + match request.send().await { + Ok(response) if response.status().is_success() => { + eprintln!("[clips-tray] native retry interruption recorded for {recording_id}"); + } + Ok(response) => { + eprintln!( + "[clips-tray] native retry interruption rejected for {recording_id}: {}", + response.status() + ); + } + Err(err) => { + eprintln!( + "[clips-tray] native retry interruption could not be recorded for {recording_id}: {err}" + ); + } + } +} + +fn native_retry_interruption_payload( + detail: &str, + attempt_id: Option<&str>, + upload_generation_id: Option<&str>, +) -> serde_json::Value { + serde_json::json!({ + "detail": detail.chars().take(1000).collect::(), + "attemptId": attempt_id.filter(|value| !value.trim().is_empty()), + "uploadGenerationId": upload_generation_id.filter(|value| !value.trim().is_empty()), + }) +} + +#[cfg(test)] +mod native_retry_upload_plan_tests { + use super::{ + is_native_upload_restart_required, is_native_upload_unfenced_restart_required, + native_replay_attempt_id, native_retry_attempt_id, native_retry_interruption_payload, + plan_native_retry_upload, saved_native_retry_attempt_id, upload_url, + NativeFullscreenUploadResult, NativeRetryUploadPlan, NativeUploadResumeResponse, + NATIVE_UPLOAD_RESTART_REQUIRED, NATIVE_UPLOAD_UNFENCED_RESTART_REQUIRED, + UPLOAD_CHUNK_BYTES, + }; + + fn response(bytes_received: u64, next_chunk_index: u64) -> NativeUploadResumeResponse { + NativeUploadResumeResponse { + resumable: true, + recovery_enabled: true, + status: Some("uploading".to_string()), + upload_mode: Some("streaming".to_string()), + bytes_received: Some(bytes_received), + next_chunk_index: Some(next_chunk_index), + attempt_id: Some("attempt-1".to_string()), + upload_generation_id: Some("generation-1".to_string()), + } + } + + #[test] + fn resumes_only_an_aligned_prefix_of_the_exact_local_stream() { + let plan = plan_native_retry_upload( + response((UPLOAD_CHUNK_BYTES * 2) as u64, 2), + (UPLOAD_CHUNK_BYTES * 3 + 1) as u64, + true, + ); + assert!(matches!( + plan, + NativeRetryUploadPlan::Resume(resume) + if resume.bytes_received == (UPLOAD_CHUNK_BYTES * 2) as u64 + && resume.next_chunk_index == 2 + && resume.attempt_id.as_deref() == Some("attempt-1") + )); + } + + #[test] + fn restarts_for_a_transcoded_or_contradictory_resume_response() { + let transcoded = plan_native_retry_upload( + response(UPLOAD_CHUNK_BYTES as u64, 1), + (UPLOAD_CHUNK_BYTES * 2) as u64, + false, + ); + assert!(matches!(transcoded, NativeRetryUploadPlan::Restart { .. })); + + let contradictory = plan_native_retry_upload( + response(UPLOAD_CHUNK_BYTES as u64, 0), + (UPLOAD_CHUNK_BYTES * 2) as u64, + true, + ); + assert!(matches!( + contradictory, + NativeRetryUploadPlan::Restart { .. } + )); + } + + #[test] + fn restarts_without_a_claim_when_resumable_retry_is_disabled() { + let plan = plan_native_retry_upload( + NativeUploadResumeResponse { + resumable: false, + recovery_enabled: false, + status: None, + upload_mode: None, + bytes_received: None, + next_chunk_index: None, + attempt_id: Some("ignored-attempt".to_string()), + upload_generation_id: Some("ignored-generation".to_string()), + }, + UPLOAD_CHUNK_BYTES as u64, + true, + ); + + assert!(matches!( + plan, + NativeRetryUploadPlan::Restart { + attempt_id: None, + .. + } + )); + } + + #[test] + fn reconciles_terminal_resume_without_an_attempt_echo() { + let terminal = plan_native_retry_upload( + NativeUploadResumeResponse { + resumable: false, + recovery_enabled: true, + status: Some("ready".to_string()), + upload_mode: None, + bytes_received: None, + next_chunk_index: None, + attempt_id: None, + upload_generation_id: None, + }, + UPLOAD_CHUNK_BYTES as u64, + true, + ); + assert!(matches!(terminal, NativeRetryUploadPlan::Reconcile)); + } + + #[test] + fn claimed_restart_carries_its_attempt_id_to_buffered_posts() { + let claimed_attempt_id = native_retry_attempt_id(); + assert!(claimed_attempt_id.len() >= 16); + assert!(claimed_attempt_id + .bytes() + .all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'_' | b'-'))); + + let restart = plan_native_retry_upload( + NativeUploadResumeResponse { + resumable: true, + recovery_enabled: true, + status: Some("uploading".to_string()), + upload_mode: Some("buffered".to_string()), + bytes_received: Some(0), + next_chunk_index: Some(0), + attempt_id: Some(claimed_attempt_id.clone()), + upload_generation_id: Some("generation-1".to_string()), + }, + UPLOAD_CHUNK_BYTES as u64, + true, + ); + assert!(matches!( + restart, + NativeRetryUploadPlan::Restart { ref attempt_id, .. } + if attempt_id.as_deref() == Some(claimed_attempt_id.as_str()) + )); + + let url = upload_url( + "https://clips.example", + "recording-1", + 0, + 2, + false, + None, + "video/mp4", + None, + None, + true, + false, + false, + Some(&claimed_attempt_id), + Some("generation-1"), + ) + .expect("buffered upload URL"); + let query = url::Url::parse(&url) + .expect("valid buffered upload URL") + .query_pairs() + .find(|(key, _)| key == "attemptId") + .map(|(_, value)| value.into_owned()); + assert_eq!(query.as_deref(), Some(claimed_attempt_id.as_str())); + } + + #[test] + fn persists_the_same_retry_claim_across_later_retries() { + let mut saved_attempt_id = None; + let first = saved_native_retry_attempt_id(&mut saved_attempt_id); + let later = saved_native_retry_attempt_id(&mut saved_attempt_id); + assert_eq!(first, later); + assert_eq!(saved_attempt_id.as_deref(), Some(first.as_str())); + } + + #[test] + fn reports_retry_interruptions_with_or_without_a_fencing_claim() { + let unfenced = native_retry_interruption_payload("upload failed", None, None); + assert_eq!(unfenced["detail"], "upload failed"); + assert!(unfenced["attemptId"].is_null()); + + let fenced = native_retry_interruption_payload( + "upload failed", + Some("attempt-1"), + Some("generation-1"), + ); + assert_eq!(fenced["attemptId"], "attempt-1"); + assert_eq!(fenced["uploadGenerationId"], "generation-1"); + } + + #[test] + fn recognizes_only_the_structured_conflict_restart_signal() { + assert!(is_native_upload_restart_required( + reqwest::StatusCode::CONFLICT, + r#"{"restartRequired":true}"#, + )); + assert!(!is_native_upload_restart_required( + reqwest::StatusCode::CONFLICT, + r#"{"restartRequired":false}"#, + )); + assert!(!is_native_upload_restart_required( + reqwest::StatusCode::BAD_REQUEST, + r#"{"restartRequired":true}"#, + )); + assert!(is_native_upload_unfenced_restart_required( + reqwest::StatusCode::CONFLICT, + r#"{"restartRequired":true,"recoveryEnabled":false}"#, + )); + assert!(!is_native_upload_unfenced_restart_required( + reqwest::StatusCode::CONFLICT, + r#"{"restartRequired":true}"#, + )); + } + + #[test] + fn drops_the_retry_claim_for_a_feature_disabled_replay() { + let disabled: Result = + Err(NATIVE_UPLOAD_UNFENCED_RESTART_REQUIRED.to_string()); + assert_eq!(native_replay_attempt_id(&disabled, Some("attempt-1")), None); + + let expired: Result = + Err(NATIVE_UPLOAD_RESTART_REQUIRED.to_string()); + assert_eq!( + native_replay_attempt_id(&expired, Some("attempt-1")).as_deref(), + Some("attempt-1") + ); + } +} + async fn reset_upload_chunks( server_url: &str, recording_id: &str, mime_type: &str, + attempt_id: Option<&str>, + upload_generation_id: Option<&str>, auth_token: &str, cookie: &str, -) -> Result { +) -> Result { let base = server_url.trim_end_matches('/'); let url = url::Url::parse(&format!("{base}/api/uploads/{recording_id}/reset-chunks")) .map_err(|e| format!("invalid reset URL: {e}"))?; @@ -4602,6 +5327,8 @@ async fn reset_upload_chunks( .json(&serde_json::json!({ "requestStreaming": true, "mimeType": mime_type, + "attemptId": attempt_id, + "uploadGenerationId": upload_generation_id, })); let trimmed_token = auth_token.trim(); if !trimmed_token.is_empty() { @@ -4624,7 +5351,12 @@ async fn reset_upload_chunks( body.chars().take(400).collect::() )); } - Ok(NativeUploadMode::from_reset_response(&body)) + let reset: NativeUploadResetResponse = serde_json::from_str(&body) + .map_err(|_| "native recording retry setup returned an unreadable response".to_string())?; + if attempt_id.is_some() && reset.upload_generation_id.is_none() { + return Err("native recording retry setup returned no upload generation".to_string()); + } + Ok(reset) } async fn send_upload_post( @@ -4646,6 +5378,53 @@ async fn send_upload_post( locally_transcoded: bool, expected_source_bytes: Option, body: Vec, +) -> Result { + send_upload_post_with_attempt( + client, + server_url, + recording_id, + auth_token, + cookie, + index, + total, + is_final, + duration_ms, + mime_type, + width, + height, + has_audio, + has_camera, + upload_mode, + locally_transcoded, + expected_source_bytes, + None, + None, + body, + ) + .await +} + +async fn send_upload_post_with_attempt( + client: &reqwest::Client, + server_url: &str, + recording_id: &str, + auth_token: &str, + cookie: &str, + index: usize, + total: usize, + is_final: bool, + duration_ms: Option, + mime_type: &str, + width: Option, + height: Option, + has_audio: bool, + has_camera: bool, + upload_mode: NativeUploadMode, + locally_transcoded: bool, + expected_source_bytes: Option, + attempt_id: Option<&str>, + upload_generation_id: Option<&str>, + body: Vec, ) -> Result { let body_len = body.len(); let url = upload_url( @@ -4661,6 +5440,8 @@ async fn send_upload_post( has_audio, has_camera, locally_transcoded, + attempt_id, + upload_generation_id, )?; eprintln!( "[clips-tray] native upload post start recording={recording_id} mode={} index={index}/{total} final={is_final} bytes={body_len}", @@ -4690,6 +5471,15 @@ async fn send_upload_post( "[live-upload] POST chunk #{index} (final={is_final}) -> {status} for {recording_id}" ); if !status.is_success() { + if is_native_upload_restart_required(status, &body) { + return Err( + if is_native_upload_unfenced_restart_required(status, &body) { + NATIVE_UPLOAD_UNFENCED_RESTART_REQUIRED.to_string() + } else { + NATIVE_UPLOAD_RESTART_REQUIRED.to_string() + }, + ); + } return Err(format!( "native recording upload returned {status}: {}", body.chars().take(400).collect::() @@ -4825,6 +5615,8 @@ fn upload_url( has_audio: bool, has_camera: bool, locally_transcoded: bool, + attempt_id: Option<&str>, + upload_generation_id: Option<&str>, ) -> Result { let base = server_url.trim_end_matches('/'); let mut url = url::Url::parse(&format!("{base}/api/uploads/{recording_id}/chunk")) @@ -4850,6 +5642,14 @@ fn upload_url( if is_final && locally_transcoded { query.append_pair("locallyTranscoded", "1"); } + if let Some(attempt_id) = attempt_id.filter(|value| !value.trim().is_empty()) { + query.append_pair("attemptId", attempt_id); + } + if let Some(upload_generation_id) = + upload_generation_id.filter(|value| !value.trim().is_empty()) + { + query.append_pair("uploadGenerationId", upload_generation_id); + } } Ok(url.to_string()) } diff --git a/templates/clips/desktop/src/app.tsx b/templates/clips/desktop/src/app.tsx index 77ac68c027..9be3594b0e 100644 --- a/templates/clips/desktop/src/app.tsx +++ b/templates/clips/desktop/src/app.tsx @@ -912,6 +912,9 @@ export function App() { [], ); const [retryingUploadId, setRetryingUploadId] = useState(null); + const [retryingUploadStatus, setRetryingUploadStatus] = useState< + string | null + >(null); const [exportingUploadId, setExportingUploadId] = useState( null, ); @@ -2635,6 +2638,15 @@ export function App() { recordingId: upload.recordingId, serverUrl: targetServerUrl, authToken, + onRecoveryDecision: ({ action, progress }) => { + setRetryingUploadStatus( + action === "resume" + ? `Resuming · ${Math.round(progress * 100)}% already uploaded` + : action === "restart" + ? "Restarting upload" + : "Finishing upload", + ); + }, }); } await loadPendingUploads(); @@ -2655,6 +2667,7 @@ export function App() { await loadPendingUploads(); } finally { setRetryingUploadId(null); + setRetryingUploadStatus(null); } } @@ -3281,6 +3294,7 @@ export function App() { void }) { function PendingUploadBanner({ uploads, retryingUploadId, + retryingUploadStatus, exportingUploadId, dismissingUploadId, onExport, @@ -4063,6 +4078,7 @@ function PendingUploadBanner({ }: { uploads: PendingDesktopUpload[]; retryingUploadId: string | null; + retryingUploadStatus: string | null; exportingUploadId: string | null; dismissingUploadId: string | null; onExport: (upload: PendingDesktopUpload) => void; @@ -4185,7 +4201,7 @@ function PendingUploadBanner({ onClick={() => onRetry(latest)} > - {retrying ? "Retrying" : "Retry"} + {retrying ? (retryingUploadStatus ?? "Retrying") : "Retry"} )} diff --git a/templates/clips/desktop/src/lib/recorder.ts b/templates/clips/desktop/src/lib/recorder.ts index ead61d9717..b0db61dcea 100644 --- a/templates/clips/desktop/src/lib/recorder.ts +++ b/templates/clips/desktop/src/lib/recorder.ts @@ -88,6 +88,13 @@ import { type CapturedTranscript, type TranscriptionCapture, } from "./transcription-capture"; +import { + buildStreamingReplayPlan, + planStreamingRecovery, + retryAttemptIdAfterRestartSignal, + retryAttemptIdAfterResumeResponse, + type UploadResumeResponse, +} from "./upload-recovery"; import { parseFinalizeReceipt, verifyFinalizeReceipt, @@ -310,6 +317,7 @@ export interface PendingBrowserRecordingUpload { retryCount: number; chunkCount: number; mimeType: string; + uploadAttemptId?: string | null; } function streamFromTracks(tracks: MediaStreamTrack[]): MediaStream { @@ -983,6 +991,16 @@ function buildRetryHeaders(mimeType: string, authToken?: string): Headers { return headers; } +class UploadRestartRequiredError extends Error { + constructor( + message: string, + readonly recoveryEnabled?: boolean, + ) { + super(message); + this.name = "UploadRestartRequiredError"; + } +} + async function postBackupChunk( url: string, blob: Blob, @@ -999,6 +1017,24 @@ async function postBackupChunk( }); const body = await res.text().catch(() => ""); if (!res.ok) { + const details = (() => { + try { + return JSON.parse(body) as { + restartRequired?: unknown; + recoveryEnabled?: unknown; + }; + } catch { + return null; + } + })(); + if (details?.restartRequired === true) { + throw new UploadRestartRequiredError( + `The prior upload session expired (${res.status})`, + typeof details.recoveryEnabled === "boolean" + ? details.recoveryEnabled + : undefined, + ); + } throw new Error( `Upload retry failed (${res.status}): ${body.slice(0, 200)}`, ); @@ -1009,7 +1045,9 @@ async function postBackupChunk( async function resetBrowserRecordingBackupUpload( meta: BrowserRecordingBackupMeta, authToken?: string, -): Promise { + attemptId?: string, + uploadGenerationId?: string, +): Promise<{ uploadMode: UploadMode; uploadGenerationId?: string }> { const res = await fetch( `${meta.serverUrl.replace(/\/+$/, "")}/api/uploads/${meta.recordingId}/reset-chunks`, { @@ -1023,6 +1061,8 @@ async function resetBrowserRecordingBackupUpload( body: JSON.stringify({ requestStreaming: true, mimeType: meta.mimeType, + ...(attemptId ? { attemptId } : {}), + ...(uploadGenerationId ? { uploadGenerationId } : {}), }), }, ); @@ -1034,14 +1074,61 @@ async function resetBrowserRecordingBackupUpload( } const body = (await res.json().catch(() => null)) as { uploadMode?: unknown; + uploadGenerationId?: unknown; } | null; - return body?.uploadMode === "streaming" ? "streaming" : "buffered"; + if (attemptId && typeof body?.uploadGenerationId !== "string") { + throw new Error("Upload retry setup returned no upload generation"); + } + return { + uploadMode: body?.uploadMode === "streaming" ? "streaming" : "buffered", + ...(typeof body?.uploadGenerationId === "string" + ? { uploadGenerationId: body.uploadGenerationId } + : {}), + }; +} + +async function getBrowserRecordingUploadResume( + meta: BrowserRecordingBackupMeta, + attemptId: string, + authToken?: string, +): Promise { + const resumeUrl = new URL( + `${meta.serverUrl.replace(/\/+$/, "")}/api/uploads/${meta.recordingId}/resume`, + ); + resumeUrl.searchParams.set("attemptId", attemptId); + const res = await fetch(resumeUrl, { + method: "GET", + headers: buildRetryHeaders("application/json", authToken), + credentials: "include", + }); + const body = await res.text().catch(() => ""); + if (!res.ok) { + throw new Error( + `Upload resume check failed (${res.status}): ${body.slice(0, 200)}`, + ); + } + let parsed: UploadResumeResponse; + try { + parsed = JSON.parse(body) as UploadResumeResponse; + } catch { + throw new Error("Upload resume check returned an unreadable response"); + } + if (parsed.resumable && parsed.attemptId !== attemptId) { + throw new Error("Upload resume check returned a mismatched retry token"); + } + return parsed; } async function replayBrowserBackupToResumableSession( meta: BrowserRecordingBackupMeta, chunks: BrowserRecordingBackupChunk[], authToken?: string, + attemptId?: string, + uploadGenerationId?: string, + resumeFrom: { bytesReceived: number; nextChunkIndex: number } = { + bytesReceived: 0, + nextChunkIndex: 0, + }, ): Promise { // The backup is stored in raw MediaRecorder blobs, which have arbitrary // boundaries. A resumable provider needs every non-final request aligned, @@ -1056,33 +1143,38 @@ async function replayBrowserBackupToResumableSession( throw new Error("Local recording backup is empty"); } - const fullChunks = Math.floor(recording.size / STREAM_CHUNK_BYTES); - const totalPosts = fullChunks + 1; - let offset = 0; + const replayPlan = buildStreamingReplayPlan({ + localBytes: recording.size, + chunkBytes: STREAM_CHUNK_BYTES, + ...resumeFrom, + }); + const totalPosts = Math.floor(recording.size / STREAM_CHUNK_BYTES) + 1; - for (let index = 0; index < fullChunks; index += 1) { - const body = recording.slice( - offset, - offset + STREAM_CHUNK_BYTES, - meta.mimeType, - ); + for (const request of replayPlan.filter((item) => !item.final)) { + const body = recording.slice(request.start, request.end, meta.mimeType); await postBackupChunk( - chunkUrl(meta.serverUrl, meta.recordingId, index, false, { + chunkUrl(meta.serverUrl, meta.recordingId, request.index, false, { total: String(totalPosts), mimeType: meta.mimeType, + ...(attemptId ? { attemptId } : {}), + ...(uploadGenerationId ? { uploadGenerationId } : {}), }), body, authToken, ); - offset += STREAM_CHUNK_BYTES; } // The final post always closes the session. When the file is exactly // aligned it is intentionally empty; the route sends the provider's close // request with the bytes committed by the previous chunks. - const finalBody = recording.slice(offset, recording.size, meta.mimeType); + const finalRequest = replayPlan[replayPlan.length - 1]!; + const finalBody = recording.slice( + finalRequest.start, + finalRequest.end, + meta.mimeType, + ); return postBackupChunk( - chunkUrl(meta.serverUrl, meta.recordingId, fullChunks, true, { + chunkUrl(meta.serverUrl, meta.recordingId, finalRequest.index, true, { total: String(totalPosts), mimeType: meta.mimeType, durationMs: String(Math.round(meta.durationMs || 0)), @@ -1090,6 +1182,8 @@ async function replayBrowserBackupToResumableSession( ...(meta.height ? { height: String(meta.height) } : {}), hasAudio: meta.hasAudio ? "1" : "0", hasCamera: meta.hasCamera ? "1" : "0", + ...(attemptId ? { attemptId } : {}), + ...(uploadGenerationId ? { uploadGenerationId } : {}), }), finalBody, authToken, @@ -1100,6 +1194,10 @@ export async function retryBrowserRecordingBackup(input: { recordingId: string; serverUrl?: string; authToken?: string; + onRecoveryDecision?: (decision: { + action: "resume" | "restart" | "reconcile"; + progress: number; + }) => void; }): Promise<{ recordingId: string; viewUrl: string }> { let meta = await getBrowserRecordingBackupMeta(input.recordingId); if (!meta) { @@ -1111,57 +1209,165 @@ export async function retryBrowserRecordingBackup(input: { } const chunks = await getBrowserRecordingBackupChunks(input.recordingId); const validatedChunks = validateBrowserRecordingBackupChunks(meta, chunks); + let activeAttemptId: string | undefined = + meta.uploadAttemptId || crypto.randomUUID(); + let activeUploadGenerationId: string | undefined; try { await putBrowserRecordingBackupMeta({ ...meta, + uploadAttemptId: activeAttemptId, lastAttemptAt: new Date().toISOString(), lastError: null, }); - const uploadMode = await resetBrowserRecordingBackupUpload( + const recordingBytes = validatedChunks.reduce( + (total, chunk) => total + chunk.blob.size, + 0, + ); + const resumeResponse = await getBrowserRecordingUploadResume( meta, + activeAttemptId, input.authToken, ); + const recoveryPlan = planStreamingRecovery({ + response: resumeResponse, + localBytes: recordingBytes, + chunkBytes: STREAM_CHUNK_BYTES, + }); + activeAttemptId = retryAttemptIdAfterResumeResponse( + activeAttemptId, + resumeResponse, + ); + activeUploadGenerationId = resumeResponse.resumable + ? resumeResponse.uploadGenerationId + : undefined; + if (recoveryPlan.action === "reconcile") { + input.onRecoveryDecision?.({ action: "reconcile", progress: 1 }); + if ( + await recoverAcceptedRecordingAfterFinalizeError({ + serverUrl: meta.serverUrl, + recordingId: meta.recordingId, + authToken: input.authToken, + }) + ) { + return { + recordingId: meta.recordingId, + viewUrl: `/r/${meta.recordingId}`, + }; + } + throw new Error( + `Upload is ${recoveryPlan.status}, but its accepted media could not be verified`, + ); + } + + let uploadMode: UploadMode = "streaming"; + let resumeFrom = { bytesReceived: 0, nextChunkIndex: 0 }; + if (recoveryPlan.action === "resume") { + resumeFrom = recoveryPlan; + input.onRecoveryDecision?.({ + action: "resume", + progress: recoveryPlan.progress, + }); + console.info("[clips-recorder] resuming saved upload", { + recordingId: meta.recordingId, + localBytes: recordingBytes, + serverAcknowledgedBytes: recoveryPlan.bytesReceived, + nextChunkIndex: recoveryPlan.nextChunkIndex, + progress: recoveryPlan.progress, + }); + } else { + input.onRecoveryDecision?.({ action: "restart", progress: 0 }); + console.info("[clips-recorder] restarting saved upload", { + recordingId: meta.recordingId, + localBytes: recordingBytes, + reason: recoveryPlan.reason, + }); + const reset = await resetBrowserRecordingBackupUpload( + meta, + input.authToken, + activeAttemptId, + activeUploadGenerationId, + ); + uploadMode = reset.uploadMode; + activeUploadGenerationId = reset.uploadGenerationId; + } if (uploadMode === "streaming") { + let receipt: FinalizeReceipt | null = null; try { - const receipt = await replayBrowserBackupToResumableSession( + receipt = await replayBrowserBackupToResumableSession( meta, validatedChunks, input.authToken, + activeAttemptId, + activeUploadGenerationId, + resumeFrom, ); - const receiptStatus = verifyFinalizeReceipt(receipt, meta); - if (receiptStatus === "processing") { - scheduleBrowserBackupCleanupAfterProcessing({ + } catch (err) { + if (err instanceof UploadRestartRequiredError) { + activeAttemptId = retryAttemptIdAfterRestartSignal( + activeAttemptId, + err.recoveryEnabled, + ); + if (err.recoveryEnabled === false) { + activeUploadGenerationId = undefined; + } + input.onRecoveryDecision?.({ action: "restart", progress: 0 }); + console.info("[clips-recorder] restarting expired upload session", { + recordingId: meta.recordingId, + localBytes: recordingBytes, + }); + const reset = await resetBrowserRecordingBackupUpload( + meta, + input.authToken, + activeAttemptId, + activeUploadGenerationId, + ); + uploadMode = reset.uploadMode; + activeUploadGenerationId = reset.uploadGenerationId; + if (uploadMode === "streaming") { + receipt = await replayBrowserBackupToResumableSession( + meta, + validatedChunks, + input.authToken, + activeAttemptId, + activeUploadGenerationId, + ); + } + } else if ( + await recoverAcceptedRecordingAfterFinalizeError({ serverUrl: meta.serverUrl, recordingId: meta.recordingId, authToken: input.authToken, - }); + }) + ) { return { recordingId: meta.recordingId, viewUrl: `/r/${meta.recordingId}`, }; + } else { + throw err; } - } catch (err) { - if ( - await recoverAcceptedRecordingAfterFinalizeError({ + } + if (uploadMode === "streaming") { + const receiptStatus = verifyFinalizeReceipt(receipt, meta); + if (receiptStatus === "processing") { + scheduleBrowserBackupCleanupAfterProcessing({ serverUrl: meta.serverUrl, recordingId: meta.recordingId, authToken: input.authToken, - }) - ) { + }); return { recordingId: meta.recordingId, viewUrl: `/r/${meta.recordingId}`, }; } - throw err; + await deleteBrowserRecordingBackup(meta.recordingId); + return { + recordingId: meta.recordingId, + viewUrl: `/r/${meta.recordingId}`, + }; } - await deleteBrowserRecordingBackup(meta.recordingId); - return { - recordingId: meta.recordingId, - viewUrl: `/r/${meta.recordingId}`, - }; } const totalPosts = validatedChunks.length + 1; @@ -1170,6 +1376,10 @@ export async function retryBrowserRecordingBackup(input: { chunkUrl(meta.serverUrl, meta.recordingId, chunk.index, false, { total: String(totalPosts), mimeType: meta.mimeType, + ...(activeAttemptId ? { attemptId: activeAttemptId } : {}), + ...(activeUploadGenerationId + ? { uploadGenerationId: activeUploadGenerationId } + : {}), }), chunk.blob, input.authToken, @@ -1189,6 +1399,10 @@ export async function retryBrowserRecordingBackup(input: { ...(meta.height ? { height: String(meta.height) } : {}), hasAudio: meta.hasAudio ? "1" : "0", hasCamera: meta.hasCamera ? "1" : "0", + ...(activeAttemptId ? { attemptId: activeAttemptId } : {}), + ...(activeUploadGenerationId + ? { uploadGenerationId: activeUploadGenerationId } + : {}), }, ); try { @@ -1229,9 +1443,29 @@ export async function retryBrowserRecordingBackup(input: { return { recordingId: meta.recordingId, viewUrl: `/r/${meta.recordingId}` }; } catch (err) { const message = err instanceof Error ? err.message : String(err); + if ( + await recoverAcceptedRecordingAfterFinalizeError({ + serverUrl: meta.serverUrl, + recordingId: meta.recordingId, + authToken: input.authToken, + }) + ) { + return { + recordingId: meta.recordingId, + viewUrl: `/r/${meta.recordingId}`, + }; + } await markBrowserRecordingBackupError(meta.recordingId, message).catch( () => {}, ); + await interruptRecordingUpload( + meta.serverUrl, + meta.recordingId, + message, + input.authToken, + activeAttemptId, + activeUploadGenerationId, + ); throw err; } } @@ -1610,6 +1844,55 @@ async function abortRecordingUpload( } } +async function interruptRecordingUpload( + serverUrl: string, + recordingId: string, + detail: string, + authToken?: string, + attemptId?: string, + uploadGenerationId?: string, +): Promise { + let lastError: unknown = null; + for (let attempt = 1; attempt <= CHUNK_UPLOAD_MAX_ATTEMPTS; attempt += 1) { + try { + const res = await fetch( + `${serverUrl.replace(/\/+$/, "")}/api/uploads/${recordingId}/interrupt`, + { + method: "POST", + headers: buildRetryHeaders("application/json", authToken), + credentials: "include", + body: JSON.stringify({ + detail, + ...(attemptId ? { attemptId } : {}), + ...(uploadGenerationId ? { uploadGenerationId } : {}), + }), + }, + ); + if (res.ok) { + console.info("[clips-recorder] upload interruption recorded", { + recordingId, + attempt, + }); + return; + } + const body = await res.text().catch(() => ""); + lastError = new Error( + `Upload interruption failed (${res.status}): ${body.slice(0, 200)}`, + ); + if (!isRetriableChunkStatus(res.status)) break; + } catch (err) { + lastError = err; + } + if (attempt < CHUNK_UPLOAD_MAX_ATTEMPTS) { + await wait(CHUNK_UPLOAD_RETRY_BASE_MS * 2 ** (attempt - 1)); + } + } + console.warn( + "[clips-recorder] upload interruption could not be recorded:", + lastError, + ); +} + async function trashRecording( serverUrl: string, recordingId: string, @@ -2427,10 +2710,11 @@ async function tryStartRewindFullscreenRecording( ) { return { recordingId: id, viewUrl }; } - await abortRecordingUpload( + await interruptRecordingUpload( params.serverUrl, id, err instanceof Error ? err.message : String(err), + params.authToken, ); throw err; } @@ -3101,10 +3385,11 @@ async function startNativeFullscreenRecording( ) { return { recordingId: id, viewUrl }; } - await abortRecordingUpload( + await interruptRecordingUpload( params.serverUrl, id, err instanceof Error ? err.message : String(err), + params.authToken, ); throw err; } @@ -4453,7 +4738,12 @@ async function startRecordingInner( await markBrowserRecordingBackupError(id, failed.message).catch( () => {}, ); - await abortRecordingUpload(params.serverUrl, id, failed.message); + await interruptRecordingUpload( + params.serverUrl, + id, + failed.message, + params.authToken, + ); } finally { await clearRecordingState(); await publishFinalizingResult({ @@ -4559,7 +4849,12 @@ async function startRecordingInner( await markBrowserRecordingBackupError(id, error.message).catch( () => {}, ); - await abortRecordingUpload(params.serverUrl, id, error.message); + await interruptRecordingUpload( + params.serverUrl, + id, + error.message, + params.authToken, + ); throw error; } await deleteBrowserRecordingBackup(id).catch((err) => { diff --git a/templates/clips/desktop/src/lib/upload-recovery.test.ts b/templates/clips/desktop/src/lib/upload-recovery.test.ts new file mode 100644 index 0000000000..eb9dd3d301 --- /dev/null +++ b/templates/clips/desktop/src/lib/upload-recovery.test.ts @@ -0,0 +1,194 @@ +import { describe, expect, it } from "vitest"; + +import { + buildStreamingReplayPlan, + planStreamingRecovery, + retryAttemptIdAfterRestartSignal, + retryAttemptIdAfterResumeResponse, +} from "./upload-recovery"; + +const CHUNK_BYTES = 3_932_160; + +describe("retryAttemptIdAfterRestartSignal", () => { + it("drops the retry claim only when the server disables recovery", () => { + expect( + retryAttemptIdAfterRestartSignal("attempt-1", false), + ).toBeUndefined(); + expect(retryAttemptIdAfterRestartSignal("attempt-1", true)).toBe( + "attempt-1", + ); + expect(retryAttemptIdAfterRestartSignal("attempt-1", undefined)).toBe( + "attempt-1", + ); + }); +}); + +describe("retryAttemptIdAfterResumeResponse", () => { + it("keeps only a server-acknowledged retry claim", () => { + expect( + retryAttemptIdAfterResumeResponse("attempt-1", { + resumable: true, + recoveryEnabled: true, + status: "uploading", + uploadMode: "streaming", + attemptId: "attempt-1", + uploadGenerationId: "generation-1", + bytesReceived: 0, + nextChunkIndex: 0, + }), + ).toBe("attempt-1"); + }); + + it("drops a local claim when the server did not acknowledge it", () => { + expect( + retryAttemptIdAfterResumeResponse("attempt-1", { + resumable: false, + recoveryEnabled: true, + status: "failed", + }), + ).toBeUndefined(); + }); +}); + +describe("planStreamingRecovery", () => { + it("resumes from an aligned authoritative offset", () => { + expect( + planStreamingRecovery({ + response: { + resumable: true, + recoveryEnabled: true, + status: "uploading", + uploadMode: "streaming", + attemptId: "attempt-1", + uploadGenerationId: "generation-1", + bytesReceived: CHUNK_BYTES * 2, + nextChunkIndex: 2, + }, + localBytes: CHUNK_BYTES * 5, + chunkBytes: CHUNK_BYTES, + }), + ).toEqual({ + action: "resume", + bytesReceived: CHUNK_BYTES * 2, + nextChunkIndex: 2, + progress: 0.4, + }); + }); + + it.each([ + [CHUNK_BYTES + 1, 1, "server chunk offset is inconsistent"], + [CHUNK_BYTES * 2, 1, "server chunk offset is inconsistent"], + [CHUNK_BYTES * 6, 6, "server byte offset is invalid"], + ])( + "restarts when offset %s and index %s contradict the local backup", + (bytesReceived, nextChunkIndex, reason) => { + expect( + planStreamingRecovery({ + response: { + resumable: true, + recoveryEnabled: true, + status: "uploading", + uploadMode: "streaming", + attemptId: "attempt-1", + uploadGenerationId: "generation-1", + bytesReceived, + nextChunkIndex, + }, + localBytes: CHUNK_BYTES * 5, + chunkBytes: CHUNK_BYTES, + }), + ).toEqual({ action: "restart", reason }); + }, + ); + + it("restarts explicitly when the session is unavailable", () => { + expect( + planStreamingRecovery({ + response: { + resumable: false, + recoveryEnabled: true, + status: "failed", + reason: "missing_session", + }, + localBytes: CHUNK_BYTES, + chunkBytes: CHUNK_BYTES, + }), + ).toEqual({ action: "restart", reason: "missing_session" }); + }); + + it("reconciles a response lost after the server accepted finalization", () => { + expect( + planStreamingRecovery({ + response: { + resumable: false, + recoveryEnabled: true, + status: "processing", + }, + localBytes: CHUNK_BYTES, + chunkBytes: CHUNK_BYTES, + }), + ).toEqual({ action: "reconcile", status: "processing" }); + }); + + it("uses the full-restart control path while the rollout flag is off", () => { + expect( + planStreamingRecovery({ + response: { + resumable: false, + recoveryEnabled: false, + status: null, + reason: "feature_disabled", + }, + localBytes: CHUNK_BYTES, + chunkBytes: CHUNK_BYTES, + }), + ).toEqual({ + action: "restart", + reason: "resumable retry is disabled", + }); + }); +}); + +describe("buildStreamingReplayPlan", () => { + it("sends only the suffix after the acknowledged prefix", () => { + expect( + buildStreamingReplayPlan({ + localBytes: CHUNK_BYTES * 3 + 500, + chunkBytes: CHUNK_BYTES, + bytesReceived: CHUNK_BYTES * 2, + nextChunkIndex: 2, + }), + ).toEqual([ + { + index: 2, + start: CHUNK_BYTES * 2, + end: CHUNK_BYTES * 3, + final: false, + }, + { + index: 3, + start: CHUNK_BYTES * 3, + end: CHUNK_BYTES * 3 + 500, + final: true, + }, + ]); + }); + + it("sends only the close sentinel when all aligned bytes are acknowledged", () => { + expect( + buildStreamingReplayPlan({ + localBytes: CHUNK_BYTES * 2, + chunkBytes: CHUNK_BYTES, + bytesReceived: CHUNK_BYTES * 2, + nextChunkIndex: 2, + }), + ).toEqual([ + { + index: 2, + start: CHUNK_BYTES * 2, + end: CHUNK_BYTES * 2, + final: true, + }, + ]); + }); +}); diff --git a/templates/clips/desktop/src/lib/upload-recovery.ts b/templates/clips/desktop/src/lib/upload-recovery.ts new file mode 100644 index 0000000000..a8f160ee82 --- /dev/null +++ b/templates/clips/desktop/src/lib/upload-recovery.ts @@ -0,0 +1,129 @@ +export type UploadResumeResponse = + | { + resumable: true; + recoveryEnabled: boolean; + status: string; + uploadMode: "streaming" | "buffered"; + attemptId: string; + uploadGenerationId?: string; + bytesReceived: number; + nextChunkIndex: number; + } + | { + resumable: false; + recoveryEnabled: boolean; + status: string | null; + failureReason?: string | null; + videoUrl?: string | null; + reason?: string; + attemptId?: string; + }; + +export type StreamingRecoveryPlan = + | { + action: "resume"; + bytesReceived: number; + nextChunkIndex: number; + progress: number; + } + | { action: "reconcile"; status: "ready" | "processing" } + | { action: "restart"; reason: string }; + +export interface StreamingReplayRequest { + index: number; + start: number; + end: number; + final: boolean; +} + +export function retryAttemptIdAfterRestartSignal( + attemptId: string | undefined, + recoveryEnabled: unknown, +): string | undefined { + return recoveryEnabled === false ? undefined : attemptId; +} + +export function retryAttemptIdAfterResumeResponse( + attemptId: string | undefined, + response: UploadResumeResponse, +): string | undefined { + return response.resumable && response.attemptId === attemptId + ? attemptId + : undefined; +} + +export function buildStreamingReplayPlan(input: { + localBytes: number; + chunkBytes: number; + bytesReceived: number; + nextChunkIndex: number; +}): StreamingReplayRequest[] { + const { localBytes, chunkBytes, bytesReceived, nextChunkIndex } = input; + const fullChunks = Math.floor(localBytes / chunkBytes); + const requests: StreamingReplayRequest[] = []; + let offset = bytesReceived; + for (let index = nextChunkIndex; index < fullChunks; index += 1) { + requests.push({ + index, + start: offset, + end: offset + chunkBytes, + final: false, + }); + offset += chunkBytes; + } + requests.push({ + index: fullChunks, + start: offset, + end: localBytes, + final: true, + }); + return requests; +} + +export function planStreamingRecovery(input: { + response: UploadResumeResponse; + localBytes: number; + chunkBytes: number; +}): StreamingRecoveryPlan { + const { response, localBytes, chunkBytes } = input; + if (!response.recoveryEnabled) { + return { action: "restart", reason: "resumable retry is disabled" }; + } + if (response.status === "ready" || response.status === "processing") { + return { action: "reconcile", status: response.status }; + } + if (!response.resumable) { + return { + action: "restart", + reason: response.reason ?? "server session is unavailable", + }; + } + if (response.uploadMode !== "streaming") { + return { action: "restart", reason: "server session is not streaming" }; + } + if (!response.attemptId) { + return { action: "restart", reason: "server retry token is missing" }; + } + const { bytesReceived, nextChunkIndex } = response; + if ( + !Number.isSafeInteger(bytesReceived) || + bytesReceived < 0 || + bytesReceived > localBytes + ) { + return { action: "restart", reason: "server byte offset is invalid" }; + } + if ( + !Number.isSafeInteger(nextChunkIndex) || + nextChunkIndex < 0 || + bytesReceived % chunkBytes !== 0 || + nextChunkIndex !== bytesReceived / chunkBytes + ) { + return { action: "restart", reason: "server chunk offset is inconsistent" }; + } + return { + action: "resume", + bytesReceived, + nextChunkIndex, + progress: localBytes > 0 ? bytesReceived / localBytes : 0, + }; +} diff --git a/templates/clips/server/db/schema.ts b/templates/clips/server/db/schema.ts index 212b0ef363..4cee17f237 100644 --- a/templates/clips/server/db/schema.ts +++ b/templates/clips/server/db/schema.ts @@ -168,6 +168,13 @@ export const recordings = table("recordings", { // Authoritative liveness for an in-flight upload: renewed by every chunk // POST, and the only thing the upload reaper is allowed to consult. uploadLeaseExpiresAt: text("upload_lease_expires_at"), + // Fences resumed writers: every recovery claim rotates this token so stale + // chunks and delayed interruption callbacks cannot mutate the new attempt. + uploadAttemptId: text("upload_attempt_id"), + // Every destructive restart receives a new generation. Unlike the attempt + // id (which is deliberately stable across a lost response), this fences the + // provider handle and buffered scratch that the restart replaces. + uploadGenerationId: text("upload_generation_id"), failureReason: text("failure_reason"), loomImportClaimId: text("loom_import_claim_id"), loomImportClaimedAt: text("loom_import_claimed_at"), diff --git a/templates/clips/server/jobs/media-verification.test.ts b/templates/clips/server/jobs/media-verification.test.ts index 2149c869af..c2f21b719d 100644 --- a/templates/clips/server/jobs/media-verification.test.ts +++ b/templates/clips/server/jobs/media-verification.test.ts @@ -7,7 +7,11 @@ const mockRunWithRequestContext = vi.hoisted(() => ); const mockOwnerEmailMatches = vi.hoisted(() => vi.fn()); const mockRecordingRows = vi.hoisted(() => ({ - rows: [] as Array<{ ownerEmail: string; orgId: string | null }>, + rows: [] as Array<{ + ownerEmail: string; + orgId: string | null; + uploadGenerationId?: string | null; + }>, })); const mockLimit = vi.hoisted(() => vi.fn(async () => mockRecordingRows.rows)); const mockWhere = vi.hoisted(() => vi.fn(() => ({ limit: mockLimit }))); @@ -38,6 +42,7 @@ vi.mock("../db/index.js", () => ({ orgId: "recordings.orgId", status: "recordings.status", trashedAt: "recordings.trashedAt", + uploadGenerationId: "recordings.uploadGenerationId", }, }, })); @@ -70,7 +75,11 @@ describe("media verification recovery sweep", () => { mockFinalizeRun.mockResolvedValue({ status: "processing" }); mockOwnerEmailMatches.mockReturnValue("owner-match"); mockRecordingRows.rows = [ - { ownerEmail: "owner@example.com", orgId: "org-1" }, + { + ownerEmail: "owner@example.com", + orgId: "org-1", + uploadGenerationId: "generation-1", + }, ]; }); @@ -101,6 +110,7 @@ describe("media verification recovery sweep", () => { expect(mockFinalizeRun).toHaveBeenCalledWith({ id: "rec-1", mediaVerificationRetryAttempt: 3, + uploadGenerationId: "generation-1", }); }); @@ -160,6 +170,7 @@ describe("media verification recovery sweep", () => { expect(mockFinalizeRun).toHaveBeenCalledWith({ id: "rec-1", mediaVerificationRetryAttempt: 3, + uploadGenerationId: "generation-1", }); }); diff --git a/templates/clips/server/jobs/media-verification.ts b/templates/clips/server/jobs/media-verification.ts index 08d2edfa45..1a9fe24553 100644 --- a/templates/clips/server/jobs/media-verification.ts +++ b/templates/clips/server/jobs/media-verification.ts @@ -67,6 +67,7 @@ export async function runMediaVerificationSweepOnce(): Promise { .select({ ownerEmail: schema.recordings.ownerEmail, orgId: schema.recordings.orgId, + uploadGenerationId: schema.recordings.uploadGenerationId, }) .from(schema.recordings) .where( @@ -92,6 +93,9 @@ export async function runMediaVerificationSweepOnce(): Promise { MAX_ATTEMPTS, marker.completedAttempts + 1, ), + ...(recording.uploadGenerationId + ? { uploadGenerationId: recording.uploadGenerationId } + : {}), }); }, ); diff --git a/templates/clips/server/lib/recording-upload-state.test.ts b/templates/clips/server/lib/recording-upload-state.test.ts index ab08c17b8e..950e8e9342 100644 --- a/templates/clips/server/lib/recording-upload-state.test.ts +++ b/templates/clips/server/lib/recording-upload-state.test.ts @@ -47,10 +47,13 @@ describe("recording upload state helpers", () => { expect(query.args).toEqual([ "owner@example.com", "recording-chunks-rec!_1-%", + "recording-chunks-rec_1-".length + 6, + "recording-chunks-rec_1-000000", + "recording-chunks-rec_1-999999", ]); }); - it("sums chunk bytes in SQL instead of reading chunk payloads", async () => { + it("sums exact legacy chunk bytes in SQL without reading chunk payloads", async () => { dbMock.execute.mockResolvedValue({ rows: [{ bytes: 7_340_032 }], rowsAffected: 0, @@ -62,23 +65,30 @@ describe("recording upload state helpers", () => { const query = dbMock.execute.mock.calls[0]?.[0]; expect(query.sql).toContain("SUM(json_extract(value, '$.bytes'))"); + expect(query.sql).toContain("length(key) = ?"); expect(query.sql).not.toContain("SELECT key, value"); }); - it("deletes all chunks for one recording by scoped prefix", async () => { - dbMock.execute.mockResolvedValue({ rows: [], rowsAffected: 3 }); + it("deletes exact legacy chunks without matching a fenced generation", async () => { + dbMock.execute.mockResolvedValue({ rows: [], rowsAffected: 1 }); await expect( deleteRecordingChunks("owner@example.com", "rec_1"), - ).resolves.toBe(3); + ).resolves.toBe(1); expect(dbMock.execute).toHaveBeenCalledWith({ sql: expect.stringContaining("DELETE FROM application_state"), - args: ["owner@example.com", "recording-chunks-rec!_1-%"], + args: [ + "owner@example.com", + "recording-chunks-rec!_1-%", + "recording-chunks-rec_1-".length + 6, + "recording-chunks-rec_1-000000", + "recording-chunks-rec_1-999999", + ], }); }); - it("uses the Postgres JSON aggregate when deployed on Postgres", async () => { + it("uses the Postgres JSON extraction when deployed on Postgres", async () => { dbMock.isPostgres.mockReturnValue(true); dbMock.execute.mockResolvedValue({ rows: [{ bytes: "4194304" }], diff --git a/templates/clips/server/lib/recording-upload-state.ts b/templates/clips/server/lib/recording-upload-state.ts index 8647ff61f6..3368cbdb1f 100644 --- a/templates/clips/server/lib/recording-upload-state.ts +++ b/templates/clips/server/lib/recording-upload-state.ts @@ -4,14 +4,46 @@ function escapeLike(value: string): string { return value.replace(/[!%_]/g, (match) => `!${match}`); } -function chunkPrefix(recordingId: string): string { - return `recording-chunks-${recordingId}-`; +function chunkPrefix( + recordingId: string, + generationId?: string | null, +): string { + return generationId + ? `recording-chunks-${recordingId}-${generationId}-` + : `recording-chunks-${recordingId}-`; } function likePrefix(prefix: string): string { return `${escapeLike(prefix)}%`; } +function exactChunkKeyArgs( + ownerEmail: string, + recordingId: string, + generationId?: string | null, +): [string, string, number, string, string] { + const prefix = chunkPrefix(recordingId, generationId); + return [ + ownerEmail, + likePrefix(prefix), + prefix.length + 6, + `${prefix}000000`, + `${prefix}999999`, + ]; +} + +const exactChunkKeyWhere = `session_id = ? AND key LIKE ? ESCAPE '!' AND length(key) = ? AND key >= ? AND key <= ?`; + +function isChunkKeyForGeneration( + key: string, + recordingId: string, + generationId?: string | null, +): boolean { + const prefix = chunkPrefix(recordingId, generationId); + if (!key.startsWith(prefix)) return false; + return /^\d+$/.test(key.slice(prefix.length)); +} + function numberFromRowValue(value: unknown): number { if (typeof value === "number" && Number.isFinite(value)) return value; if (typeof value === "bigint") return Number(value); @@ -81,21 +113,25 @@ export function validateRecordingChunkKeys( export async function listRecordingChunkKeys( ownerEmail: string, recordingId: string, + generationId?: string | null, ): Promise { const { rows } = await getDbExec().execute({ - sql: `SELECT key FROM application_state WHERE session_id = ? AND key LIKE ? ESCAPE '!'`, - args: [ownerEmail, likePrefix(chunkPrefix(recordingId))], + sql: `SELECT key FROM application_state WHERE ${exactChunkKeyWhere}`, + args: exactChunkKeyArgs(ownerEmail, recordingId, generationId), }); - return rows.map((row) => String(row.key)); + return rows + .map((row) => String(row.key)) + .filter((key) => isChunkKeyForGeneration(key, recordingId, generationId)); } export async function deleteRecordingChunks( ownerEmail: string, recordingId: string, + generationId?: string | null, ): Promise { const result = await getDbExec().execute({ - sql: `DELETE FROM application_state WHERE session_id = ? AND key LIKE ? ESCAPE '!'`, - args: [ownerEmail, likePrefix(chunkPrefix(recordingId))], + sql: `DELETE FROM application_state WHERE ${exactChunkKeyWhere}`, + args: exactChunkKeyArgs(ownerEmail, recordingId, generationId), }); return result.rowsAffected ?? 0; } @@ -103,13 +139,14 @@ export async function deleteRecordingChunks( export async function sumRecordingChunkBytes( ownerEmail: string, recordingId: string, + generationId?: string | null, ): Promise { const bytesExpression = isPostgres() ? `COALESCE(SUM((value::jsonb ->> 'bytes')::bigint), 0)` : `COALESCE(SUM(json_extract(value, '$.bytes')), 0)`; const { rows } = await getDbExec().execute({ - sql: `SELECT ${bytesExpression} AS bytes FROM application_state WHERE session_id = ? AND key LIKE ? ESCAPE '!'`, - args: [ownerEmail, likePrefix(chunkPrefix(recordingId))], + sql: `SELECT ${bytesExpression} AS bytes FROM application_state WHERE ${exactChunkKeyWhere}`, + args: exactChunkKeyArgs(ownerEmail, recordingId, generationId), }); return numberFromRowValue(rows[0]?.bytes); } diff --git a/templates/clips/server/lib/resumable-session.ts b/templates/clips/server/lib/resumable-session.ts index 427fc9f5e3..06b41ff7c3 100644 --- a/templates/clips/server/lib/resumable-session.ts +++ b/templates/clips/server/lib/resumable-session.ts @@ -12,12 +12,16 @@ export interface StoredResumableSession { lastCommittedIndex?: number; } -const key = (recordingId: string) => `resumable-session-${recordingId}`; +const key = (recordingId: string, generationId?: string | null) => + generationId + ? `resumable-session-${recordingId}-${generationId}` + : `resumable-session-${recordingId}`; export async function getResumableSession( recordingId: string, + generationId?: string | null, ): Promise { - const raw = await readAppState(key(recordingId)); + const raw = await readAppState(key(recordingId, generationId)); if (!raw || typeof raw !== "object") return null; return raw as unknown as StoredResumableSession; } @@ -25,15 +29,17 @@ export async function getResumableSession( export async function setResumableSession( recordingId: string, session: StoredResumableSession, + generationId?: string | null, ): Promise { await writeAppState( - key(recordingId), + key(recordingId, generationId), session as unknown as Record, ); } export async function deleteResumableSession( recordingId: string, + generationId?: string | null, ): Promise { - await deleteAppState(key(recordingId)); + await deleteAppState(key(recordingId, generationId)); } diff --git a/templates/clips/server/lib/upload-interruption.ts b/templates/clips/server/lib/upload-interruption.ts new file mode 100644 index 0000000000..36b5e9e353 --- /dev/null +++ b/templates/clips/server/lib/upload-interruption.ts @@ -0,0 +1,8 @@ +export const RETRYABLE_UPLOAD_INTERRUPTION_REASON = + "Upload was interrupted. The local recording is safe; retry from the Clips desktop app."; + +export function isRetryableUploadInterruption( + failureReason: string | null | undefined, +): boolean { + return failureReason === RETRYABLE_UPLOAD_INTERRUPTION_REASON; +} diff --git a/templates/clips/server/lib/upload-lease.test.ts b/templates/clips/server/lib/upload-lease.test.ts index 10b1d3d98f..3d352e17b9 100644 --- a/templates/clips/server/lib/upload-lease.test.ts +++ b/templates/clips/server/lib/upload-lease.test.ts @@ -79,6 +79,7 @@ describe("upload lease", () => { status TEXT NOT NULL, failure_reason TEXT, upload_lease_expires_at TEXT, + upload_generation_id TEXT, updated_at TEXT NOT NULL )`); await sqlite.execute(`CREATE TABLE application_state ( @@ -127,6 +128,37 @@ describe("upload lease", () => { expect(await chunkKeys()).toEqual([]); }); + it("removes the generation-scoped session for a reaped upload", async () => { + await insertRecording({ + id: "fenced-dead", + status: "uploading", + lease: iso(-1_000), + }); + await sqlite.execute({ + sql: `UPDATE recordings SET upload_generation_id = ? WHERE id = ?`, + args: ["generation-1", "fenced-dead"], + }); + await sqlite.execute({ + sql: `INSERT INTO application_state (key, value) VALUES (?, ?)`, + args: ["resumable-session-fenced-dead-generation-1", "{}"], + }); + await sqlite.execute({ + sql: `INSERT INTO application_state (key, value) VALUES (?, ?)`, + args: ["resumable-session-fenced-dead", "{}"], + }); + + const result = await reapExpiredUploads({ now: NOW }); + + expect(result.failed).toBe(1); + const { rows } = await sqlite.execute({ + sql: `SELECT key FROM application_state WHERE key LIKE ? ORDER BY key`, + args: ["resumable-session-fenced-dead%"], + }); + expect(rows.map((row) => String(row.key))).toEqual([ + "resumable-session-fenced-dead", + ]); + }); + it("reaches a long-stuck 'processing' recording that no upload session tracks", async () => { await insertRecording({ id: "stuck", diff --git a/templates/clips/server/lib/upload-lease.ts b/templates/clips/server/lib/upload-lease.ts index 262c4788e0..69f72bd561 100644 --- a/templates/clips/server/lib/upload-lease.ts +++ b/templates/clips/server/lib/upload-lease.ts @@ -12,7 +12,7 @@ */ import { getDbExec, isPostgres } from "@agent-native/core/db"; -import { and, eq, inArray, sql } from "drizzle-orm"; +import { and, eq, inArray, isNull, sql } from "drizzle-orm"; import { getDb, schema } from "../db/index.js"; @@ -37,6 +37,7 @@ export type UploadLeaseResult = | { held: true } | { held: false; + staleAttempt: boolean; status: string | null; failureReason: string | null; videoUrl: string | null; @@ -53,7 +54,12 @@ export type UploadLeaseResult = */ export async function renewUploadLease( recordingId: string, - options: { now?: number; uploadProgress?: number } = {}, + options: { + now?: number; + uploadProgress?: number; + attemptId?: string | null; + generationId?: string | null; + } = {}, ): Promise { const now = options.now ?? Date.now(); const held = await getDb() @@ -72,6 +78,23 @@ export async function renewUploadLease( and( eq(schema.recordings.id, recordingId), inArray(schema.recordings.status, [...IN_PROGRESS_STATUSES]), + ...(options.attemptId === undefined + ? [] + : [ + options.attemptId === null + ? isNull(schema.recordings.uploadAttemptId) + : eq(schema.recordings.uploadAttemptId, options.attemptId), + ]), + ...(options.generationId === undefined + ? [] + : [ + options.generationId === null + ? isNull(schema.recordings.uploadGenerationId) + : eq( + schema.recordings.uploadGenerationId, + options.generationId, + ), + ]), ), ) .returning({ id: schema.recordings.id }); @@ -87,12 +110,23 @@ export async function renewUploadLease( videoUrl: schema.recordings.videoUrl, videoSizeBytes: schema.recordings.videoSizeBytes, durationMs: schema.recordings.durationMs, + uploadAttemptId: schema.recordings.uploadAttemptId, + uploadGenerationId: schema.recordings.uploadGenerationId, }) .from(schema.recordings) .where(eq(schema.recordings.id, recordingId)); return { held: false, + staleAttempt: + (options.attemptId !== undefined || options.generationId !== undefined) && + IN_PROGRESS_STATUSES.includes( + row?.status as (typeof IN_PROGRESS_STATUSES)[number], + ) && + ((options.attemptId !== undefined && + row?.uploadAttemptId !== options.attemptId) || + (options.generationId !== undefined && + row?.uploadGenerationId !== options.generationId)), status: row?.status ?? null, failureReason: row?.failureReason ?? null, videoUrl: row?.videoUrl ?? null, @@ -106,6 +140,7 @@ export interface ReapedUpload { ownerEmail: string; status: string; leaseExpiresAt: string; + uploadGenerationId: string | null; } export interface ReapResult { @@ -137,7 +172,7 @@ export async function reapExpiredUploads( // guard:allow-unscoped — system upload reaper, owner-agnostic by design. const probe = await exec.execute({ - sql: `SELECT id, owner_email, status, upload_lease_expires_at + sql: `SELECT id, owner_email, status, upload_lease_expires_at, upload_generation_id FROM recordings WHERE status IN ('uploading', 'processing') AND upload_lease_expires_at IS NOT NULL @@ -154,6 +189,10 @@ export async function reapExpiredUploads( ownerEmail: String(row.owner_email ?? ""), status: String(row.status ?? ""), leaseExpiresAt: String(row.upload_lease_expires_at ?? ""), + uploadGenerationId: + typeof row.upload_generation_id === "string" + ? row.upload_generation_id + : null, })); let failed = 0; @@ -184,9 +223,16 @@ export async function reapExpiredUploads( failed = terminated.size; for (const id of terminated) { + const generationId = expired.find( + (row) => row.id === id, + )?.uploadGenerationId; await exec.execute({ sql: `DELETE FROM application_state WHERE key = ${p(1)}`, - args: [`resumable-session-${id}`], + args: [ + generationId + ? `resumable-session-${id}-${generationId}` + : `resumable-session-${id}`, + ], }); } } diff --git a/templates/clips/server/plugins/db.ts b/templates/clips/server/plugins/db.ts index 62dd5cfee7..eb9b834afb 100644 --- a/templates/clips/server/plugins/db.ts +++ b/templates/clips/server/plugins/db.ts @@ -899,6 +899,16 @@ const migrations = runMigrations( `UPDATE recordings SET upload_lease_expires_at = '${uploadLeaseExpiry()}' WHERE upload_lease_expires_at IS NULL AND status IN ('uploading', 'processing')`, ].join("; "), }, + { + version: 54, + name: "recording-upload-attempt-fence", + sql: `ALTER TABLE recordings ADD COLUMN IF NOT EXISTS upload_attempt_id TEXT`, + }, + { + version: 55, + name: "recording-upload-generation-fence", + sql: `ALTER TABLE recordings ADD COLUMN IF NOT EXISTS upload_generation_id TEXT`, + }, ], { table: "clips_migrations" }, ); diff --git a/templates/clips/server/routes/api/_agent-native-background/post-finalize-worker.post.test.ts b/templates/clips/server/routes/api/_agent-native-background/post-finalize-worker.post.test.ts index 06d6d2961b..4423e47cd0 100644 --- a/templates/clips/server/routes/api/_agent-native-background/post-finalize-worker.post.test.ts +++ b/templates/clips/server/routes/api/_agent-native-background/post-finalize-worker.post.test.ts @@ -6,6 +6,7 @@ const mockDispatchPostFinalizeJob = vi.hoisted(() => vi.fn()); const mockRunWithRequestContext = vi.hoisted(() => vi.fn()); const mockVerifyScopedAgentAccessToken = vi.hoisted(() => vi.fn()); const mockRunLoomImportJob = vi.hoisted(() => vi.fn()); +const mockFinalizeRun = vi.hoisted(() => vi.fn()); const mockUpdateReturning = vi.hoisted(() => vi.fn(async () => [{ id: "rec-1" }]), ); @@ -20,6 +21,7 @@ const mockDb = vi.hoisted(() => ({ ownerEmail: "owner@example.test", orgId: "org-1", status: "processing", + uploadGenerationId: "generation-1", }, ]), }; @@ -54,7 +56,7 @@ vi.mock("@agent-native/core/server", () => ({ })); vi.mock("../../../../actions/finalize-recording.js", () => ({ - default: { run: vi.fn() }, + default: { run: (...args: unknown[]) => mockFinalizeRun(...args) }, })); vi.mock("../../../../actions/lib/ensure-seekable-video.js", () => ({ @@ -73,6 +75,7 @@ vi.mock("../../../db/index.js", () => ({ ownerEmail: "recordings.ownerEmail", orgId: "recordings.orgId", status: "recordings.status", + uploadGenerationId: "recordings.uploadGenerationId", loomImportClaimId: "recordings.loomImportClaimId", loomImportClaimedAt: "recordings.loomImportClaimedAt", }, @@ -112,6 +115,7 @@ describe("post-finalize worker", () => { (_context: unknown, callback: () => unknown) => callback(), ); mockDispatchPostFinalizeJob.mockResolvedValue({ accepted: true }); + mockFinalizeRun.mockResolvedValue({ status: "processing" }); }); afterEach(() => { @@ -157,6 +161,24 @@ describe("post-finalize worker", () => { }); }); + it("re-enters finalization with the processing row generation", async () => { + mockReadBody.mockResolvedValue({ + recordingId: "rec-1", + kind: "media-ready", + token: "valid-token", + retryAttempt: 2, + }); + await expect(handler({} as any)).resolves.toMatchObject({ + ok: true, + kind: "media-ready", + }); + expect(mockFinalizeRun).toHaveBeenCalledWith({ + id: "rec-1", + mediaVerificationRetryAttempt: 2, + uploadGenerationId: "generation-1", + }); + }); + it("skips a Loom import when its atomic claim is already held", async () => { mockReadBody.mockResolvedValue({ recordingId: "rec-1", diff --git a/templates/clips/server/routes/api/_agent-native-background/post-finalize-worker.post.ts b/templates/clips/server/routes/api/_agent-native-background/post-finalize-worker.post.ts index 90ea909f93..086c86623e 100644 --- a/templates/clips/server/routes/api/_agent-native-background/post-finalize-worker.post.ts +++ b/templates/clips/server/routes/api/_agent-native-background/post-finalize-worker.post.ts @@ -66,6 +66,7 @@ export default defineEventHandler(async (event: H3Event) => { ownerEmail: schema.recordings.ownerEmail, orgId: schema.recordings.orgId, status: schema.recordings.status, + uploadGenerationId: schema.recordings.uploadGenerationId, }) .from(schema.recordings) .where(eq(schema.recordings.id, recordingId)) @@ -120,6 +121,9 @@ export default defineEventHandler(async (event: H3Event) => { const result = await finalizeRecording.run({ id: recordingId, mediaVerificationRetryAttempt: retryAttempt ?? 1, + ...(recording.uploadGenerationId + ? { uploadGenerationId: recording.uploadGenerationId } + : {}), }); return { ok: true, kind, result }; } diff --git a/templates/clips/server/routes/api/public-recording.get.test.ts b/templates/clips/server/routes/api/public-recording.get.test.ts index 1c0ac08e67..977d58344d 100644 --- a/templates/clips/server/routes/api/public-recording.get.test.ts +++ b/templates/clips/server/routes/api/public-recording.get.test.ts @@ -254,6 +254,37 @@ describe("/api/public-recording route", () => { }); }); + it("exposes an interrupted upload as failed immediately after a share reload", async () => { + const event = { setCookies: [] as unknown[] }; + mockGetQuery.mockReturnValue({ id: "rec-1" }); + mockGetDb.mockReturnValue( + createDbWithSelectResults([ + [ + makeRecording({ + password: null, + status: "failed", + uploadProgress: 40, + failureReason: + "Upload was interrupted. The local recording is safe; retry from the Clips desktop app.", + }), + ], + [], + [], + [], + [], + ]), + ); + + await expect(handler(event as any)).resolves.toMatchObject({ + recording: { + status: "failed", + uploadProgress: 40, + failureReason: + "Upload was interrupted. The local recording is safe; retry from the Clips desktop app.", + }, + }); + }); + it("allows a scoped agent access token to load private clips without changing visibility", async () => { const event = { setCookies: [] as unknown[] }; mockGetQuery.mockReturnValue({ diff --git a/templates/clips/server/routes/api/uploads/[recordingId]/abort.post.test.ts b/templates/clips/server/routes/api/uploads/[recordingId]/abort.post.test.ts index f19e036dd5..6e07f35106 100644 --- a/templates/clips/server/routes/api/uploads/[recordingId]/abort.post.test.ts +++ b/templates/clips/server/routes/api/uploads/[recordingId]/abort.post.test.ts @@ -2,8 +2,8 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; const mockWriteAppState = vi.hoisted(() => vi.fn()); const mockReadAppState = vi.hoisted(() => vi.fn()); -const mockDeleteAppStateByPrefix = vi.hoisted(() => vi.fn()); -const mockDeleteAppState = vi.hoisted(() => vi.fn()); +const mockCompareAndSetManyAppState = vi.hoisted(() => vi.fn()); +const mockDeleteRecordingChunks = vi.hoisted(() => vi.fn()); const mockGetRouterParam = vi.hoisted(() => vi.fn()); const mockReadBody = vi.hoisted(() => vi.fn()); const mockSetResponseStatus = vi.hoisted(() => vi.fn()); @@ -14,6 +14,7 @@ const mockGetResumableSession = vi.hoisted(() => vi.fn()); const mockAbortSession = vi.hoisted(() => vi.fn()); const mockResolveResumableUploadProvider = vi.hoisted(() => vi.fn()); const mockUpdateSets = vi.hoisted(() => [] as Record[]); +const mockUpdateRows = vi.hoisted(() => ({ rows: [{ id: "rec-1" }] })); const mockSelectRows = vi.hoisted(() => ({ rows: [] as Array>, })); @@ -25,20 +26,24 @@ const mockDb = vi.hoisted(() => ({ }; return builder; }), - update: vi.fn(() => ({ - set: vi.fn((values: Record) => { - mockUpdateSets.push(values); - return { where: vi.fn(async () => undefined) }; - }), - })), + update: vi.fn(() => { + const builder = { + set: vi.fn((values: Record) => { + mockUpdateSets.push(values); + return builder; + }), + where: vi.fn(() => builder), + returning: vi.fn(async () => mockUpdateRows.rows), + }; + return builder; + }), })); vi.mock("@agent-native/core/application-state", () => ({ - deleteAppState: (...args: unknown[]) => mockDeleteAppState(...args), + compareAndSetManyAppState: (...args: unknown[]) => + mockCompareAndSetManyAppState(...args), readAppState: (...args: unknown[]) => mockReadAppState(...args), writeAppState: (...args: unknown[]) => mockWriteAppState(...args), - deleteAppStateByPrefix: (...args: unknown[]) => - mockDeleteAppStateByPrefix(...args), })); vi.mock("@agent-native/core/server", () => ({ @@ -48,6 +53,7 @@ vi.mock("@agent-native/core/server", () => ({ vi.mock("drizzle-orm", () => ({ and: vi.fn(() => "and"), eq: vi.fn(() => "eq"), + isNull: vi.fn(() => "is-null"), })); vi.mock("h3", () => ({ @@ -66,6 +72,8 @@ vi.mock("../../../../db/index.js", () => ({ status: "recordings.status", videoUrl: "recordings.videoUrl", failureReason: "recordings.failureReason", + uploadAttemptId: "recordings.uploadAttemptId", + uploadGenerationId: "recordings.uploadGenerationId", }, }, })); @@ -76,6 +84,11 @@ vi.mock("../../../../lib/recordings.js", () => ({ ownerEmailMatches: (...args: unknown[]) => mockOwnerEmailMatches(...args), })); +vi.mock("../../../../lib/recording-upload-state.js", () => ({ + deleteRecordingChunks: (...args: unknown[]) => + mockDeleteRecordingChunks(...args), +})); + vi.mock("../../../../lib/resumable-session.js", () => ({ deleteResumableSession: (...args: unknown[]) => mockDeleteResumableSession(...args), @@ -101,6 +114,7 @@ describe("/api/uploads/:recordingId/abort route", () => { }, ]; mockUpdateSets.length = 0; + mockUpdateRows.rows = [{ id: "rec-1" }]; mockGetRouterParam.mockReturnValue("rec-1"); mockReadBody.mockResolvedValue({ reason: "Upload was stored-but-unservable: media URL timed out", @@ -110,24 +124,28 @@ describe("/api/uploads/:recordingId/abort route", () => { orgId: "org-1", }); mockOwnerEmailMatches.mockReturnValue("owner-match"); - mockDeleteAppStateByPrefix.mockResolvedValue(2); - mockDeleteAppState.mockResolvedValue(true); + mockDeleteRecordingChunks.mockResolvedValue(2); + mockCompareAndSetManyAppState.mockResolvedValue(true); mockDeleteResumableSession.mockResolvedValue(undefined); mockGetResumableSession.mockResolvedValue(null); mockAbortSession.mockResolvedValue(undefined); mockResolveResumableUploadProvider.mockResolvedValue({ resumable: { abortSession: mockAbortSession }, }); - mockReadAppState.mockResolvedValue({ - recordingId: "rec-1", - status: "uploading", - mimeType: "video/mp4", - durationMs: 1234, - width: 1280, - height: 720, - hasAudio: true, - hasCamera: false, - }); + mockReadAppState.mockImplementation(async (key: string) => + key === "recording-media-verification-rec-1" + ? null + : { + recordingId: "rec-1", + status: "uploading", + mimeType: "video/mp4", + durationMs: 1234, + width: 1280, + height: 720, + hasAudio: true, + hasCamera: false, + }, + ); mockWriteAppState.mockResolvedValue(undefined); }); @@ -138,24 +156,23 @@ describe("/api/uploads/:recordingId/abort route", () => { chunksCleared: 0, }); - expect(mockDeleteAppStateByPrefix).not.toHaveBeenCalled(); + expect(mockDeleteRecordingChunks).not.toHaveBeenCalled(); expect(mockDeleteResumableSession).not.toHaveBeenCalled(); - expect(mockWriteAppState).toHaveBeenCalledWith( - "recording-upload-rec-1", + expect(mockCompareAndSetManyAppState).toHaveBeenCalledWith([ expect.objectContaining({ - recordingId: "rec-1", - status: "failed", - mimeType: "video/mp4", - durationMs: 1234, - width: 1280, - height: 720, - hasAudio: true, - hasCamera: false, + key: "recording-upload-rec-1", + nextValue: expect.objectContaining({ + recordingId: "rec-1", + status: "failed", + mimeType: "video/mp4", + durationMs: 1234, + width: 1280, + height: 720, + hasAudio: true, + hasCamera: false, + }), }), - ); - expect(mockDeleteAppState).toHaveBeenCalledWith( - "recording-media-verification-rec-1", - ); + ]); expect(mockUpdateSets).toEqual( expect.arrayContaining([ expect.objectContaining({ @@ -190,8 +207,8 @@ describe("/api/uploads/:recordingId/abort route", () => { }); expect(mockDb.update).not.toHaveBeenCalled(); - expect(mockDeleteAppState).not.toHaveBeenCalled(); - expect(mockDeleteAppStateByPrefix).not.toHaveBeenCalled(); + expect(mockCompareAndSetManyAppState).not.toHaveBeenCalled(); + expect(mockDeleteRecordingChunks).not.toHaveBeenCalled(); expect(mockDeleteResumableSession).not.toHaveBeenCalled(); }); @@ -212,10 +229,12 @@ describe("/api/uploads/:recordingId/abort route", () => { chunksCleared: 2, }); - expect(mockDeleteAppStateByPrefix).toHaveBeenCalledWith( - "recording-chunks-rec-1-", + expect(mockDeleteRecordingChunks).toHaveBeenCalledWith( + "owner@example.com", + "rec-1", + null, ); - expect(mockDeleteResumableSession).toHaveBeenCalledWith("rec-1"); + expect(mockDeleteResumableSession).toHaveBeenCalledWith("rec-1", null); }); it("aborts provider storage before deleting a resumable session", async () => { @@ -247,6 +266,250 @@ describe("/api/uploads/:recordingId/abort route", () => { ); }); + it("addresses the active generation-scoped session on abort", async () => { + mockSelectRows.rows = [ + { + id: "rec-1", + status: "uploading", + videoUrl: null, + failureReason: null, + uploadAttemptId: null, + uploadGenerationId: "generation-1", + }, + ]; + mockReadBody.mockResolvedValue({ + reason: "Cancelled", + uploadGenerationId: "generation-1", + }); + mockGetResumableSession.mockResolvedValue({ + providerId: "s3", + sessionId: "upload-example", + meta: {}, + bytesUploaded: 123, + }); + + await handler({} as any); + + expect(mockGetResumableSession).toHaveBeenCalledWith( + "rec-1", + "generation-1", + ); + expect(mockDeleteResumableSession).toHaveBeenCalledWith( + "rec-1", + "generation-1", + ); + }); + + it("cleans a provider session created after the abort preflight read", async () => { + mockSelectRows.rows = [ + { + id: "rec-1", + status: "uploading", + videoUrl: null, + failureReason: null, + uploadAttemptId: null, + uploadGenerationId: "generation-1", + }, + ]; + mockReadBody.mockResolvedValue({ + reason: "Cancelled", + uploadGenerationId: "generation-1", + }); + mockGetResumableSession.mockResolvedValueOnce(null).mockResolvedValueOnce({ + providerId: "s3", + sessionId: "late-session", + meta: { objectKey: "clips/rec-1.webm" }, + bytesUploaded: 0, + }); + + await expect(handler({} as any)).resolves.toEqual( + expect.objectContaining({ ok: true }), + ); + + expect(mockAbortSession).toHaveBeenCalledWith({ + sessionId: "late-session", + meta: { objectKey: "clips/rec-1.webm" }, + }); + expect(mockDeleteResumableSession).toHaveBeenCalledWith( + "rec-1", + "generation-1", + ); + }); + + it("does not clean up when a replacement generation wins the abort CAS", async () => { + mockSelectRows.rows = [ + { + id: "rec-1", + status: "uploading", + videoUrl: null, + failureReason: null, + uploadAttemptId: null, + uploadGenerationId: "generation-1", + }, + ]; + mockReadBody.mockResolvedValue({ + reason: "Cancelled", + uploadGenerationId: "generation-1", + }); + mockUpdateRows.rows = []; + + await expect(handler({} as any)).resolves.toEqual({ + error: "A newer upload retry is already active.", + staleAttempt: true, + }); + + expect(mockSetResponseStatus).toHaveBeenCalledWith(expect.anything(), 409); + expect(mockWriteAppState).not.toHaveBeenCalled(); + expect(mockDeleteRecordingChunks).not.toHaveBeenCalled(); + expect(mockGetResumableSession).toHaveBeenCalledWith( + "rec-1", + "generation-1", + ); + }); + + it("preserves replacement auxiliary state that changes after the abort CAS", async () => { + mockSelectRows.rows = [ + { + id: "rec-1", + status: "uploading", + videoUrl: null, + failureReason: null, + uploadAttemptId: null, + uploadGenerationId: "generation-1", + }, + ]; + mockReadBody.mockResolvedValue({ + reason: "Cancelled", + uploadGenerationId: "generation-1", + }); + mockReadAppState + .mockResolvedValueOnce({ + recordingId: "rec-1", + status: "uploading", + uploadGenerationId: "generation-1", + }) + .mockResolvedValueOnce(null); + mockCompareAndSetManyAppState.mockResolvedValue(false); + const consoleInfo = vi + .spyOn(console, "info") + .mockImplementation(() => undefined); + + try { + await expect(handler({} as any)).resolves.toEqual({ + ok: true, + recordingId: "rec-1", + chunksCleared: 2, + }); + } finally { + consoleInfo.mockRestore(); + } + + expect(mockCompareAndSetManyAppState).toHaveBeenCalledWith([ + { + key: "recording-upload-rec-1", + expectedValue: { + recordingId: "rec-1", + status: "uploading", + uploadGenerationId: "generation-1", + }, + nextValue: expect.objectContaining({ status: "failed" }), + }, + ]); + expect(mockWriteAppState).toHaveBeenCalledTimes(1); + expect(mockWriteAppState).toHaveBeenCalledWith("refresh-signal", { + ts: expect.any(Number), + }); + }); + + it("surfaces auxiliary-state failures after claiming the abort", async () => { + mockSelectRows.rows = [ + { + id: "rec-1", + status: "uploading", + videoUrl: null, + failureReason: null, + uploadAttemptId: null, + uploadGenerationId: null, + }, + ]; + mockReadBody.mockResolvedValue({ reason: "Cancelled" }); + mockCompareAndSetManyAppState.mockRejectedValue( + new Error("application state unavailable"), + ); + + await expect(handler({} as any)).rejects.toThrow( + "application state unavailable", + ); + + expect(mockDeleteRecordingChunks).not.toHaveBeenCalled(); + expect(mockWriteAppState).not.toHaveBeenCalled(); + }); + + it("fails before claiming the row when resumable state is unreadable", async () => { + mockSelectRows.rows = [ + { + id: "rec-1", + status: "uploading", + videoUrl: null, + failureReason: null, + uploadAttemptId: null, + uploadGenerationId: "generation-1", + }, + ]; + mockReadBody.mockResolvedValue({ + reason: "Cancelled", + uploadGenerationId: "generation-1", + }); + mockGetResumableSession.mockRejectedValue( + new Error("session store unavailable"), + ); + + await expect(handler({} as any)).rejects.toThrow( + "session store unavailable", + ); + + expect(mockDb.update).not.toHaveBeenCalled(); + expect(mockCompareAndSetManyAppState).not.toHaveBeenCalled(); + expect(mockDeleteRecordingChunks).not.toHaveBeenCalled(); + expect(mockDeleteResumableSession).not.toHaveBeenCalled(); + }); + + it("retries exact cleanup after an auxiliary-state failure", async () => { + mockSelectRows.rows = [ + { + id: "rec-1", + status: "uploading", + videoUrl: null, + failureReason: null, + uploadAttemptId: null, + uploadGenerationId: null, + }, + ]; + mockReadBody.mockResolvedValue({ reason: "Cancelled" }); + mockCompareAndSetManyAppState + .mockRejectedValueOnce(new Error("application state unavailable")) + .mockResolvedValueOnce(true); + + await expect(handler({} as any)).rejects.toThrow( + "application state unavailable", + ); + expect(mockDeleteRecordingChunks).not.toHaveBeenCalled(); + + mockSelectRows.rows[0]!.status = "failed"; + await expect(handler({} as any)).resolves.toEqual({ + ok: true, + recordingId: "rec-1", + chunksCleared: 2, + }); + + expect(mockCompareAndSetManyAppState).toHaveBeenCalledTimes(2); + expect(mockDeleteRecordingChunks).toHaveBeenCalledWith( + "owner@example.com", + "rec-1", + null, + ); + }); + it("preserves the resumable session when provider abort cleanup fails", async () => { mockSelectRows.rows = [ { @@ -309,6 +572,6 @@ describe("/api/uploads/:recordingId/abort route", () => { sessionId: "upload-example", meta: { objectKey: "clips/rec-1.webm" }, }); - expect(mockDeleteResumableSession).toHaveBeenCalledWith("rec-1"); + expect(mockDeleteResumableSession).toHaveBeenCalledWith("rec-1", null); }); }); diff --git a/templates/clips/server/routes/api/uploads/[recordingId]/abort.post.ts b/templates/clips/server/routes/api/uploads/[recordingId]/abort.post.ts index d99cdadb97..76be13009b 100644 --- a/templates/clips/server/routes/api/uploads/[recordingId]/abort.post.ts +++ b/templates/clips/server/routes/api/uploads/[recordingId]/abort.post.ts @@ -6,14 +6,13 @@ */ import { - deleteAppState, + compareAndSetManyAppState, readAppState, writeAppState, - deleteAppStateByPrefix, } from "@agent-native/core/application-state"; import { runWithRequestContext } from "@agent-native/core/server"; import { isStoredButUnservableFinalizeError } from "@shared/finalize-recovery.js"; -import { and, eq } from "drizzle-orm"; +import { and, eq, isNull } from "drizzle-orm"; import { defineEventHandler, getRouterParam, @@ -24,6 +23,7 @@ import { import { getDb, schema } from "../../../../db/index.js"; import { mediaVerificationStateKey } from "../../../../lib/media-verification-state.js"; +import { deleteRecordingChunks } from "../../../../lib/recording-upload-state.js"; import { getEventOwnerContext, ownerEmailMatches, @@ -44,11 +44,25 @@ export default defineEventHandler(async (event: H3Event) => { const { userEmail: ownerEmail, orgId } = await getEventOwnerContext(event); const body = (await readBody(event).catch(() => null)) as { reason?: unknown; + attemptId?: unknown; + uploadGenerationId?: unknown; } | null; const failureReason = typeof body?.reason === "string" && body.reason.trim() ? body.reason.trim().slice(0, 1000) : "Upload aborted by user"; + const requestedAttemptId = + typeof body?.attemptId === "string" && + body.attemptId.length > 0 && + body.attemptId.length <= 128 + ? body.attemptId + : null; + const requestedGenerationId = + typeof body?.uploadGenerationId === "string" && + body.uploadGenerationId.length > 0 && + body.uploadGenerationId.length <= 128 + ? body.uploadGenerationId + : null; return runWithRequestContext({ userEmail: ownerEmail, orgId }, async () => { const db = getDb(); @@ -59,6 +73,8 @@ export default defineEventHandler(async (event: H3Event) => { status: schema.recordings.status, videoUrl: schema.recordings.videoUrl, failureReason: schema.recordings.failureReason, + uploadAttemptId: schema.recordings.uploadAttemptId, + uploadGenerationId: schema.recordings.uploadGenerationId, }) .from(schema.recordings) .where( @@ -73,17 +89,43 @@ export default defineEventHandler(async (event: H3Event) => { return { error: "Recording not found" }; } + const existingAttemptId = existing.uploadAttemptId ?? null; + const existingGenerationId = existing.uploadGenerationId ?? null; + if ( + requestedAttemptId !== existingAttemptId || + requestedGenerationId !== existingGenerationId + ) { + setResponseStatus(event, 409); + return { + error: "A newer upload retry is already active.", + staleAttempt: true, + }; + } + if (existing.status === "ready" && existing.videoUrl) { return { ok: true, recordingId, alreadyReady: true, chunksCleared: 0 }; } - const existingUploadStateRaw = await readAppState( - `recording-upload-${recordingId}`, - ).catch(() => null); + const uploadStateKey = `recording-upload-${recordingId}`; + const verificationStateKey = mediaVerificationStateKey(recordingId); + const [existingUploadStateRaw, existingVerificationStateRaw] = + await Promise.all([ + readAppState(uploadStateKey), + readAppState(verificationStateKey), + ]); const existingUploadState = existingUploadStateRaw && typeof existingUploadStateRaw === "object" ? (existingUploadStateRaw as Record) : {}; + const existingUploadStateSnapshot = + existingUploadStateRaw && typeof existingUploadStateRaw === "object" + ? (existingUploadStateRaw as Record) + : null; + const existingVerificationStateSnapshot = + existingVerificationStateRaw && + typeof existingVerificationStateRaw === "object" + ? (existingVerificationStateRaw as Record) + : null; if ( existing.status === "processing" && existingUploadState.pendingMediaVerification === true @@ -99,46 +141,87 @@ export default defineEventHandler(async (event: H3Event) => { const preserveRecoveryState = isStoredButUnservableFinalizeError(failureReason) || isStoredButUnservableFinalizeError(existing.failureReason); - const resumableSession = preserveRecoveryState + let resumableSession = preserveRecoveryState ? null - : await getResumableSession(recordingId).catch(() => null); - - // Already a terminal failure (e.g. a duplicate/retried abort call, or - // finalize's own failChunkAssembly already flipped it) — no-op unless a - // prior provider cleanup failure intentionally retained a resumable - // session for this retry. - if ( - existing.status === "failed" && - !preserveRecoveryState && - !resumableSession - ) { - return { ok: true, recordingId, alreadyFailed: true, chunksCleared: 0 }; - } + : await getResumableSession(recordingId, existingGenerationId); const now = new Date().toISOString(); - await db + const aborted = await db .update(schema.recordings) .set({ status: "failed", failureReason, updatedAt: now, }) - .where(eq(schema.recordings.id, recordingId)); - - await writeAppState(`recording-upload-${recordingId}`, { - ...existingUploadState, - recordingId, - status: "failed", - failureReason, - updatedAt: now, - }); - await deleteAppState(mediaVerificationStateKey(recordingId)).catch( - () => {}, - ); + .where( + and( + eq(schema.recordings.id, recordingId), + ownerEmailMatches(schema.recordings.ownerEmail, ownerEmail), + eq(schema.recordings.status, existing.status), + existingAttemptId === null + ? isNull(schema.recordings.uploadAttemptId) + : eq(schema.recordings.uploadAttemptId, existingAttemptId), + existingGenerationId === null + ? isNull(schema.recordings.uploadGenerationId) + : eq(schema.recordings.uploadGenerationId, existingGenerationId), + ), + ) + .returning({ id: schema.recordings.id }); + + if (aborted.length !== 1) { + setResponseStatus(event, 409); + return { + error: "A newer upload retry is already active.", + staleAttempt: true, + }; + } + + const auxiliaryStateUpdated = await compareAndSetManyAppState([ + { + key: uploadStateKey, + expectedValue: existingUploadStateSnapshot, + nextValue: { + ...existingUploadState, + recordingId, + status: "failed", + failureReason, + updatedAt: now, + }, + }, + ...(existingVerificationStateSnapshot + ? [ + { + key: verificationStateKey, + expectedValue: existingVerificationStateSnapshot, + nextValue: null, + }, + ] + : []), + ]); + if (!auxiliaryStateUpdated) { + console.info( + `[abort] upload state changed after abort claim; preserving replacement state for ${recordingId}`, + ); + } + + // Reset may have started this exact generation's provider session after + // our preflight read but before the abort claim. Re-read after the row CAS + // so either abort observes the late handle or reset observes the lost row + // claim and compensates it. + if (!preserveRecoveryState) { + resumableSession = await getResumableSession( + recordingId, + existingGenerationId, + ); + } const cleared = preserveRecoveryState ? 0 - : await deleteAppStateByPrefix(`recording-chunks-${recordingId}-`); + : await deleteRecordingChunks( + ownerEmail, + recordingId, + existingGenerationId, + ); if (!preserveRecoveryState) { if (resumableSession) { const provider = await resolveResumableUploadProvider( @@ -166,10 +249,14 @@ export default defineEventHandler(async (event: H3Event) => { // retry can still address the multipart upload. Deleting it here would // permanently orphan the provider-side session. if (providerCleanupSucceeded) { - await deleteResumableSession(recordingId).catch(() => {}); + await deleteResumableSession(recordingId, existingGenerationId).catch( + () => {}, + ); } } else { - await deleteResumableSession(recordingId).catch(() => {}); + await deleteResumableSession(recordingId, existingGenerationId).catch( + () => {}, + ); } } await writeAppState("refresh-signal", { ts: Date.now() }); diff --git a/templates/clips/server/routes/api/uploads/[recordingId]/chunk.post.test.ts b/templates/clips/server/routes/api/uploads/[recordingId]/chunk.post.test.ts index 38dc98cf88..431e3d7f04 100644 --- a/templates/clips/server/routes/api/uploads/[recordingId]/chunk.post.test.ts +++ b/templates/clips/server/routes/api/uploads/[recordingId]/chunk.post.test.ts @@ -7,6 +7,7 @@ const mockAppState = vi.hoisted(() => new Map>()); const mockReadAppState = vi.hoisted(() => vi.fn()); const mockWriteAppState = vi.hoisted(() => vi.fn()); const mockTrack = vi.hoisted(() => vi.fn()); +const mockIsFeatureFlagEnabled = vi.hoisted(() => vi.fn()); const mockGetRouterParam = vi.hoisted(() => vi.fn()); const mockGetQuery = vi.hoisted(() => vi.fn()); const mockGetHeader = vi.hoisted(() => vi.fn()); @@ -18,6 +19,7 @@ const mockDeleteRecordingChunks = vi.hoisted(() => vi.fn()); const mockRenewUploadLease = vi.hoisted(() => vi.fn()); const mockSumRecordingChunkBytes = vi.hoisted(() => vi.fn()); const mockGetResumableSession = vi.hoisted(() => vi.fn()); +const mockDeleteResumableSession = vi.hoisted(() => vi.fn()); const mockSetResumableSession = vi.hoisted(() => vi.fn()); const mockRelayChunk = vi.hoisted(() => vi.fn()); const mockResolveResumableUploadProvider = vi.hoisted(() => vi.fn()); @@ -50,6 +52,11 @@ vi.mock("@agent-native/core/application-state", () => ({ writeAppState: (...args: unknown[]) => mockWriteAppState(...args), })); +vi.mock("@agent-native/core/feature-flags", () => ({ + isFeatureFlagEnabled: (...args: unknown[]) => + mockIsFeatureFlagEnabled(...args), +})); + vi.mock("@agent-native/core/server", () => ({ runWithRequestContext: (_ctx: unknown, fn: () => unknown) => fn(), })); @@ -94,6 +101,7 @@ vi.mock("../../../../db/index.js", () => ({ hasAudio: "recordings.hasAudio", hasCamera: "recordings.hasCamera", uploadProgress: "recordings.uploadProgress", + uploadGenerationId: "recordings.uploadGenerationId", updatedAt: "recordings.updatedAt", }, }, @@ -113,6 +121,8 @@ vi.mock("../../../../lib/recordings.js", () => ({ })); vi.mock("../../../../lib/resumable-session.js", () => ({ + deleteResumableSession: (...args: unknown[]) => + mockDeleteResumableSession(...args), getResumableSession: (...args: unknown[]) => mockGetResumableSession(...args), setResumableSession: (...args: unknown[]) => mockSetResumableSession(...args), })); @@ -179,10 +189,12 @@ describe("/api/uploads/:recordingId/chunk route", () => { }); mockOwnerEmailMatches.mockReturnValue("owner-match"); mockGetResumableSession.mockResolvedValue(null); + mockDeleteResumableSession.mockResolvedValue(undefined); mockSetResumableSession.mockResolvedValue(undefined); mockIsStreamingUploadDisabled.mockReturnValue(false); mockShouldRejectVideoUploadWithoutStorage.mockResolvedValue(false); mockAllowsSqlRecordingChunkScratch.mockReturnValue(true); + mockIsFeatureFlagEnabled.mockResolvedValue(true); mockResolveResumableUploadProvider.mockResolvedValue({ resumable: { relayChunk: mockRelayChunk }, }); @@ -244,6 +256,105 @@ describe("/api/uploads/:recordingId/chunk route", () => { ); }); + it("rejects chunk indices that cannot fit the exact scratch-key contract", async () => { + setRequest({ + query: { + index: "1000000", + total: "1000001", + mimeType: "video/webm", + }, + body: new Uint8Array([1]), + }); + + await expect(handler({} as any)).rejects.toMatchObject({ + message: "Invalid chunk index", + statusCode: 400, + }); + + expect(mockReadRawBody).not.toHaveBeenCalled(); + expect(mockWriteAppState).not.toHaveBeenCalled(); + }); + + it("rejects a stale retry token before reading or storing its chunk", async () => { + mockRenewUploadLease.mockResolvedValueOnce({ + held: false, + staleAttempt: true, + status: "uploading", + failureReason: null, + videoUrl: null, + videoSizeBytes: null, + durationMs: null, + }); + setRequest({ + query: { + index: "0", + total: "2", + isFinal: "0", + mimeType: "video/webm", + attemptId: "stale-attempt", + }, + body: new Uint8Array([1, 2, 3]), + }); + + await expect(handler({} as any)).resolves.toEqual({ + ok: false, + error: "A newer upload retry is already active.", + staleAttempt: true, + }); + expect(mockSetResponseStatus).toHaveBeenCalledWith({}, 409); + expect(mockReadRawBody).not.toHaveBeenCalled(); + expect(mockWriteAppState).not.toHaveBeenCalled(); + }); + + it("forces a full restart when the retry flag switches off between chunks", async () => { + mockGetResumableSession.mockResolvedValue({ + providerId: "s3", + sessionId: "sess-1", + meta: { objectKey: "clips/rec-1.webm" }, + bytesUploaded: 0, + lastCommittedIndex: -1, + }); + mockIsFeatureFlagEnabled + .mockResolvedValueOnce(true) + .mockResolvedValueOnce(false); + + setRequest({ + query: { + index: "0", + total: "2", + mimeType: "video/webm", + attemptId: "retry-attempt", + }, + body: new Uint8Array([1, 2, 3]), + }); + await expect(handler({} as any)).resolves.toEqual({ + ok: true, + finalized: false, + index: 0, + bytes: 3, + }); + + setRequest({ + query: { + index: "1", + total: "2", + mimeType: "video/webm", + attemptId: "retry-attempt", + }, + body: new Uint8Array([4, 5, 6]), + }); + await expect(handler({} as any)).resolves.toEqual({ + ok: false, + error: "Resumable upload retry is disabled.", + restartRequired: true, + recoveryEnabled: false, + }); + expect(mockSetResponseStatus).toHaveBeenLastCalledWith({}, 409); + expect(mockReadRawBody).toHaveBeenCalledOnce(); + expect(mockRelayChunk).toHaveBeenCalledOnce(); + expect(mockRenewUploadLease).toHaveBeenCalledTimes(3); + }); + it("stores in-order chunks and advances upload progress state", async () => { setRequest({ query: { index: "0", total: "4", mimeType: "video/webm" }, @@ -307,8 +418,8 @@ describe("/api/uploads/:recordingId/chunk route", () => { ([, options]) => options?.uploadProgress !== undefined, ), ).toEqual([ - ["rec-1", { uploadProgress: 25 }], - ["rec-1", { uploadProgress: 50 }], + ["rec-1", { attemptId: null, generationId: null, uploadProgress: 25 }], + ["rec-1", { attemptId: null, generationId: null, uploadProgress: 50 }], ]); expect(mockUpdateSets).toEqual([]); expect(mockFinalizeRun).not.toHaveBeenCalled(); @@ -497,6 +608,7 @@ describe("/api/uploads/:recordingId/chunk route", () => { expect(mockDeleteRecordingChunks).toHaveBeenCalledWith( "owner@example.com", "rec-1", + null, ); expect(chunkKeys()).toEqual([]); expect(mockWriteAppState).not.toHaveBeenCalled(); @@ -812,13 +924,17 @@ describe("/api/uploads/:recordingId/chunk route", () => { bytes, { mimeType: "video/webm" }, ); - expect(mockSetResumableSession).toHaveBeenCalledWith("rec-1", { - providerId: "s3", - sessionId: "sess-1", - meta: { objectKey: "clips/rec-1.webm" }, - bytesUploaded: 105, - lastCommittedIndex: 3, - }); + expect(mockSetResumableSession).toHaveBeenCalledWith( + "rec-1", + { + providerId: "s3", + sessionId: "sess-1", + meta: { objectKey: "clips/rec-1.webm" }, + bytesUploaded: 105, + lastCommittedIndex: 3, + }, + null, + ); expect(mockFinalizeRun).not.toHaveBeenCalled(); }); @@ -859,6 +975,152 @@ describe("/api/uploads/:recordingId/chunk route", () => { expect(mockFinalizeRun).not.toHaveBeenCalled(); }); + it("retires an expired provider session so the desktop can restart safely", async () => { + mockGetResumableSession.mockResolvedValue({ + providerId: "s3", + sessionId: "expired-session", + meta: { objectKey: "clips/rec-1.webm" }, + bytesUploaded: 100, + lastCommittedIndex: 2, + }); + mockRelayChunk.mockResolvedValue({ ok: false, status: 410 }); + setRequest({ + query: { index: "3", total: "0", mimeType: "video/webm" }, + body: new Uint8Array([1, 2, 3]), + }); + + await expect(handler({} as any)).resolves.toEqual({ + ok: false, + error: "Chunk upload failed (410)", + restartRequired: true, + }); + expect(mockSetResponseStatus).toHaveBeenCalledWith({}, 409); + expect(mockDeleteResumableSession).toHaveBeenCalledWith("rec-1", null); + expect(mockSetResumableSession).not.toHaveBeenCalled(); + }); + + it("keeps replacement-generation scratch when a stale writer loses its lease", async () => { + (mockSelectRows.rows[0] as Record).uploadGenerationId = + "generation-a"; + // A gets admitted and reads its body. While it is in flight, reset moves + // the row to B; A's pre-write renewal must then clean only A scratch. + mockRenewUploadLease + .mockResolvedValueOnce({ held: true }) + .mockImplementationOnce(async () => { + (mockSelectRows.rows[0] as Record).uploadGenerationId = + "generation-b"; + return { held: false, staleAttempt: true }; + }); + setRequest({ + query: { + index: "0", + total: "1", + mimeType: "video/webm", + uploadGenerationId: "generation-a", + }, + body: new Uint8Array([1]), + }); + + await expect(handler({} as any)).resolves.toEqual( + expect.objectContaining({ ok: false }), + ); + + // The loser may clean up only its own generation; B's scratch is never a + // valid target for a delayed A request. + expect(mockDeleteRecordingChunks).toHaveBeenCalledWith( + "owner@example.com", + "rec-1", + "generation-a", + ); + expect(mockDeleteRecordingChunks).not.toHaveBeenCalledWith( + "owner@example.com", + "rec-1", + "generation-b", + ); + }); + + it("does not delete a replacement session when an old provider response expires", async () => { + (mockSelectRows.rows[0] as Record).uploadGenerationId = + "generation-a"; + mockGetResumableSession.mockResolvedValue({ + providerId: "s3", + sessionId: "old-session", + meta: {}, + bytesUploaded: 100, + lastCommittedIndex: 0, + }); + mockRenewUploadLease + .mockResolvedValueOnce({ held: true }) + .mockResolvedValueOnce({ held: true }); + mockRelayChunk.mockImplementationOnce(async () => { + // B exists before A receives the delayed provider expiry. + (mockSelectRows.rows[0] as Record).uploadGenerationId = + "generation-b"; + return { ok: false, status: 410 }; + }); + setRequest({ + query: { + index: "1", + mimeType: "video/webm", + uploadGenerationId: "generation-a", + }, + body: new Uint8Array([1]), + }); + + await expect(handler({} as any)).resolves.toEqual( + expect.objectContaining({ restartRequired: true }), + ); + expect(mockDeleteResumableSession).toHaveBeenCalledWith( + "rec-1", + "generation-a", + ); + expect(mockDeleteResumableSession).not.toHaveBeenCalledWith( + "rec-1", + "generation-b", + ); + }); + + it("does not restore a replacement session after a delayed old provider success", async () => { + (mockSelectRows.rows[0] as Record).uploadGenerationId = + "generation-a"; + mockGetResumableSession.mockResolvedValue({ + providerId: "s3", + sessionId: "old-session", + meta: {}, + bytesUploaded: 100, + lastCommittedIndex: 0, + }); + // This test owns all three lease boundaries: A admission, A provider + // dispatch, then the post-provider fence after reset installed B. + mockRenewUploadLease + .mockResolvedValueOnce({ held: true }) + .mockResolvedValueOnce({ held: true }) + .mockResolvedValueOnce({ held: false, staleAttempt: true }); + mockRelayChunk.mockImplementationOnce(async () => { + (mockSelectRows.rows[0] as Record).uploadGenerationId = + "generation-b"; + return { ok: true, status: 308 }; + }); + setRequest({ + query: { + index: "1", + mimeType: "video/webm", + uploadGenerationId: "generation-a", + }, + body: new Uint8Array([1]), + }); + + await expect(handler({} as any)).resolves.toEqual( + expect.objectContaining({ ok: false }), + ); + expect(mockSetResumableSession).not.toHaveBeenCalled(); + expect(mockSetResumableSession).not.toHaveBeenCalledWith( + "rec-1", + expect.anything(), + "generation-b", + ); + }); + it("returns exact resumable source-byte proof when finalize committed before its response was lost", async () => { mockGetResumableSession.mockResolvedValue({ providerId: "s3", diff --git a/templates/clips/server/routes/api/uploads/[recordingId]/chunk.post.ts b/templates/clips/server/routes/api/uploads/[recordingId]/chunk.post.ts index 8bd79e6abb..d24f183069 100644 --- a/templates/clips/server/routes/api/uploads/[recordingId]/chunk.post.ts +++ b/templates/clips/server/routes/api/uploads/[recordingId]/chunk.post.ts @@ -17,6 +17,7 @@ import { readAppState, writeAppState, } from "@agent-native/core/application-state"; +import { isFeatureFlagEnabled } from "@agent-native/core/feature-flags"; import { runWithRequestContext } from "@agent-native/core/server"; import { track } from "@agent-native/core/tracking"; import { normalizeChunkUploadNumber } from "@shared/recording-core.js"; @@ -34,6 +35,7 @@ import { } from "h3"; import finalizeRecording from "../../../../../actions/finalize-recording.js"; +import { UPLOAD_RETRY_RESUME_FLAG } from "../../../../../shared/feature-flags.js"; import { getDb, schema } from "../../../../db/index.js"; import { debugLog } from "../../../../lib/debug.js"; import { @@ -45,6 +47,7 @@ import { ownerEmailMatches, } from "../../../../lib/recordings.js"; import { + deleteResumableSession, getResumableSession, setResumableSession, type StoredResumableSession, @@ -166,6 +169,26 @@ export default defineEventHandler(async (event: H3Event) => { } const query = getQuery(event); + const attemptIdValue = Array.isArray(query.attemptId) + ? query.attemptId[0] + : query.attemptId; + const attemptId = + typeof attemptIdValue === "string" && + attemptIdValue.length > 0 && + attemptIdValue.length <= 128 && + !/[\r\n]/.test(attemptIdValue) + ? attemptIdValue + : null; + const uploadGenerationIdValue = Array.isArray(query.uploadGenerationId) + ? query.uploadGenerationId[0] + : query.uploadGenerationId; + const uploadGenerationId = + typeof uploadGenerationIdValue === "string" && + uploadGenerationIdValue.length > 0 && + uploadGenerationIdValue.length <= 128 && + !/[\r\n]/.test(uploadGenerationIdValue) + ? uploadGenerationIdValue + : null; const index = Number(query.index ?? 0); const total = Number(query.total ?? 0); const isFinal = query.isFinal === "1" || query.isFinal === "true"; @@ -188,7 +211,12 @@ export default defineEventHandler(async (event: H3Event) => { mimeType, }); - if (!Number.isFinite(index) || !Number.isInteger(index) || index < 0) { + if ( + !Number.isFinite(index) || + !Number.isInteger(index) || + index < 0 || + index > 999_999 + ) { throw createError({ statusCode: 400, message: "Invalid chunk index" }); } @@ -210,6 +238,23 @@ export default defineEventHandler(async (event: H3Event) => { } debugLog("[chunk] resolved owner:", ownerEmail); + if ( + attemptId !== null && + !(await isFeatureFlagEnabled(UPLOAD_RETRY_RESUME_FLAG, { + userEmail: ownerEmail, + userKey: ownerEmail, + orgId, + })) + ) { + setResponseStatus(event, 409); + return { + ok: false, + error: "Resumable upload retry is disabled.", + restartRequired: true, + recoveryEnabled: false, + }; + } + return runWithRequestContext({ userEmail: ownerEmail, orgId }, async () => { const db = getDb(); @@ -219,6 +264,7 @@ export default defineEventHandler(async (event: H3Event) => { .select({ id: schema.recordings.id, status: schema.recordings.status, + uploadGenerationId: schema.recordings.uploadGenerationId, }) .from(schema.recordings) .where( @@ -252,13 +298,31 @@ export default defineEventHandler(async (event: H3Event) => { // The first renewal admits the request. Later renewals immediately before // every durable write/finalization boundary close the body-read/provider // gap where /abort can otherwise commit while this request is in flight. + if ((existing.uploadGenerationId ?? null) !== uploadGenerationId) { + setResponseStatus(event, 409); + return { + ok: false, + error: "A newer upload generation is already active.", + staleAttempt: true, + }; + } const lease = await renewUploadLease(recordingId, { + attemptId, + generationId: uploadGenerationId, uploadProgress: total > 0 ? Math.min(100, Math.round(((index + 1) / total) * 100)) : undefined, }); if (!lease.held) { + if (lease.staleAttempt) { + setResponseStatus(event, 409); + return { + ok: false, + error: "A newer upload retry is already active.", + staleAttempt: true, + }; + } if (lease.status === "ready") { const readyState = await readAppState( `recording-upload-${recordingId}`, @@ -273,7 +337,11 @@ export default defineEventHandler(async (event: H3Event) => { sourceSizeBytes: stateNumber(readyState, "sourceSizeBytes"), }; } - await deleteRecordingChunks(ownerEmail, recordingId).catch((err) => { + await deleteRecordingChunks( + ownerEmail, + recordingId, + uploadGenerationId, + ).catch((err) => { console.warn("[chunk] failed upload chunk cleanup failed:", { recordingId, err: err instanceof Error ? err.message : String(err), @@ -285,9 +353,16 @@ export default defineEventHandler(async (event: H3Event) => { } const rejectIfLeaseLost = async () => { - const current = await renewUploadLease(recordingId); + const current = await renewUploadLease(recordingId, { + attemptId, + generationId: uploadGenerationId, + }); if (current.held) return null; - await deleteRecordingChunks(ownerEmail, recordingId).catch((err) => { + await deleteRecordingChunks( + ownerEmail, + recordingId, + uploadGenerationId, + ).catch((err) => { console.warn("[chunk] failed upload chunk cleanup failed:", { recordingId, err: err instanceof Error ? err.message : String(err), @@ -308,7 +383,10 @@ export default defineEventHandler(async (event: H3Event) => { } // Resumable streaming path — forward chunks directly to the provider. - const resumableSession = await getResumableSession(recordingId); + const resumableSession = await getResumableSession( + recordingId, + uploadGenerationId, + ); if (resumableSession && isStreamingUploadDisabled()) { console.warn( `[chunk] streaming uploads are disabled, but preserving existing resumable session for in-flight recording: ${recordingId}`, @@ -324,6 +402,8 @@ export default defineEventHandler(async (event: H3Event) => { mimeType, query, ownerEmail, + attemptId, + uploadGenerationId, ); } @@ -418,7 +498,11 @@ export default defineEventHandler(async (event: H3Event) => { maxBytes: MAX_RECORDING_UPLOAD_BYTES, updatedAt: now, }); - await deleteRecordingChunks(ownerEmail, recordingId).catch((err) => { + await deleteRecordingChunks( + ownerEmail, + recordingId, + uploadGenerationId, + ).catch((err) => { console.warn("[chunk] oversized upload chunk cleanup failed:", { recordingId, err: err instanceof Error ? err.message : String(err), @@ -439,12 +523,13 @@ export default defineEventHandler(async (event: H3Event) => { // Pad index to 6 digits so string-sort order matches numeric order if the // finalize path ever sorts lexically. (finalize also parses back to a number.) const paddedIndex = String(index).padStart(6, "0"); - const chunkKey = `recording-chunks-${recordingId}-${paddedIndex}`; + const chunkKey = `recording-chunks-${recordingId}${uploadGenerationId ? `-${uploadGenerationId}` : ""}-${paddedIndex}`; const previousChunk = await readAppState(chunkKey); const previousBytes = stateNumber(previousChunk, "bytes") ?? 0; const persistedBytesBefore = await sumRecordingChunkBytes( ownerEmail, recordingId, + uploadGenerationId, ); const nextBytes = Math.max(0, persistedBytesBefore - previousBytes) + bytes.byteLength; @@ -463,7 +548,11 @@ export default defineEventHandler(async (event: H3Event) => { data: toBase64(bytes), createdAt: new Date().toISOString(), }); - bytesReceived = await sumRecordingChunkBytes(ownerEmail, recordingId); + bytesReceived = await sumRecordingChunkBytes( + ownerEmail, + recordingId, + uploadGenerationId, + ); if (bytesReceived > MAX_RECORDING_UPLOAD_BYTES) { return failRecordingTooLarge(bytesReceived); } @@ -487,6 +576,7 @@ export default defineEventHandler(async (event: H3Event) => { if (leaseFailure) return leaseFailure; await writeAppState(`recording-upload-${recordingId}`, { recordingId, + uploadGenerationId, status: isFinal ? "processing" : "uploading", progress, chunksReceived, @@ -508,6 +598,7 @@ export default defineEventHandler(async (event: H3Event) => { if (leaseFailure) return leaseFailure; await writeAppState(`recording-upload-${recordingId}`, { recordingId, + uploadGenerationId, status: isFinal ? "processing" : "uploading", chunksReceived: Math.max( stateNumber(uploadState, "chunksReceived") ?? 0, @@ -532,7 +623,11 @@ export default defineEventHandler(async (event: H3Event) => { if (isFinal) { const leaseFailure = await rejectIfLeaseLost(); if (leaseFailure) return leaseFailure; - bytesReceived = await sumRecordingChunkBytes(ownerEmail, recordingId); + bytesReceived = await sumRecordingChunkBytes( + ownerEmail, + recordingId, + uploadGenerationId, + ); if (bytesReceived > MAX_RECORDING_UPLOAD_BYTES) { return failRecordingTooLarge(bytesReceived); } @@ -541,7 +636,7 @@ export default defineEventHandler(async (event: H3Event) => { debugLog("[chunk] isFinal — invoking finalize", { recordingId }); try { const result = await finalizeRecording.run( - buildFinalizeArgs(recordingId, mimeType, query), + buildFinalizeArgs(recordingId, mimeType, query, uploadGenerationId), ); debugLog("[chunk] finalize ok", { recordingId, @@ -713,6 +808,7 @@ function buildFinalizeArgs( recordingId: string, mimeType: string, query: Record, + uploadGenerationId: string | null, ) { const queryBoolean = (value: unknown): boolean | undefined => { if (value === undefined) return undefined; @@ -729,6 +825,7 @@ function buildFinalizeArgs( hasCamera: queryBoolean(query.hasCamera), locallyTranscoded: queryBoolean(query.locallyTranscoded), mimeType, + ...(uploadGenerationId ? { uploadGenerationId } : {}), }; } @@ -743,6 +840,8 @@ async function handleResumableChunk( mimeType: string, query: Record, ownerEmail: string, + attemptId: string | null, + uploadGenerationId: string | null, ) { const uploadProvider = await resolveResumableUploadProvider( session.providerId, @@ -771,7 +870,10 @@ async function handleResumableChunk( error: "Cannot finalize an empty resumable upload", }; } - const lease = await renewUploadLease(recordingId); + const lease = await renewUploadLease(recordingId, { + attemptId, + generationId: uploadGenerationId, + }); if (!lease.held) { setResponseStatus(event, 409); return { @@ -798,7 +900,10 @@ async function handleResumableChunk( }; } if (closeRes.updatedMeta) { - const postCloseLease = await renewUploadLease(recordingId); + const postCloseLease = await renewUploadLease(recordingId, { + attemptId, + generationId: uploadGenerationId, + }); if (!postCloseLease.held) { setResponseStatus(event, 409); return { @@ -808,10 +913,14 @@ async function handleResumableChunk( "Recording upload has already failed.", }; } - await setResumableSession(recordingId, { - ...session, - meta: { ...session.meta, ...closeRes.updatedMeta }, - }); + await setResumableSession( + recordingId, + { + ...session, + meta: { ...session.meta, ...closeRes.updatedMeta }, + }, + uploadGenerationId, + ); } } else { // Idempotent replay guard: a client retry (after a lost response) can @@ -835,7 +944,10 @@ async function handleResumableChunk( }; } } else { - const lease = await renewUploadLease(recordingId); + const lease = await renewUploadLease(recordingId, { + attemptId, + generationId: uploadGenerationId, + }); if (!lease.held) { setResponseStatus(event, 409); return { @@ -866,14 +978,25 @@ async function handleResumableChunk( ? putResult.ok && putResult.status !== 308 : putResult.ok; if (!resultOk) { - setResponseStatus(event, 502); + const restartRequired = + putResult.status === 404 || putResult.status === 410; + if (restartRequired) { + await deleteResumableSession(recordingId, uploadGenerationId).catch( + () => {}, + ); + } + setResponseStatus(event, restartRequired ? 409 : 502); return { ok: false, error: `Chunk upload failed (${putResult.status})`, + ...(restartRequired ? { restartRequired: true } : {}), }; } - const postUploadLease = await renewUploadLease(recordingId); + const postUploadLease = await renewUploadLease(recordingId, { + attemptId, + generationId: uploadGenerationId, + }); if (!postUploadLease.held) { setResponseStatus(event, 409); return { @@ -883,14 +1006,18 @@ async function handleResumableChunk( "Recording upload has already failed.", }; } - await setResumableSession(recordingId, { - ...session, - ...(putResult.updatedMeta - ? { meta: { ...session.meta, ...putResult.updatedMeta } } - : {}), - bytesUploaded: start + bytes.byteLength, - lastCommittedIndex: index, - }); + await setResumableSession( + recordingId, + { + ...session, + ...(putResult.updatedMeta + ? { meta: { ...session.meta, ...putResult.updatedMeta } } + : {}), + bytesUploaded: start + bytes.byteLength, + lastCommittedIndex: index, + }, + uploadGenerationId, + ); finalizedSourceSizeBytes = start + bytes.byteLength; if (!isFinal) { @@ -901,7 +1028,10 @@ async function handleResumableChunk( // isFinal — delegate to finalize-recording, which reads the resumable // session and calls provider.resumable.completeSession. - const finalLease = await renewUploadLease(recordingId); + const finalLease = await renewUploadLease(recordingId, { + attemptId, + generationId: uploadGenerationId, + }); if (!finalLease.held) { setResponseStatus(event, 409); return { @@ -911,7 +1041,7 @@ async function handleResumableChunk( } try { const result = await finalizeRecording.run( - buildFinalizeArgs(recordingId, mimeType, query), + buildFinalizeArgs(recordingId, mimeType, query, uploadGenerationId), ); if ((result as any)?.status === "failed") { setResponseStatus(event, 409); diff --git a/templates/clips/server/routes/api/uploads/[recordingId]/interrupt.post.test.ts b/templates/clips/server/routes/api/uploads/[recordingId]/interrupt.post.test.ts new file mode 100644 index 0000000000..975e7d7a37 --- /dev/null +++ b/templates/clips/server/routes/api/uploads/[recordingId]/interrupt.post.test.ts @@ -0,0 +1,244 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const mockReadAppState = vi.hoisted(() => vi.fn()); +const mockWriteAppState = vi.hoisted(() => vi.fn()); +const mockCompareAndSetAppState = vi.hoisted(() => vi.fn()); +const mockSetResponseStatus = vi.hoisted(() => vi.fn()); +const mockIsFeatureFlagEnabled = vi.hoisted(() => vi.fn()); +const mockAbortUpload = vi.hoisted(() => vi.fn()); +const mockSelectRows = vi.hoisted(() => ({ + rows: [] as Array>, +})); +const mockUpdatedRows = vi.hoisted(() => ({ rows: [{ id: "rec-1" }] })); +const mockUpdateSets = vi.hoisted(() => [] as Record[]); +const mockBody = vi.hoisted(() => ({ + value: { detail: "TypeError: Load failed" } as Record, +})); +const mockDb = vi.hoisted(() => ({ + select: vi.fn(() => { + const builder = { + from: vi.fn(() => builder), + where: vi.fn(async () => mockSelectRows.rows), + }; + return builder; + }), + update: vi.fn(() => { + const builder = { + set: vi.fn((values: Record) => { + mockUpdateSets.push(values); + return builder; + }), + where: vi.fn(() => builder), + returning: vi.fn(async () => mockUpdatedRows.rows), + }; + return builder; + }), +})); + +vi.mock("@agent-native/core/application-state", () => ({ + compareAndSetAppState: (...args: unknown[]) => + mockCompareAndSetAppState(...args), + readAppState: (...args: unknown[]) => mockReadAppState(...args), + writeAppState: (...args: unknown[]) => mockWriteAppState(...args), +})); +vi.mock("@agent-native/core/feature-flags", () => ({ + isFeatureFlagEnabled: (...args: unknown[]) => + mockIsFeatureFlagEnabled(...args), +})); +vi.mock("@agent-native/core/server", () => ({ + runWithRequestContext: (_ctx: unknown, fn: () => unknown) => fn(), +})); +vi.mock("drizzle-orm", () => ({ + and: vi.fn(() => "and"), + eq: vi.fn(() => "eq"), + isNull: vi.fn(() => "is-null"), +})); +vi.mock("h3", () => ({ + defineEventHandler: (handler: unknown) => handler, + getRouterParam: () => "rec-1", + readBody: async () => mockBody.value, + setResponseStatus: (...args: unknown[]) => mockSetResponseStatus(...args), +})); +vi.mock("../../../../db/index.js", () => ({ + getDb: () => mockDb, + schema: { recordings: {} }, +})); +vi.mock("../../../../lib/recordings.js", () => ({ + getEventOwnerContext: async () => ({ + userEmail: "owner@example.com", + orgId: "org-1", + }), + ownerEmailMatches: () => "owner-match", +})); +vi.mock("./abort.post.js", () => ({ + default: (...args: unknown[]) => mockAbortUpload(...args), +})); + +import handler from "./interrupt.post"; + +describe("/api/uploads/:recordingId/interrupt route", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockSelectRows.rows = [ + { + id: "rec-1", + status: "uploading", + failureReason: null, + videoUrl: null, + }, + ]; + mockUpdatedRows.rows = [{ id: "rec-1" }]; + mockBody.value = { detail: "TypeError: Load failed" }; + mockUpdateSets.length = 0; + mockReadAppState.mockResolvedValue({ + recordingId: "rec-1", + bytesReceived: 7_864_320, + progress: 40, + }); + mockWriteAppState.mockResolvedValue(undefined); + mockCompareAndSetAppState.mockResolvedValue(true); + mockIsFeatureFlagEnabled.mockResolvedValue(true); + mockAbortUpload.mockResolvedValue({ ok: true, legacyAbort: true }); + }); + + it("uses the existing destructive abort when resumable retry is disabled", async () => { + mockIsFeatureFlagEnabled.mockResolvedValue(false); + + await expect(handler({} as any)).resolves.toEqual({ + ok: true, + legacyAbort: true, + }); + expect(mockAbortUpload).toHaveBeenCalledOnce(); + expect(mockDb.select).not.toHaveBeenCalled(); + expect(mockWriteAppState).not.toHaveBeenCalled(); + }); + + it("marks the row retryable without deleting resumable state", async () => { + await expect(handler({} as any)).resolves.toEqual({ + ok: true, + recordingId: "rec-1", + status: "failed", + resumable: true, + }); + expect(mockUpdateSets).toEqual([ + expect.objectContaining({ + status: "failed", + failureReason: + "Upload was interrupted. The local recording is safe; retry from the Clips desktop app.", + }), + ]); + expect(mockCompareAndSetAppState).toHaveBeenCalledWith( + "recording-upload-rec-1", + expect.objectContaining({ bytesReceived: 7_864_320 }), + expect.objectContaining({ + status: "failed", + retryableInterruption: true, + bytesReceived: 7_864_320, + interruptionDetail: "TypeError: Load failed", + }), + ); + }); + + it("does not overwrite media that the server already accepted", async () => { + mockSelectRows.rows = [ + { + id: "rec-1", + status: "processing", + failureReason: null, + videoUrl: "https://cdn.example.com/rec-1", + }, + ]; + + await expect(handler({} as any)).resolves.toEqual({ + ok: true, + recordingId: "rec-1", + status: "processing", + alreadyAccepted: true, + }); + expect(mockDb.update).not.toHaveBeenCalled(); + expect(mockWriteAppState).not.toHaveBeenCalled(); + }); + + it("rejects a lost compare-and-set instead of claiming interruption", async () => { + mockUpdatedRows.rows = []; + + await expect(handler({} as any)).resolves.toEqual({ + error: "Recording upload changed while it was interrupted", + }); + expect(mockSetResponseStatus).toHaveBeenCalledWith({}, 409); + expect(mockWriteAppState).not.toHaveBeenCalled(); + }); + + it("fails before claiming the row when upload state is unreadable", async () => { + mockReadAppState.mockRejectedValue(new Error("state store unavailable")); + + await expect(handler({} as any)).rejects.toThrow("state store unavailable"); + + expect(mockDb.update).not.toHaveBeenCalled(); + expect(mockCompareAndSetAppState).not.toHaveBeenCalled(); + expect(mockWriteAppState).not.toHaveBeenCalled(); + }); + + it("preserves replacement upload state after the row claim", async () => { + mockCompareAndSetAppState.mockResolvedValue(false); + const consoleInfo = vi + .spyOn(console, "info") + .mockImplementation(() => undefined); + + try { + await expect(handler({} as any)).resolves.toEqual( + expect.objectContaining({ ok: true, resumable: true }), + ); + } finally { + consoleInfo.mockRestore(); + } + + expect(mockCompareAndSetAppState).toHaveBeenCalledOnce(); + expect(mockWriteAppState).toHaveBeenCalledTimes(1); + expect(mockWriteAppState).toHaveBeenCalledWith("refresh-signal", { + ts: expect.any(Number), + }); + }); + + it("retries auxiliary reconciliation for an already interrupted row", async () => { + mockSelectRows.rows = [ + { + id: "rec-1", + status: "failed", + failureReason: + "Upload was interrupted. The local recording is safe; retry from the Clips desktop app.", + uploadAttemptId: null, + uploadGenerationId: null, + }, + ]; + + await expect(handler({} as any)).resolves.toEqual( + expect.objectContaining({ + ok: true, + alreadyInterrupted: true, + resumable: true, + }), + ); + + expect(mockDb.update).not.toHaveBeenCalled(); + expect(mockCompareAndSetAppState).toHaveBeenCalledOnce(); + }); + + it("does not let a delayed callback interrupt a newer retry", async () => { + mockSelectRows.rows = [ + { + id: "rec-1", + status: "uploading", + failureReason: null, + uploadAttemptId: "new-attempt", + }, + ]; + + await expect(handler({} as any)).resolves.toEqual({ + error: "A newer upload retry is already active.", + staleAttempt: true, + }); + expect(mockSetResponseStatus).toHaveBeenCalledWith({}, 409); + expect(mockDb.update).not.toHaveBeenCalled(); + }); +}); diff --git a/templates/clips/server/routes/api/uploads/[recordingId]/interrupt.post.ts b/templates/clips/server/routes/api/uploads/[recordingId]/interrupt.post.ts new file mode 100644 index 0000000000..2d7d3d490f --- /dev/null +++ b/templates/clips/server/routes/api/uploads/[recordingId]/interrupt.post.ts @@ -0,0 +1,197 @@ +// guard:allow-api-route — Upload transport interruption preserves resumable state for desktop recovery. + +/** + * Mark a live upload as interrupted without discarding its provider session or + * buffered chunks. Explicit cancellation continues to use /abort. + * + * Route: POST /api/uploads/:recordingId/interrupt + */ + +import { + compareAndSetAppState, + readAppState, + writeAppState, +} from "@agent-native/core/application-state"; +import { isFeatureFlagEnabled } from "@agent-native/core/feature-flags"; +import { runWithRequestContext } from "@agent-native/core/server"; +import { and, eq, isNull } from "drizzle-orm"; +import { + defineEventHandler, + getRouterParam, + readBody, + setResponseStatus, + type H3Event, +} from "h3"; + +import { UPLOAD_RETRY_RESUME_FLAG } from "../../../../../shared/feature-flags.js"; +import { getDb, schema } from "../../../../db/index.js"; +import { + getEventOwnerContext, + ownerEmailMatches, +} from "../../../../lib/recordings.js"; +import { + isRetryableUploadInterruption, + RETRYABLE_UPLOAD_INTERRUPTION_REASON, +} from "../../../../lib/upload-interruption.js"; +import abortUpload from "./abort.post.js"; + +export default defineEventHandler(async (event: H3Event) => { + const recordingId = getRouterParam(event, "recordingId"); + if (!recordingId) { + setResponseStatus(event, 400); + return { error: "Missing recordingId" }; + } + + const { userEmail: ownerEmail, orgId } = await getEventOwnerContext(event); + if ( + !(await isFeatureFlagEnabled(UPLOAD_RETRY_RESUME_FLAG, { + userEmail: ownerEmail, + userKey: ownerEmail, + orgId, + })) + ) { + return abortUpload(event); + } + const body = (await readBody(event).catch(() => null)) as { + detail?: unknown; + attemptId?: unknown; + uploadGenerationId?: unknown; + } | null; + const attemptId = + typeof body?.attemptId === "string" && + body.attemptId.length > 0 && + body.attemptId.length <= 128 + ? body.attemptId + : null; + const interruptionDetail = + typeof body?.detail === "string" && body.detail.trim() + ? body.detail.trim().slice(0, 1000) + : null; + const uploadGenerationId = + typeof body?.uploadGenerationId === "string" && + body.uploadGenerationId.length > 0 && + body.uploadGenerationId.length <= 128 + ? body.uploadGenerationId + : null; + + return runWithRequestContext({ userEmail: ownerEmail, orgId }, async () => { + const db = getDb(); + const [existing] = await db + .select({ + id: schema.recordings.id, + status: schema.recordings.status, + failureReason: schema.recordings.failureReason, + videoUrl: schema.recordings.videoUrl, + uploadAttemptId: schema.recordings.uploadAttemptId, + uploadGenerationId: schema.recordings.uploadGenerationId, + }) + .from(schema.recordings) + .where( + and( + eq(schema.recordings.id, recordingId), + ownerEmailMatches(schema.recordings.ownerEmail, ownerEmail), + ), + ); + + if (!existing) { + setResponseStatus(event, 404); + return { error: "Recording not found" }; + } + if (existing.status === "ready" || existing.status === "processing") { + return { + ok: true, + recordingId, + status: existing.status, + alreadyAccepted: true, + }; + } + if ((existing.uploadAttemptId ?? null) !== attemptId) { + setResponseStatus(event, 409); + return { + error: "A newer upload retry is already active.", + staleAttempt: true, + }; + } + if ((existing.uploadGenerationId ?? null) !== uploadGenerationId) { + setResponseStatus(event, 409); + return { + error: "A newer upload generation is already active.", + staleAttempt: true, + }; + } + const alreadyInterrupted = + existing.status === "failed" && + isRetryableUploadInterruption(existing.failureReason); + if (existing.status !== "uploading" && !alreadyInterrupted) { + setResponseStatus(event, 409); + return { error: "Recording upload is no longer active" }; + } + + const uploadStateKey = `recording-upload-${recordingId}`; + const uploadStateRaw = await readAppState(uploadStateKey); + const uploadState = uploadStateRaw ?? {}; + const interruptedAt = new Date().toISOString(); + if (!alreadyInterrupted) { + const interrupted = await db + .update(schema.recordings) + .set({ + status: "failed", + failureReason: RETRYABLE_UPLOAD_INTERRUPTION_REASON, + updatedAt: interruptedAt, + }) + .where( + and( + eq(schema.recordings.id, recordingId), + ownerEmailMatches(schema.recordings.ownerEmail, ownerEmail), + eq(schema.recordings.status, "uploading"), + attemptId === null + ? isNull(schema.recordings.uploadAttemptId) + : eq(schema.recordings.uploadAttemptId, attemptId), + uploadGenerationId === null + ? isNull(schema.recordings.uploadGenerationId) + : eq(schema.recordings.uploadGenerationId, uploadGenerationId), + ), + ) + .returning({ id: schema.recordings.id }); + + if (interrupted.length !== 1) { + setResponseStatus(event, 409); + return { error: "Recording upload changed while it was interrupted" }; + } + } + + const uploadStateUpdated = await compareAndSetAppState( + uploadStateKey, + uploadStateRaw, + { + ...uploadState, + recordingId, + status: "failed", + failureReason: RETRYABLE_UPLOAD_INTERRUPTION_REASON, + retryableInterruption: true, + uploadAttemptId: attemptId, + uploadGenerationId, + ...(interruptionDetail ? { interruptionDetail } : {}), + updatedAt: interruptedAt, + }, + ); + if (!uploadStateUpdated) { + console.info( + `[upload-interrupt] upload state changed after interruption claim; preserving replacement state for ${recordingId}`, + ); + } + await writeAppState("refresh-signal", { ts: Date.now() }); + + console.info("[upload-interrupt] resumable state preserved", { + recordingId, + hasInterruptionDetail: interruptionDetail !== null, + }); + return { + ok: true, + recordingId, + status: "failed", + resumable: true, + ...(alreadyInterrupted ? { alreadyInterrupted: true } : {}), + }; + }); +}); diff --git a/templates/clips/server/routes/api/uploads/[recordingId]/reset-chunks.post.test.ts b/templates/clips/server/routes/api/uploads/[recordingId]/reset-chunks.post.test.ts index 0b34b0ee48..3b9227fa6a 100644 --- a/templates/clips/server/routes/api/uploads/[recordingId]/reset-chunks.post.test.ts +++ b/templates/clips/server/routes/api/uploads/[recordingId]/reset-chunks.post.test.ts @@ -1,16 +1,21 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; const mockWriteAppState = vi.hoisted(() => vi.fn()); -const mockDeleteAppStateByPrefix = vi.hoisted(() => vi.fn()); +const mockReadAppState = vi.hoisted(() => vi.fn()); +const mockCompareAndSetAppState = vi.hoisted(() => vi.fn()); +const mockDeleteRecordingChunks = vi.hoisted(() => vi.fn()); const mockGetActiveFileUploadProviderForRequest = vi.hoisted(() => vi.fn()); const mockGetRouterParam = vi.hoisted(() => vi.fn()); const mockReadBody = vi.hoisted(() => vi.fn()); const mockSetResponseStatus = vi.hoisted(() => vi.fn()); +const mockIsFeatureFlagEnabled = vi.hoisted(() => vi.fn()); const mockGetEventOwnerContext = vi.hoisted(() => vi.fn()); const mockOwnerEmailMatches = vi.hoisted(() => vi.fn()); const mockDeleteResumableSession = vi.hoisted(() => vi.fn()); const mockSetResumableSession = vi.hoisted(() => vi.fn()); const mockStartSession = vi.hoisted(() => vi.fn()); +const mockAbortSession = vi.hoisted(() => vi.fn()); +const mockRenewUploadLease = vi.hoisted(() => vi.fn()); const mockShouldEnableStreamingUpload = vi.hoisted(() => vi.fn()); const mockAllowsSqlRecordingChunkScratch = vi.hoisted(() => vi.fn()); const mockIsMediaVerificationPending = vi.hoisted(() => vi.fn()); @@ -20,6 +25,8 @@ const mockExistingRecording = vi.hoisted(() => ({ id: "rec-1", status: "uploading", videoUrl: null as string | null, + uploadAttemptId: null as string | null, + uploadGenerationId: null as string | null, }, })); const mockDb = vi.hoisted(() => ({ @@ -30,18 +37,24 @@ const mockDb = vi.hoisted(() => ({ }; return builder; }), - update: vi.fn(() => ({ - set: vi.fn((values: Record) => { - mockUpdateSets.push(values); - return { where: vi.fn(async () => undefined) }; - }), - })), + update: vi.fn(() => { + const builder = { + set: vi.fn((values: Record) => { + mockUpdateSets.push(values); + return builder; + }), + where: vi.fn(() => builder), + returning: vi.fn(async () => [{ id: "rec-1" }]), + }; + return builder; + }), })); vi.mock("@agent-native/core/application-state", () => ({ + compareAndSetAppState: (...args: unknown[]) => + mockCompareAndSetAppState(...args), + readAppState: (...args: unknown[]) => mockReadAppState(...args), writeAppState: (...args: unknown[]) => mockWriteAppState(...args), - deleteAppStateByPrefix: (...args: unknown[]) => - mockDeleteAppStateByPrefix(...args), })); vi.mock("@agent-native/core/file-upload", () => ({ @@ -49,6 +62,11 @@ vi.mock("@agent-native/core/file-upload", () => ({ mockGetActiveFileUploadProviderForRequest(...args), })); +vi.mock("@agent-native/core/feature-flags", () => ({ + isFeatureFlagEnabled: (...args: unknown[]) => + mockIsFeatureFlagEnabled(...args), +})); + vi.mock("@agent-native/core/server", () => ({ runWithRequestContext: (_ctx: unknown, fn: () => unknown) => fn(), })); @@ -56,6 +74,7 @@ vi.mock("@agent-native/core/server", () => ({ vi.mock("drizzle-orm", () => ({ and: vi.fn(() => "and"), eq: vi.fn(() => "eq"), + isNull: vi.fn(() => "is-null"), })); vi.mock("h3", () => ({ @@ -73,6 +92,8 @@ vi.mock("../../../../db/index.js", () => ({ ownerEmail: "recordings.ownerEmail", status: "recordings.status", videoUrl: "recordings.videoUrl", + uploadAttemptId: "recordings.uploadAttemptId", + uploadGenerationId: "recordings.uploadGenerationId", failureReason: "recordings.failureReason", uploadProgress: "recordings.uploadProgress", updatedAt: "recordings.updatedAt", @@ -86,6 +107,11 @@ vi.mock("../../../../lib/recordings.js", () => ({ ownerEmailMatches: (...args: unknown[]) => mockOwnerEmailMatches(...args), })); +vi.mock("../../../../lib/recording-upload-state.js", () => ({ + deleteRecordingChunks: (...args: unknown[]) => + mockDeleteRecordingChunks(...args), +})); + vi.mock("../../../../lib/media-verification-state.js", () => ({ isMediaVerificationPending: (...args: unknown[]) => mockIsMediaVerificationPending(...args), @@ -102,6 +128,11 @@ vi.mock("../../../../lib/streaming-upload-mode.js", () => ({ mockShouldEnableStreamingUpload(...args), })); +vi.mock("../../../../lib/upload-lease.js", () => ({ + renewUploadLease: (...args: unknown[]) => mockRenewUploadLease(...args), + uploadLeaseExpiry: () => "2099-01-01T00:00:00.000Z", +})); + vi.mock("../../../../lib/video-storage.js", () => ({ allowsSqlRecordingChunkScratch: (...args: unknown[]) => mockAllowsSqlRecordingChunkScratch(...args), @@ -119,17 +150,27 @@ describe("/api/uploads/:recordingId/reset-chunks route", () => { orgId: "org-1", }); mockOwnerEmailMatches.mockReturnValue("owner-match"); - mockDeleteAppStateByPrefix.mockResolvedValue(3); + mockDeleteRecordingChunks.mockResolvedValue(3); mockDeleteResumableSession.mockResolvedValue(undefined); mockSetResumableSession.mockResolvedValue(undefined); mockWriteAppState.mockResolvedValue(undefined); + mockReadAppState.mockResolvedValue({ + recordingId: "rec-1", + status: "uploading", + }); + mockCompareAndSetAppState.mockResolvedValue(true); + mockRenewUploadLease.mockResolvedValue({ held: true }); + mockAbortSession.mockResolvedValue(undefined); mockAllowsSqlRecordingChunkScratch.mockReturnValue(false); mockShouldEnableStreamingUpload.mockReturnValue(true); mockIsMediaVerificationPending.mockResolvedValue(false); + mockIsFeatureFlagEnabled.mockResolvedValue(true); mockExistingRecording.current = { id: "rec-1", status: "uploading", videoUrl: null, + uploadAttemptId: null, + uploadGenerationId: null, }; mockStartSession.mockResolvedValue({ sessionId: "session-1", @@ -137,14 +178,38 @@ describe("/api/uploads/:recordingId/reset-chunks route", () => { }); mockGetActiveFileUploadProviderForRequest.mockResolvedValue({ id: "test-provider", - resumable: { startSession: mockStartSession }, + resumable: { + startSession: mockStartSession, + abortSession: mockAbortSession, + }, + }); + }); + + it("clears a recovery claim when the flag is disabled mid-retry", async () => { + mockIsFeatureFlagEnabled.mockResolvedValue(false); + mockExistingRecording.current.uploadAttemptId = "old-attempt"; + mockReadBody.mockResolvedValue({ + requestStreaming: true, + mimeType: "video/webm", + useGenerationFence: true, }); + + await expect(handler({} as any)).resolves.toEqual( + expect.objectContaining({ ok: true, uploadGenerationId: null }), + ); + expect(mockUpdateSets).toContainEqual( + expect.objectContaining({ + uploadAttemptId: null, + uploadGenerationId: null, + }), + ); }); it("recreates a resumable session for a browser backup retry", async () => { mockReadBody.mockResolvedValue({ requestStreaming: true, mimeType: "video/webm;codecs=vp9,opus", + useGenerationFence: true, }); await expect(handler({} as any)).resolves.toEqual( @@ -153,22 +218,64 @@ describe("/api/uploads/:recordingId/reset-chunks route", () => { recordingId: "rec-1", uploadMode: "streaming", chunksCleared: 3, + uploadGenerationId: expect.any(String), }), ); - expect(mockDeleteResumableSession).toHaveBeenCalledWith("rec-1"); + expect(mockDeleteResumableSession).toHaveBeenCalledWith("rec-1", null); expect(mockStartSession).toHaveBeenCalledWith( "rec-1.webm", "video/webm", expect.any(Number), ); - expect(mockSetResumableSession).toHaveBeenCalledWith("rec-1", { - providerId: "test-provider", - sessionId: "session-1", - meta: { provider: "test", stableUrl: true, recordAsset: false }, - bytesUploaded: 0, - lastCommittedIndex: -1, + expect(mockSetResumableSession).toHaveBeenCalledWith( + "rec-1", + { + providerId: "test-provider", + sessionId: "session-1", + meta: { provider: "test", stableUrl: true, recordAsset: false }, + bytesUploaded: 0, + lastCommittedIndex: -1, + }, + expect.any(String), + ); + }); + + it("fences a claimed retry even without the browser opt-in", async () => { + mockExistingRecording.current.uploadAttemptId = "attempt-1"; + mockReadBody.mockResolvedValue({ attemptId: "attempt-1" }); + + await expect(handler({} as any)).resolves.toEqual( + expect.objectContaining({ uploadGenerationId: expect.any(String) }), + ); + + expect(mockUpdateSets).toContainEqual( + expect.objectContaining({ uploadGenerationId: expect.any(String) }), + ); + }); + + it("keeps legacy native resets on the unfenced generation contract", async () => { + mockReadBody.mockResolvedValue({ + requestStreaming: true, + mimeType: "video/mp4", }); + + await expect(handler({} as any)).resolves.toEqual( + expect.objectContaining({ + ok: true, + uploadMode: "streaming", + uploadGenerationId: null, + }), + ); + + expect(mockUpdateSets).toContainEqual( + expect.objectContaining({ uploadGenerationId: null }), + ); + expect(mockSetResumableSession).toHaveBeenCalledWith( + "rec-1", + expect.any(Object), + null, + ); }); it("keeps an explicitly buffered reset on the buffered path", async () => { @@ -183,12 +290,54 @@ describe("/api/uploads/:recordingId/reset-chunks route", () => { expect(mockSetResumableSession).not.toHaveBeenCalled(); }); + it("fails before claiming reset when upload state is unreadable", async () => { + mockReadBody.mockResolvedValue({}); + mockReadAppState.mockRejectedValue(new Error("state store unavailable")); + + await expect(handler({} as any)).rejects.toThrow("state store unavailable"); + + expect(mockDb.update).not.toHaveBeenCalled(); + expect(mockDeleteRecordingChunks).not.toHaveBeenCalled(); + expect(mockStartSession).not.toHaveBeenCalled(); + }); + + it("compensates a provider session when abort wins during startup", async () => { + mockReadBody.mockResolvedValue({ + requestStreaming: true, + mimeType: "video/webm", + useGenerationFence: true, + }); + mockRenewUploadLease.mockResolvedValue({ + held: false, + staleAttempt: true, + status: "failed", + }); + + await expect(handler({} as any)).resolves.toEqual({ + error: "Recording upload changed while its retry was starting.", + staleAttempt: true, + }); + + expect(mockAbortSession).toHaveBeenCalledWith({ + sessionId: "session-1", + meta: { provider: "test" }, + }); + expect(mockDeleteResumableSession).toHaveBeenLastCalledWith( + "rec-1", + expect.any(String), + ); + expect(mockCompareAndSetAppState).not.toHaveBeenCalled(); + expect(mockWriteAppState).not.toHaveBeenCalled(); + }); + it("does not reset a recording that is already ready", async () => { mockReadBody.mockResolvedValue({}); mockExistingRecording.current = { id: "rec-1", status: "ready", videoUrl: "https://cdn.example/video.webm", + uploadAttemptId: null, + uploadGenerationId: null, }; await expect(handler({} as any)).resolves.toEqual({ @@ -196,7 +345,7 @@ describe("/api/uploads/:recordingId/reset-chunks route", () => { }); expect(mockSetResponseStatus).toHaveBeenCalledWith(expect.anything(), 409); - expect(mockDeleteAppStateByPrefix).not.toHaveBeenCalled(); + expect(mockDeleteRecordingChunks).not.toHaveBeenCalled(); expect(mockDeleteResumableSession).not.toHaveBeenCalled(); expect(mockUpdateSets).toHaveLength(0); }); @@ -207,6 +356,8 @@ describe("/api/uploads/:recordingId/reset-chunks route", () => { id: "rec-1", status: "processing", videoUrl: "https://cdn.example/video.webm", + uploadAttemptId: null, + uploadGenerationId: null, }; mockIsMediaVerificationPending.mockResolvedValue(true); @@ -220,7 +371,7 @@ describe("/api/uploads/:recordingId/reset-chunks route", () => { recordingStatus: "processing", }); expect(mockSetResponseStatus).toHaveBeenCalledWith(expect.anything(), 409); - expect(mockDeleteAppStateByPrefix).not.toHaveBeenCalled(); + expect(mockDeleteRecordingChunks).not.toHaveBeenCalled(); expect(mockDeleteResumableSession).not.toHaveBeenCalled(); expect(mockUpdateSets).toHaveLength(0); }); diff --git a/templates/clips/server/routes/api/uploads/[recordingId]/reset-chunks.post.ts b/templates/clips/server/routes/api/uploads/[recordingId]/reset-chunks.post.ts index 1c6bc7232e..5b81bc01f1 100644 --- a/templates/clips/server/routes/api/uploads/[recordingId]/reset-chunks.post.ts +++ b/templates/clips/server/routes/api/uploads/[recordingId]/reset-chunks.post.ts @@ -24,15 +24,19 @@ * Route: POST /api/uploads/:recordingId/reset-chunks */ +import { randomUUID } from "node:crypto"; + import { + compareAndSetAppState, + readAppState, writeAppState, - deleteAppStateByPrefix, } from "@agent-native/core/application-state"; +import { isFeatureFlagEnabled } from "@agent-native/core/feature-flags"; import { getActiveFileUploadProviderForRequest } from "@agent-native/core/file-upload"; import { runWithRequestContext } from "@agent-native/core/server"; import type { UploadMode } from "@shared/recording-core.js"; import { MAX_UPLOAD_BYTES as MAX_RECORDING_UPLOAD_BYTES } from "@shared/upload-limits.js"; -import { and, eq } from "drizzle-orm"; +import { and, eq, isNull } from "drizzle-orm"; import { defineEventHandler, getRouterParam, @@ -41,8 +45,10 @@ import { type H3Event, } from "h3"; +import { UPLOAD_RETRY_RESUME_FLAG } from "../../../../../shared/feature-flags.js"; import { getDb, schema } from "../../../../db/index.js"; import { isMediaVerificationPending } from "../../../../lib/media-verification-state.js"; +import { deleteRecordingChunks } from "../../../../lib/recording-upload-state.js"; import { getEventOwnerContext, ownerEmailMatches, @@ -52,7 +58,10 @@ import { setResumableSession, } from "../../../../lib/resumable-session.js"; import { shouldEnableStreamingUpload } from "../../../../lib/streaming-upload-mode.js"; -import { uploadLeaseExpiry } from "../../../../lib/upload-lease.js"; +import { + renewUploadLease, + uploadLeaseExpiry, +} from "../../../../lib/upload-lease.js"; import { allowsSqlRecordingChunkScratch } from "../../../../lib/video-storage.js"; interface CompressionMeta { @@ -98,7 +107,15 @@ export default defineEventHandler(async (event: H3Event) => { compression?: CompressionMeta | null; requestStreaming?: boolean; mimeType?: string; + attemptId?: string; + uploadGenerationId?: string; + useGenerationFence?: boolean; } | null; + const recoveryEnabled = await isFeatureFlagEnabled(UPLOAD_RETRY_RESUME_FLAG, { + userEmail: ownerEmail, + userKey: ownerEmail, + orgId, + }); // Sanitize compression metadata. The recorder is the only client we trust // here, but the values land in Sentry extras — so we still bound them to @@ -121,6 +138,8 @@ export default defineEventHandler(async (event: H3Event) => { id: schema.recordings.id, status: schema.recordings.status, videoUrl: schema.recordings.videoUrl, + uploadAttemptId: schema.recordings.uploadAttemptId, + uploadGenerationId: schema.recordings.uploadGenerationId, }) .from(schema.recordings) .where( @@ -140,6 +159,35 @@ export default defineEventHandler(async (event: H3Event) => { return { error: "Recording is already ready" }; } + const requestedAttemptId = + typeof body?.attemptId === "string" && + body.attemptId.length > 0 && + body.attemptId.length <= 128 + ? body.attemptId + : null; + const existingAttemptId = existing.uploadAttemptId ?? null; + const existingGenerationId = existing.uploadGenerationId ?? null; + if (recoveryEnabled && existingAttemptId !== requestedAttemptId) { + setResponseStatus(event, 409); + return { + error: "A newer upload retry is already active.", + staleAttempt: true, + }; + } + const requestedGenerationId = + typeof body?.uploadGenerationId === "string" && + body.uploadGenerationId.length > 0 && + body.uploadGenerationId.length <= 128 + ? body.uploadGenerationId + : null; + if (recoveryEnabled && existingGenerationId !== requestedGenerationId) { + setResponseStatus(event, 409); + return { + error: "A newer upload generation is already active.", + staleAttempt: true, + }; + } + if ( await isMediaVerificationPending({ ownerEmail, @@ -150,15 +198,72 @@ export default defineEventHandler(async (event: H3Event) => { setResponseStatus(event, 409); return { error: "Recording is still being verified" }; } + if (existing.status !== "uploading" && existing.status !== "failed") { + setResponseStatus(event, 409); + return { error: "Recording upload is no longer resettable" }; + } - const cleared = await deleteAppStateByPrefix( - `recording-chunks-${recordingId}-`, + // Fence this reset before deleting any provider or buffered state. A + // retry that lost the token race must not tear down the winner's session. + const now = new Date().toISOString(); + // Only clients that can carry the returned generation may opt into the + // fence. Retry claims always opt in; legacy reset callers keep the null + // generation wire contract until they are upgraded. + const useGenerationFence = + recoveryEnabled && + (requestedAttemptId !== null || + requestedGenerationId !== null || + body?.useGenerationFence === true); + const nextGenerationId = useGenerationFence ? randomUUID() : null; + const uploadStateKey = `recording-upload-${recordingId}`; + const uploadStateSnapshot = await readAppState(uploadStateKey); + const reset = await db + .update(schema.recordings) + .set({ + status: "uploading", + failureReason: null, + uploadProgress: 0, + uploadGenerationId: nextGenerationId, + ...(!recoveryEnabled ? { uploadAttemptId: null } : {}), + uploadLeaseExpiresAt: uploadLeaseExpiry(), + updatedAt: now, + }) + .where( + and( + eq(schema.recordings.id, recordingId), + ownerEmailMatches(schema.recordings.ownerEmail, ownerEmail), + eq(schema.recordings.status, existing.status), + existingAttemptId === null + ? isNull(schema.recordings.uploadAttemptId) + : eq(schema.recordings.uploadAttemptId, existingAttemptId), + existingGenerationId === null + ? isNull(schema.recordings.uploadGenerationId) + : eq(schema.recordings.uploadGenerationId, existingGenerationId), + ), + ) + .returning({ id: schema.recordings.id }); + + if (reset.length !== 1) { + setResponseStatus(event, 409); + return { + error: "A newer upload retry is already active.", + staleAttempt: true, + }; + } + + const cleared = await deleteRecordingChunks( + ownerEmail, + recordingId, + existingGenerationId, ); // Clear any stale resumable session so a buffered retry does not // accidentally route through handleResumableChunk with stale offsets. - await deleteResumableSession(recordingId).catch(() => {}); + await deleteResumableSession(recordingId, existingGenerationId).catch( + () => {}, + ); let uploadMode: UploadMode = "buffered"; + let compensateStartedSession: (() => Promise) | null = null; if (body?.requestStreaming === true) { const mimeType = normalizeVideoMimeType(body.mimeType); if (!mimeType) { @@ -184,17 +289,33 @@ export default defineEventHandler(async (event: H3Event) => { mimeType, MAX_RECORDING_UPLOAD_BYTES, ); - await setResumableSession(recordingId, { - providerId: uploadProvider.id, - sessionId: session.sessionId, - meta: { - ...session.meta, - stableUrl: true, - recordAsset: false, + await setResumableSession( + recordingId, + { + providerId: uploadProvider.id, + sessionId: session.sessionId, + meta: { + ...session.meta, + stableUrl: true, + recordAsset: false, + }, + bytesUploaded: 0, + lastCommittedIndex: -1, }, - bytesUploaded: 0, - lastCommittedIndex: -1, - }); + nextGenerationId, + ); + compensateStartedSession = async () => { + if (!uploadProvider.resumable?.abortSession) { + throw new Error( + `Resumable upload provider ${uploadProvider.id} cannot abort the discarded retry session`, + ); + } + await uploadProvider.resumable.abortSession({ + sessionId: session.sessionId, + meta: session.meta, + }); + await deleteResumableSession(recordingId, nextGenerationId); + }; uploadMode = "streaming"; } catch (err) { if (!bufferedFallbackAvailable) { @@ -219,19 +340,64 @@ export default defineEventHandler(async (event: H3Event) => { } } + const resetLease = await renewUploadLease(recordingId, { + attemptId: recoveryEnabled ? existingAttemptId : null, + generationId: nextGenerationId, + }); + if (!resetLease.held) { + await compensateStartedSession?.(); + setResponseStatus(event, 409); + return { + error: "Recording upload changed while its retry was starting.", + staleAttempt: true, + }; + } + // Reset the per-recording upload progress so the UI poller sees the // re-upload restart from 0 and doesn't briefly show "100% then // re-running" on the post-compression chunked upload pass. - const now = new Date().toISOString(); - await writeAppState(`recording-upload-${recordingId}`, { - recordingId, - status: "uploading", - progress: 0, - chunksReceived: 0, - bytesReceived: 0, - maxBytes: MAX_RECORDING_UPLOAD_BYTES, - updatedAt: now, - }); + const uploadStateUpdated = await compareAndSetAppState( + uploadStateKey, + uploadStateSnapshot, + { + recordingId, + status: "uploading", + progress: 0, + chunksReceived: 0, + bytesReceived: 0, + uploadAttemptId: recoveryEnabled ? existingAttemptId : null, + uploadGenerationId: nextGenerationId, + maxBytes: MAX_RECORDING_UPLOAD_BYTES, + updatedAt: now, + }, + ); + if (!uploadStateUpdated) { + const [current] = await db + .select({ + status: schema.recordings.status, + uploadAttemptId: schema.recordings.uploadAttemptId, + uploadGenerationId: schema.recordings.uploadGenerationId, + }) + .from(schema.recordings) + .where( + and( + eq(schema.recordings.id, recordingId), + ownerEmailMatches(schema.recordings.ownerEmail, ownerEmail), + ), + ); + if ( + current?.status !== "uploading" || + (current.uploadAttemptId ?? null) !== + (recoveryEnabled ? existingAttemptId : null) || + (current.uploadGenerationId ?? null) !== nextGenerationId + ) { + setResponseStatus(event, 409); + return { + error: "Recording upload changed while its retry was starting.", + staleAttempt: true, + }; + } + } // Stash compression metadata under its own key. We don't merge it into // `recording-upload-{id}` because the recorder client overwrites that @@ -245,25 +411,13 @@ export default defineEventHandler(async (event: H3Event) => { }); } - await db - .update(schema.recordings) - .set({ - status: "uploading", - failureReason: null, - uploadProgress: 0, - // A retry re-takes the lease. Without this the row would go back to - // 'uploading' still carrying the expired lease that killed it. - uploadLeaseExpiresAt: uploadLeaseExpiry(), - updatedAt: now, - }) - .where(eq(schema.recordings.id, recordingId)); - return { ok: true, recordingId, chunksCleared: cleared, compressionRecorded: !!compression, uploadMode, + uploadGenerationId: nextGenerationId, }; }); }); diff --git a/templates/clips/server/routes/api/uploads/[recordingId]/resume.get.test.ts b/templates/clips/server/routes/api/uploads/[recordingId]/resume.get.test.ts index 694be72f1a..7505613df7 100644 --- a/templates/clips/server/routes/api/uploads/[recordingId]/resume.get.test.ts +++ b/templates/clips/server/routes/api/uploads/[recordingId]/resume.get.test.ts @@ -4,7 +4,13 @@ const mockRenewUploadLease = vi.hoisted(() => vi.fn()); const mockGetResumableSession = vi.hoisted(() => vi.fn()); const mockListRecordingChunkKeys = vi.hoisted(() => vi.fn()); const mockSumRecordingChunkBytes = vi.hoisted(() => vi.fn()); +const mockReadAppState = vi.hoisted(() => vi.fn()); +const mockWriteAppState = vi.hoisted(() => vi.fn()); +const mockCompareAndSetAppState = vi.hoisted(() => vi.fn()); const mockSetResponseStatus = vi.hoisted(() => vi.fn()); +const mockGetQuery = vi.hoisted(() => vi.fn()); +const mockIsFeatureFlagEnabled = vi.hoisted(() => vi.fn()); +const mockUpdateRows = vi.hoisted(() => ({ rows: [{ id: "rec-1" }] })); const mockSelectRows = vi.hoisted(() => ({ rows: [] as Array>, })); @@ -16,6 +22,26 @@ const mockDb = vi.hoisted(() => ({ }; return builder; }), + update: vi.fn(() => { + const builder = { + set: vi.fn(() => builder), + where: vi.fn(() => builder), + returning: vi.fn(async () => mockUpdateRows.rows), + }; + return builder; + }), +})); + +vi.mock("@agent-native/core/application-state", () => ({ + compareAndSetAppState: (...args: unknown[]) => + mockCompareAndSetAppState(...args), + readAppState: (...args: unknown[]) => mockReadAppState(...args), + writeAppState: (...args: unknown[]) => mockWriteAppState(...args), +})); + +vi.mock("@agent-native/core/feature-flags", () => ({ + isFeatureFlagEnabled: (...args: unknown[]) => + mockIsFeatureFlagEnabled(...args), })); vi.mock("@agent-native/core/server", () => ({ @@ -25,11 +51,13 @@ vi.mock("@agent-native/core/server", () => ({ vi.mock("drizzle-orm", () => ({ and: vi.fn(() => "and"), eq: vi.fn(() => "eq"), + isNull: vi.fn(() => "is-null"), })); vi.mock("h3", () => ({ defineEventHandler: (handler: unknown) => handler, getRouterParam: () => "rec-1", + getQuery: (...args: unknown[]) => mockGetQuery(...args), setResponseHeader: vi.fn(), setResponseStatus: (...args: unknown[]) => mockSetResponseStatus(...args), createError: ({ statusCode, message }: any) => @@ -66,6 +94,7 @@ vi.mock("../../../../lib/resumable-session.js", () => ({ vi.mock("../../../../lib/upload-lease.js", () => ({ renewUploadLease: (...args: unknown[]) => mockRenewUploadLease(...args), + uploadLeaseExpiry: () => "2099-01-01T00:00:00.000Z", })); import handler from "./resume.get"; @@ -74,10 +103,31 @@ describe("/api/uploads/:recordingId/resume route", () => { beforeEach(() => { vi.clearAllMocks(); mockSelectRows.rows = [{ id: "rec-1", status: "uploading" }]; + mockGetQuery.mockReturnValue({ attemptId: "client-attempt-0001" }); mockRenewUploadLease.mockResolvedValue({ held: true }); mockGetResumableSession.mockResolvedValue(null); mockListRecordingChunkKeys.mockResolvedValue([]); mockSumRecordingChunkBytes.mockResolvedValue(0); + mockReadAppState.mockResolvedValue({ progress: 50 }); + mockWriteAppState.mockResolvedValue(undefined); + mockCompareAndSetAppState.mockResolvedValue(true); + mockUpdateRows.rows = [{ id: "rec-1" }]; + mockIsFeatureFlagEnabled.mockResolvedValue(true); + }); + + it("leaves upload state untouched when resumable retry is disabled", async () => { + mockIsFeatureFlagEnabled.mockResolvedValue(false); + + await expect(handler({} as any)).resolves.toEqual({ + recoveryEnabled: false, + resumable: false, + recordingId: "rec-1", + status: null, + reason: "feature_disabled", + }); + expect(mockDb.select).not.toHaveBeenCalled(); + expect(mockDb.update).not.toHaveBeenCalled(); + expect(mockWriteAppState).not.toHaveBeenCalled(); }); it("reports the provider's committed offset for a streaming upload", async () => { @@ -90,11 +140,12 @@ describe("/api/uploads/:recordingId/resume route", () => { expect.objectContaining({ resumable: true, uploadMode: "streaming", + attemptId: "client-attempt-0001", bytesReceived: 4_194_304, nextChunkIndex: 4, }), ); - expect(mockRenewUploadLease).toHaveBeenCalledWith("rec-1"); + expect(mockDb.update).toHaveBeenCalledOnce(); }); it("resumes a buffered upload at the first gap, not after the highest index", async () => { @@ -109,6 +160,7 @@ describe("/api/uploads/:recordingId/resume route", () => { expect.objectContaining({ resumable: true, uploadMode: "buffered", + attemptId: "client-attempt-0001", bytesReceived: 15, nextChunkIndex: 2, }), @@ -116,12 +168,14 @@ describe("/api/uploads/:recordingId/resume route", () => { }); it("reports a terminal recording as not resumable", async () => { - mockRenewUploadLease.mockResolvedValue({ - held: false, - status: "failed", - failureReason: "Upload aborted by user", - videoUrl: null, - }); + mockSelectRows.rows = [ + { + id: "rec-1", + status: "failed", + failureReason: "Upload aborted by user", + videoUrl: null, + }, + ]; await expect(handler({} as any)).resolves.toEqual( expect.objectContaining({ @@ -132,6 +186,185 @@ describe("/api/uploads/:recordingId/resume route", () => { ); }); + it("atomically reclaims an interrupted upload with its existing session", async () => { + mockSelectRows.rows = [ + { + id: "rec-1", + status: "failed", + failureReason: + "Upload was interrupted. The local recording is safe; retry from the Clips desktop app.", + uploadProgress: 40, + }, + ]; + mockGetResumableSession.mockResolvedValue({ + bytesUploaded: 7_864_320, + lastCommittedIndex: 1, + }); + + await expect(handler({} as any)).resolves.toEqual( + expect.objectContaining({ + resumable: true, + status: "uploading", + bytesReceived: 7_864_320, + nextChunkIndex: 2, + }), + ); + expect(mockDb.update).toHaveBeenCalledOnce(); + expect(mockCompareAndSetAppState).toHaveBeenCalledWith( + "recording-upload-rec-1", + expect.objectContaining({ progress: 50 }), + expect.objectContaining({ + status: "uploading", + progress: 40, + bytesReceived: 7_864_320, + retryableInterruption: false, + }), + ); + expect(mockDb.update).toHaveBeenCalledOnce(); + }); + + it("claims a restart token when the prior provider session is gone", async () => { + mockSelectRows.rows = [ + { + id: "rec-1", + status: "failed", + failureReason: + "Upload was interrupted. The local recording is safe; retry from the Clips desktop app.", + }, + ]; + + await expect(handler({} as any)).resolves.toEqual( + expect.objectContaining({ + resumable: true, + status: "uploading", + uploadMode: "buffered", + attemptId: "client-attempt-0001", + }), + ); + expect(mockDb.update).toHaveBeenCalledOnce(); + expect(mockRenewUploadLease).not.toHaveBeenCalled(); + }); + + it("fences a concurrent retry that lost the writer-token claim", async () => { + mockUpdateRows.rows = []; + + await expect(handler({} as any)).resolves.toEqual({ + resumable: false, + recoveryEnabled: true, + recordingId: "rec-1", + status: "uploading", + reason: "retry_already_active", + }); + expect(mockSetResponseStatus).toHaveBeenCalledWith({}, 409); + expect(mockWriteAppState).not.toHaveBeenCalled(); + }); + + it("fails before claiming the row when upload state is unreadable", async () => { + mockReadAppState.mockRejectedValue(new Error("state store unavailable")); + + await expect(handler({} as any)).rejects.toThrow("state store unavailable"); + + expect(mockDb.update).not.toHaveBeenCalled(); + expect(mockCompareAndSetAppState).not.toHaveBeenCalled(); + expect(mockWriteAppState).not.toHaveBeenCalled(); + }); + + it("does not return a stale resume plan when interruption wins publication", async () => { + mockCompareAndSetAppState.mockImplementation(async () => { + mockSelectRows.rows = [ + { + id: "rec-1", + status: "failed", + uploadAttemptId: "client-attempt-0001", + uploadGenerationId: null, + }, + ]; + return false; + }); + + await expect(handler({} as any)).resolves.toEqual({ + resumable: false, + recoveryEnabled: true, + recordingId: "rec-1", + status: "failed", + reason: "upload_state_changed", + }); + + expect(mockSetResponseStatus).toHaveBeenCalledWith({}, 409); + expect(mockWriteAppState).not.toHaveBeenCalled(); + }); + + it("preserves newer progress published by the same active claim", async () => { + mockSelectRows.rows = [ + { + id: "rec-1", + status: "uploading", + uploadAttemptId: "client-attempt-0001", + uploadGenerationId: null, + }, + ]; + mockCompareAndSetAppState.mockResolvedValue(false); + const consoleInfo = vi + .spyOn(console, "info") + .mockImplementation(() => undefined); + + try { + await expect(handler({} as any)).resolves.toEqual( + expect.objectContaining({ resumable: true, status: "uploading" }), + ); + } finally { + consoleInfo.mockRestore(); + } + + expect(mockWriteAppState).toHaveBeenCalledWith("refresh-signal", { + ts: expect.any(Number), + }); + }); + + it("does not let a different claim steal an active retry", async () => { + mockSelectRows.rows = [ + { + id: "rec-1", + status: "uploading", + uploadAttemptId: "active-attempt-0001", + }, + ]; + + await expect(handler({} as any)).resolves.toEqual({ + resumable: false, + recoveryEnabled: true, + recordingId: "rec-1", + status: "uploading", + reason: "retry_already_active", + }); + expect(mockSetResponseStatus).toHaveBeenCalledWith({}, 409); + expect(mockDb.update).not.toHaveBeenCalled(); + }); + + it("lets the same claim re-read its offset after a lost response", async () => { + mockSelectRows.rows = [ + { + id: "rec-1", + status: "uploading", + uploadAttemptId: "client-attempt-0001", + }, + ]; + mockGetResumableSession.mockResolvedValue({ + bytesUploaded: 3_932_160, + lastCommittedIndex: 0, + }); + + await expect(handler({} as any)).resolves.toEqual( + expect.objectContaining({ + resumable: true, + attemptId: "client-attempt-0001", + bytesReceived: 3_932_160, + nextChunkIndex: 1, + }), + ); + expect(mockDb.update).toHaveBeenCalledOnce(); + }); + it("404s a recording the caller does not own", async () => { mockSelectRows.rows = []; @@ -141,4 +374,14 @@ describe("/api/uploads/:recordingId/resume route", () => { expect(mockSetResponseStatus).toHaveBeenCalledWith({}, 404); expect(mockRenewUploadLease).not.toHaveBeenCalled(); }); + + it("rejects a retry claim the client cannot safely reuse", async () => { + mockGetQuery.mockReturnValue({ attemptId: "short" }); + + await expect(handler({} as any)).rejects.toMatchObject({ + statusCode: 400, + message: "A valid upload retry attemptId is required", + }); + expect(mockDb.select).not.toHaveBeenCalled(); + }); }); diff --git a/templates/clips/server/routes/api/uploads/[recordingId]/resume.get.ts b/templates/clips/server/routes/api/uploads/[recordingId]/resume.get.ts index 88547e1606..de3244b895 100644 --- a/templates/clips/server/routes/api/uploads/[recordingId]/resume.get.ts +++ b/templates/clips/server/routes/api/uploads/[recordingId]/resume.get.ts @@ -11,17 +11,25 @@ * Route: GET /api/uploads/:recordingId/resume */ +import { + compareAndSetAppState, + readAppState, + writeAppState, +} from "@agent-native/core/application-state"; +import { isFeatureFlagEnabled } from "@agent-native/core/feature-flags"; import { runWithRequestContext } from "@agent-native/core/server"; -import { and, eq } from "drizzle-orm"; +import { and, eq, isNull } from "drizzle-orm"; import { createError, defineEventHandler, getRouterParam, + getQuery, setResponseHeader, setResponseStatus, type H3Event, } from "h3"; +import { UPLOAD_RETRY_RESUME_FLAG } from "../../../../../shared/feature-flags.js"; import { getDb, schema } from "../../../../db/index.js"; import { listRecordingChunkKeys, @@ -33,7 +41,11 @@ import { ownerEmailMatches, } from "../../../../lib/recordings.js"; import { getResumableSession } from "../../../../lib/resumable-session.js"; -import { renewUploadLease } from "../../../../lib/upload-lease.js"; +import { + isRetryableUploadInterruption, + RETRYABLE_UPLOAD_INTERRUPTION_REASON, +} from "../../../../lib/upload-interruption.js"; +import { uploadLeaseExpiry } from "../../../../lib/upload-lease.js"; export default defineEventHandler(async (event: H3Event) => { setResponseHeader(event, "Cache-Control", "private, max-age=0, no-store"); @@ -41,6 +53,21 @@ export default defineEventHandler(async (event: H3Event) => { if (!recordingId) { throw createError({ statusCode: 400, message: "Missing recordingId" }); } + const requestedAttemptIdValue = getQuery(event).attemptId; + const requestedAttemptId = Array.isArray(requestedAttemptIdValue) + ? requestedAttemptIdValue[0] + : requestedAttemptIdValue; + if ( + typeof requestedAttemptId !== "string" || + requestedAttemptId.length < 16 || + requestedAttemptId.length > 128 || + !/^[A-Za-z0-9_-]+$/.test(requestedAttemptId) + ) { + throw createError({ + statusCode: 400, + message: "A valid upload retry attemptId is required", + }); + } let ownerEmail: string; let orgId: string | undefined; @@ -52,6 +79,21 @@ export default defineEventHandler(async (event: H3Event) => { throw createError({ statusCode: 401, message: "Unauthorized" }); } + const recoveryEnabled = await isFeatureFlagEnabled(UPLOAD_RETRY_RESUME_FLAG, { + userEmail: ownerEmail, + userKey: ownerEmail, + orgId, + }); + if (!recoveryEnabled) { + return { + recoveryEnabled: false, + resumable: false, + recordingId, + status: null, + reason: "feature_disabled", + }; + } + return runWithRequestContext({ userEmail: ownerEmail, orgId }, async () => { const [recording] = await getDb() .select({ @@ -59,6 +101,9 @@ export default defineEventHandler(async (event: H3Event) => { status: schema.recordings.status, failureReason: schema.recordings.failureReason, videoUrl: schema.recordings.videoUrl, + uploadProgress: schema.recordings.uploadProgress, + uploadAttemptId: schema.recordings.uploadAttemptId, + uploadGenerationId: schema.recordings.uploadGenerationId, }) .from(schema.recordings) .where( @@ -73,31 +118,160 @@ export default defineEventHandler(async (event: H3Event) => { return { error: "Recording not found" }; } - const lease = await renewUploadLease(recordingId); - if (!lease.held) { + // Legacy rows keep their null generation and unscoped scratch. A reset + // upgrades them by installing a fresh generation before it deletes data. + const existingGenerationId = recording.uploadGenerationId ?? null; + const generationId = existingGenerationId; + const session = generationId + ? await getResumableSession(recordingId, generationId) + : await getResumableSession(recordingId); + const retryableFailure = + recording.status === "failed" && + isRetryableUploadInterruption(recording.failureReason); + const existingAttemptId = recording.uploadAttemptId ?? null; + if ( + recording.status === "uploading" && + existingAttemptId !== null && + existingAttemptId !== requestedAttemptId + ) { + setResponseStatus(event, 409); + return { + resumable: false, + recoveryEnabled: true, + recordingId, + status: "uploading", + reason: "retry_already_active", + }; + } + if (recording.status !== "uploading" && !retryableFailure) { + return { + resumable: false, + recoveryEnabled: true, + recordingId, + status: recording.status, + failureReason: recording.failureReason, + videoUrl: recording.videoUrl, + }; + } + + const uploadStateKey = `recording-upload-${recordingId}`; + const uploadStateRaw = await readAppState(uploadStateKey); + const uploadState = uploadStateRaw ?? {}; + const attemptId = requestedAttemptId; + const now = new Date().toISOString(); + const claimed = await getDb() + .update(schema.recordings) + .set({ + status: "uploading", + failureReason: null, + uploadAttemptId: attemptId, + ...(generationId ? { uploadGenerationId: generationId } : {}), + uploadLeaseExpiresAt: uploadLeaseExpiry(), + updatedAt: now, + }) + .where( + and( + eq(schema.recordings.id, recordingId), + ownerEmailMatches(schema.recordings.ownerEmail, ownerEmail), + retryableFailure + ? eq(schema.recordings.status, "failed") + : eq(schema.recordings.status, "uploading"), + retryableFailure + ? eq( + schema.recordings.failureReason, + RETRYABLE_UPLOAD_INTERRUPTION_REASON, + ) + : undefined, + existingAttemptId === null + ? isNull(schema.recordings.uploadAttemptId) + : eq(schema.recordings.uploadAttemptId, existingAttemptId), + existingGenerationId === null + ? isNull(schema.recordings.uploadGenerationId) + : eq(schema.recordings.uploadGenerationId, existingGenerationId), + ), + ) + .returning({ id: schema.recordings.id }); + + if (claimed.length !== 1) { + setResponseStatus(event, 409); return { resumable: false, + recoveryEnabled: true, recordingId, - status: lease.status, - failureReason: lease.failureReason, - videoUrl: lease.videoUrl, + status: "uploading", + reason: "retry_already_active", }; } - const session = await getResumableSession(recordingId).catch(() => null); + const uploadStateUpdated = await compareAndSetAppState( + uploadStateKey, + uploadStateRaw, + { + ...uploadState, + recordingId, + status: "uploading", + failureReason: null, + retryableInterruption: false, + progress: recording.uploadProgress, + uploadAttemptId: attemptId, + uploadGenerationId: generationId, + ...(session ? { bytesReceived: session.bytesUploaded } : {}), + updatedAt: now, + }, + ); + if (!uploadStateUpdated) { + const [current] = await getDb() + .select({ + status: schema.recordings.status, + uploadAttemptId: schema.recordings.uploadAttemptId, + uploadGenerationId: schema.recordings.uploadGenerationId, + }) + .from(schema.recordings) + .where( + and( + eq(schema.recordings.id, recordingId), + ownerEmailMatches(schema.recordings.ownerEmail, ownerEmail), + ), + ); + if ( + current?.status !== "uploading" || + (current.uploadAttemptId ?? null) !== attemptId || + (current.uploadGenerationId ?? null) !== generationId + ) { + setResponseStatus(event, 409); + return { + resumable: false, + recoveryEnabled: true, + recordingId, + status: current?.status ?? null, + reason: "upload_state_changed", + }; + } + console.info( + `[upload-resume] upload state advanced under the same claim; preserving newer progress for ${recordingId}`, + ); + } + await writeAppState("refresh-signal", { ts: Date.now() }); + if (session) { return { resumable: true, + recoveryEnabled: true, recordingId, - status: recording.status, + status: "uploading", uploadMode: "streaming" as const, + attemptId, + ...(generationId ? { uploadGenerationId: generationId } : {}), bytesReceived: session.bytesUploaded, nextChunkIndex: (session.lastCommittedIndex ?? -1) + 1, }; } const stored = new Set( - (await listRecordingChunkKeys(ownerEmail, recordingId)) + (generationId + ? await listRecordingChunkKeys(ownerEmail, recordingId, generationId) + : await listRecordingChunkKeys(ownerEmail, recordingId) + ) .map(recordingChunkIndexFromKey) .filter((index): index is number => index !== null), ); @@ -108,10 +282,15 @@ export default defineEventHandler(async (event: H3Event) => { return { resumable: true, + recoveryEnabled: true, recordingId, - status: recording.status, + status: "uploading", uploadMode: "buffered" as const, - bytesReceived: await sumRecordingChunkBytes(ownerEmail, recordingId), + attemptId, + ...(generationId ? { uploadGenerationId: generationId } : {}), + bytesReceived: generationId + ? await sumRecordingChunkBytes(ownerEmail, recordingId, generationId) + : await sumRecordingChunkBytes(ownerEmail, recordingId), nextChunkIndex, }; }); diff --git a/templates/clips/shared/feature-flags.ts b/templates/clips/shared/feature-flags.ts index dd5b3732bc..4bcfd0cb5b 100644 --- a/templates/clips/shared/feature-flags.ts +++ b/templates/clips/shared/feature-flags.ts @@ -28,7 +28,19 @@ export const CUSTOM_SCK_LIVE_UPLOAD_FLAG = defineFeatureFlag({ "Upload recording chunks while capture is still in progress. Requires the custom ScreenCaptureKit pipeline.", }); +/** + * Desktop app and hosted upload routes: preserve interrupted resumable + * sessions and continue saved retries from the provider-confirmed offset. + */ +export const UPLOAD_RETRY_RESUME_FLAG = defineFeatureFlag({ + key: "uploadRetryResume", + displayName: "Resumable upload retry", + description: + "Resume interrupted desktop uploads from their last confirmed byte instead of replaying the full local backup.", +}); + export const CLIPS_FEATURE_FLAGS = defineFeatureFlags([ USE_CUSTOM_SCK_PIPELINE_FLAG, CUSTOM_SCK_LIVE_UPLOAD_FLAG, + UPLOAD_RETRY_RESUME_FLAG, ]); diff --git a/templates/clips/shared/recording-core.test.ts b/templates/clips/shared/recording-core.test.ts index 43bdeebc0b..8967a85117 100644 --- a/templates/clips/shared/recording-core.test.ts +++ b/templates/clips/shared/recording-core.test.ts @@ -27,6 +27,7 @@ describe("recording upload URL helpers", () => { height: 2954.7, hasAudio: true, hasCamera: false, + uploadGenerationId: "generation-1", }), ); @@ -35,6 +36,7 @@ describe("recording upload URL helpers", () => { expect(params.get("height")).toBe("2955"); expect(params.get("hasAudio")).toBe("1"); expect(params.get("hasCamera")).toBe("0"); + expect(params.get("uploadGenerationId")).toBe("generation-1"); }); it("omits null, empty, and non-finite upload metadata", () => { @@ -54,6 +56,7 @@ describe("recording upload URL helpers", () => { height: null, hasAudio: true, hasCamera: false, + uploadGenerationId: "", }), ); @@ -62,5 +65,6 @@ describe("recording upload URL helpers", () => { expect(params.get("height")).toBeNull(); expect(params.get("hasAudio")).toBe("1"); expect(params.get("hasCamera")).toBe("0"); + expect(params.get("uploadGenerationId")).toBeNull(); }); }); diff --git a/templates/clips/shared/recording-core.ts b/templates/clips/shared/recording-core.ts index 58a6fb02b9..f8ffea46c4 100644 --- a/templates/clips/shared/recording-core.ts +++ b/templates/clips/shared/recording-core.ts @@ -76,6 +76,7 @@ export type ChunkUploadParams = { height?: number | null; hasAudio?: boolean; hasCamera?: boolean; + uploadGenerationId?: string; }; export function normalizeChunkUploadNumber(value: unknown): number | undefined { @@ -115,6 +116,9 @@ export function chunkUploadQuery(params: ChunkUploadParams): string { if (params.hasCamera !== undefined) { q.set("hasCamera", params.hasCamera ? "1" : "0"); } + if (params.uploadGenerationId) { + q.set("uploadGenerationId", params.uploadGenerationId); + } return q.toString(); }