Repository navigation
fix: recover failed and legacy Loom imports - #2331
richiemcilroy wants to merge 26 commits into
Conversation
richiemcilroy
changed the title
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
hey @greptileai, please re-review the PR |
|
@greptileai review |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
![capy-ai[bot]](https://avatars.githubusercontent.com/in/1915919?s=60&v=4){kind=small}
|
hey @greptileai, please re-review the PR |
| uploadUpdatedAt: Date; | ||
| receipt: VideoMetadata["loomImportRun"]; | ||
| }): Promise<FailedVideoProcessingRecoveryStatus> { | ||
| if (await isLoomImportRunning(receipt)) return "already-claimed"; |
There was a problem hiding this comment.
Stale uncertain receipt permanently blocks Loom import recovery
Medium Severity
The new gate at line 149 treats an unqueryable run as live forever: isLoomImportRunning returns true on any getRun rejection and has no lease or expiry, and the PR's own test at loom-import-start.test.ts:199 pins that behaviour for a not-found error. Dispatch is unconfirmed in two reachable cases: startLoomImportWorkflow writes the pending receipt before calling world.queue, so a crash between those calls leaves pending with a never-dispatched runId, and a queue call failing with a non-rejection error (timeout, 5xx) records uncertain. If no run exists, getRun keeps rejecting on every fifteen-minute cron tick, the gate returns already-claimed, and updatedAt never advances so the row keeps matching the stale-upload query. Nothing clears loomImportRun once written, so the import is stranded at Importing from Loom with no error shown, and no path recovers it: manual retry returns a false already-processing because retry-processing.ts:85 uses the same helper, and re-importing returns already been imported at actions/loom.ts:461. Lines 196-198 compound this by keeping the claim on a non-retryable failure, moving the row to phase processing, which neither the pending nor failed query selects again. Bound the receipt check with a lease so recovery can reclaim an import whose run never materialised.
|
hey @greptileai, please re-review the PR |
|
hey @greptileai, please re-review the PR |
|
hey @greptileai, please re-review the PR |
|
hey @greptileai, please re-review the PR |
|
hey @greptileai, please re-review the PR |
|
hey @greptileai, please re-review the PR |
|
hey @greptileai, please re-review the PR |
Loom imports could misclassify temporary source failures as private videos, lose streaming segment credentials, and reject recovery as duplicates. Share source resolution between the action and workflow, preserve access credentials, and retry temporary lookup failures durably. Failed imports retain their video IDs, ownership and placement, and reuse saved originals.
Recovery and safety
10 GB imports
Support originals and processed output up to 10 GiB (10,737,418,240 bytes) with 64 MiB S3 multipart parts. Bind callbacks to the registered workflow attempt, recording, owner, organization, object key and upload ID. Conditional completion preserves existing objects. Retries reuse a saved output only after local full decode and remote identity, size and SHA-256 verification, without uploading it again. Preview failure is optional only on that verified reuse path. Failed uploads clean up incomplete reservations while retaining committed originals. Enforce the limit during direct and HLS downloads and before output upload.
Worker capability checks wait durably during rollout. Deploy the media-server change to all replicas before the web change. This PR does not establish production import completion, authorize deployment, or clear uncertain dispatches based on age.
Current-main integration
Integrated main at 0069c7e while preserving its cancellation, shutdown drain, temporary-file cleanup, processing heartbeat, ordinary parallel-download behavior and latest redacted verification/NVENC diagnostics. A bounded Loom import bypassed its byte limit when combined with the newer parallel-download path; bounded imports now use the size-checked path. The new regression test fails without the fix and passes with it. Stalled Loom recovery now checks the recorded workflow and worker state, claims the unchanged attempt and uses the shared receipt-saving startup path. Active, uncertain and legacy attempts remain held for reconciliation. Confirmed-inactive processing attempts can recover directly through the guarded startup path; active and uncertain checks rotate through the bounded queue so old holds cannot starve newer imports. Regression tests cover an eight-hold queue across two scans and preserving originals after rejected startup.
Verification
Deploy the verified media image to every serving replica before releasing the web change. Production canary, customer source/mapping reconciliation and playback verification remain separate release gates.
The PR appears safe to merge; no new actionable defects were established.Summary
This PR adds guarded recovery for failed and legacy Loom imports.
The current code addresses the failures described in the earlier threads. Those threads were unnumbered, so they have no numbered entries above. This review inspected code and tests; it did not execute tests.
Reviews (12) · Last reviewed commit: "merge: preserve current media verificati..." · Reviewed by Greptile