fix(downloads): iOS image-model download stuck failed — durable staging + retry re-download#557
Closed
alichherawalla wants to merge 7 commits into
Closed
fix(downloads): iOS image-model download stuck failed — durable staging + retry re-download#557alichherawalla wants to merge 7 commits into
alichherawalla wants to merge 7 commits into
Conversation
…se scripts uat.sh (beta) and release.sh (production/local) now post the release notes to Slack via scripts/notify-slack-release.mjs after the GitHub release is cut, before the notes tempfile is removed. Fail-soft (|| true) so a chat post can never fail a shipped build. The webhook is read from the gitignored .env.keygen (the local secret store that already holds the RevenueCat/Resend/Keygen/Cloudflare tokens), mirroring the SLACK_WEBHOOK_URL repo secret the CI release workflows use. Extracted with a scoped sed for that one key, not a full source, so the other secrets in .env.keygen stay out of the script env. notify-slack-release.mjs no-ops when the webhook is unset, so this is a no-op on machines without it.
release.sh omitted CHANNEL_LABEL, so prod announcements rendered with no channel tag while uat.sh betas showed `beta`. Pass CHANNEL_LABEL=stable for parity — the notify script's documented values are beta | stable.
…STemporaryDirectory Root cause of the SDXL (Core ML) image-model download failing on iOS with "...coreml_apple_...xl-base-ios couldn't be opened because there is no such file": a completed single-file download was staged in NSTemporaryDirectory(), which iOS reaps across app relaunches and under memory pressure. The staged zip's path is persisted (localUri) and finalize (moveCompletedDownload -> unzip) can now run AFTER a relaunch (the queued-survival rework), by which point the OS has already deleted the temp file -> unzip opens a zip that no longer exists. A multi-GB image zip is the worst case: it commonly spans a backgrounding/ relaunch before its unzip finishes. Since CoreML is iOS-only, the shared download-infra change surfaced it only on iOS. Stage completed downloads (both the URLSession delegate hand-off copy and the handleSingleFileCompletion move) in a DURABLE Documents/.download-staging dir that survives relaunch + memory pressure, excluded from iCloud backup (dir flag is not inherited, so the staged file is excluded per-file too). Native change — device-verified separately (jest fakes the native module).
…ble on retry When an image zip download completed but its staged bytes are gone at finalize (iOS temp-purge on pre-durable-staging builds, or the user cleared storage), resumeZipDownload rethrew moveCompletedDownload's "no such file" and the row stuck at 'failed'. Retry re-ran the same finalize on the same dead download, so tapping Retry did nothing (retryImageDownload's tryResumeImageFinalization handles it and never falls through to the fresh-download path). Now, when neither a valid zip nor an extracted dir survives, resumeZipDownload re-downloads from scratch via proceedWithDownload (descriptor reconstructed from the entry's persisted metadata) instead of dead-ending. The user recovers by re-downloading rather than being stuck on a permanently-failed card. Red-flow test drives the real retry-finalize path over a faked native boundary where moveCompletedDownload rejects (staged source purged) and no artifact survives; asserts a fresh native download is issued + the row goes active again. Verified red (reverted fallback -> stuck 'failed', no new row) then green.
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
📝 WalkthroughWalkthroughCompleted iOS downloads now use durable Documents-based staging. Resume handling reconstructs image descriptors from persisted metadata and starts a fresh download when staged ZIP bytes are unavailable. Integration tests cover recovery, while release scripts add fail-soft Slack announcements. ChangesiOS Download Recovery
Release Notifications
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant DownloadManagerScreen
participant resumeImageDownload
participant moveCompletedDownload
participant proceedWithDownload
participant NativeDownloadManager
DownloadManagerScreen->>resumeImageDownload: retry failed image download
resumeImageDownload->>moveCompletedDownload: move completed staged artifact
moveCompletedDownload-->>resumeImageDownload: return missing-file error
resumeImageDownload->>proceedWithDownload: reconstruct descriptor from metadata
proceedWithDownload->>NativeDownloadManager: start fresh native download
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ios/DownloadManagerModule.swift`:
- Around line 1332-1337: Clean up staged .tmp files in the completion and
startup flows: update handleSingleFileCompletion and
handleMultiFileTaskCompletion to remove location when moving fails or
before/after the multi-file copy fallback, and initialize cleanup in or
immediately after setupSession() to delete orphaned .tmp files from
completedStagingDirectory().
In `@src/screens/ModelsScreen/imageDownloadResume.ts`:
- Around line 92-94: Update the failure message in the resume-download flow near
setStatus to replace the em dash with a period, preserving the existing meaning
as two separate sentences.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a472f777-63d6-4ab6-be5e-e636679b8e48
📒 Files selected for processing (3)
__tests__/integration/downloads/iosImageStagingPurgedRedownloads.redflow.test.tsios/DownloadManagerModule.swiftsrc/screens/ModelsScreen/imageDownloadResume.ts
Comment on lines
+1332
to
+1337
| // We must copy it to a safe location SYNCHRONOUSLY before returning. That location must be | ||
| // DURABLE (not NSTemporaryDirectory) — if the app is killed between here and handleCompletion, | ||
| // a temp copy would be reaped and the completed bytes lost. See completedStagingDirectory(). | ||
| let fileManager = FileManager.default | ||
| let safeTmp = NSTemporaryDirectory() + "dl_task_\(downloadTask.taskIdentifier)_\(UUID().uuidString).tmp" | ||
| let stagingDir = DownloadManagerModule.completedStagingDirectory() | ||
| let safeTmp = "\(stagingDir)/dl_task_\(downloadTask.taskIdentifier)_\(UUID().uuidString).tmp" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Resource leak: .tmp files are not cleaned up on failure or app termination.
Because .download-staging is a durable directory (unlike NSTemporaryDirectory), any .tmp files left here will persist indefinitely. These are multi-GB model artifacts, so leaking them will quickly exhaust device storage.
Currently, safeTmp files leak in three scenarios:
- Single-file error: If
moveItemfails inhandleSingleFileCompletion,location(the.tmpfile) is left behind. - Multi-file fallback: If
moveItemfails inhandleMultiFileTaskCompletionand falls back tocopyItem,locationis left behind regardless of whether the copy succeeds or fails. - App termination: If the app is killed between
didFinishDownloadingToand the asynchandleCompletionblock, the.tmpfile is orphaned. Since URLSession foreground tasks do not survive restart, the file will never be processed.
To fix this, clean up location in the error/fallback paths of the completion handlers, and clear any orphaned .tmp files when initializing the session.
♻️ Proposed fixes to prevent `.tmp` file leaks
1. Clean up in handleSingleFileCompletion (around line 1238):
} catch {
NSLog("[DownloadManager] Failed to move single file: %@", error.localizedDescription)
+ try? fileManager.removeItem(at: location)
info.status = "failed"
taskToDownloadId.removeValue(forKey: taskId)2. Clean up in handleMultiFileTaskCompletion (around line 1151):
} catch {
NSLog("[DownloadManager] Failed to save file %@: %@",
fileTask.relativePath, error.localizedDescription)
}
+ try? fileManager.removeItem(at: location)
}
fileTask.completed = true3. Clean up orphaned .tmp files on startup (add inside or immediately after setupSession()):
queue.async(flags: .barrier) {
let stagingDir = DownloadManagerModule.completedStagingDirectory()
if let files = try? FileManager.default.contentsOfDirectory(atPath: stagingDir) {
for file in files where file.hasSuffix(".tmp") {
try? FileManager.default.removeItem(atPath: "\(stagingDir)/\(file)")
}
}
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ios/DownloadManagerModule.swift` around lines 1332 - 1337, Clean up staged
.tmp files in the completion and startup flows: update
handleSingleFileCompletion and handleMultiFileTaskCompletion to remove location
when moving fails or before/after the multi-file copy fallback, and initialize
cleanup in or immediately after setupSession() to delete orphaned .tmp files
from completedStagingDirectory().
Comment on lines
+92
to
+94
| useDownloadStore.getState().setStatus(ctx.entry.downloadId, 'failed', { | ||
| message: 'Download expired and could not be re-downloaded — remove and download again.', | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Avoid using em dashes in UI strings.
As per coding guidelines, UI strings and content must avoid em dashes. Consider using a period to separate the clauses into two distinct sentences.
📝 Proposed fix
- message: 'Download expired and could not be re-downloaded — remove and download again.',
+ message: 'Download expired and could not be re-downloaded. Remove and download again.',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| useDownloadStore.getState().setStatus(ctx.entry.downloadId, 'failed', { | |
| message: 'Download expired and could not be re-downloaded — remove and download again.', | |
| }); | |
| useDownloadStore.getState().setStatus(ctx.entry.downloadId, 'failed', { | |
| message: 'Download expired and could not be re-downloaded. Remove and download again.', | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/screens/ModelsScreen/imageDownloadResume.ts` around lines 92 - 94, Update
the failure message in the resume-download flow near setStatus to replace the em
dash with a period, preserving the existing meaning as two separate sentences.
Source: Coding guidelines
…uard on both platforms Addresses hygiene/tests self-audit on the retry-recovery fix: - DRY: extract imageDescriptorFromMetadata into a pure zero-IO module (imageDescriptor.ts), used by BOTH the iOS retry path (retryHandlers) and resume's re-download fallback, instead of two hand-built descriptors that could drift. - Copy: drop the em dash from the user-facing re-download failure message (brand guide). - Tests: resumeImageDownload is shared (iOS retry + Android app-open resume), so the red-flow now runs the purged-artifact recovery on BOTH ios and android with a device-faithful fixture each (CoreML zip / MNN zip). Red-verified on both (reverted fallback -> stuck 'failed', no new row).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/screens/ModelsScreen/imageDescriptor.ts`:
- Around line 8-23: Update imageDescriptorFromMetadata so the required
ImageModelDescriptor name field uses an empty-string fallback when
meta.imageModelName is nullish, matching the existing handling of other string
properties.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e40fd5af-4383-474c-ad85-ffd65e3138e5
📒 Files selected for processing (4)
__tests__/integration/downloads/iosImageStagingPurgedRedownloads.redflow.test.tssrc/screens/DownloadManagerScreen/retryHandlers.tssrc/screens/ModelsScreen/imageDescriptor.tssrc/screens/ModelsScreen/imageDownloadResume.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/integration/downloads/iosImageStagingPurgedRedownloads.redflow.test.ts
- src/screens/ModelsScreen/imageDownloadResume.ts
Comment on lines
+8
to
+23
| export function imageDescriptorFromMetadata(modelId: string, meta: Record<string, any>): ImageModelDescriptor { | ||
| return { | ||
| id: modelId, | ||
| name: meta.imageModelName, | ||
| description: meta.imageModelDescription ?? '', | ||
| downloadUrl: meta.imageModelDownloadUrl ?? '', | ||
| size: meta.imageModelSize ?? 0, | ||
| style: meta.imageModelStyle ?? '', | ||
| backend: meta.imageModelBackend ?? 'coreml', | ||
| attentionVariant: meta.imageModelAttentionVariant, | ||
| huggingFaceRepo: meta.imageModelRepo, | ||
| huggingFaceFiles: meta.imageModelHuggingFaceFiles, | ||
| coremlFiles: meta.imageModelCoremlFiles, | ||
| repo: meta.imageModelRepo, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Provide a fallback for the required name property.
The name property is required by the ImageModelDescriptor interface, but meta.imageModelName is mapped without a default. If it is undefined in the persisted metadata, it could violate the downstream contract. Consider adding a fallback (e.g., ?? '') for consistency with the other string properties.
🐛 Proposed fix
- name: meta.imageModelName,
+ name: meta.imageModelName ?? '',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function imageDescriptorFromMetadata(modelId: string, meta: Record<string, any>): ImageModelDescriptor { | |
| return { | |
| id: modelId, | |
| name: meta.imageModelName, | |
| description: meta.imageModelDescription ?? '', | |
| downloadUrl: meta.imageModelDownloadUrl ?? '', | |
| size: meta.imageModelSize ?? 0, | |
| style: meta.imageModelStyle ?? '', | |
| backend: meta.imageModelBackend ?? 'coreml', | |
| attentionVariant: meta.imageModelAttentionVariant, | |
| huggingFaceRepo: meta.imageModelRepo, | |
| huggingFaceFiles: meta.imageModelHuggingFaceFiles, | |
| coremlFiles: meta.imageModelCoremlFiles, | |
| repo: meta.imageModelRepo, | |
| }; | |
| } | |
| export function imageDescriptorFromMetadata(modelId: string, meta: Record<string, any>): ImageModelDescriptor { | |
| return { | |
| id: modelId, | |
| name: meta.imageModelName ?? '', | |
| description: meta.imageModelDescription ?? '', | |
| downloadUrl: meta.imageModelDownloadUrl ?? '', | |
| size: meta.imageModelSize ?? 0, | |
| style: meta.imageModelStyle ?? '', | |
| backend: meta.imageModelBackend ?? 'coreml', | |
| attentionVariant: meta.imageModelAttentionVariant, | |
| huggingFaceRepo: meta.imageModelRepo, | |
| huggingFaceFiles: meta.imageModelHuggingFaceFiles, | |
| coremlFiles: meta.imageModelCoremlFiles, | |
| repo: meta.imageModelRepo, | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/screens/ModelsScreen/imageDescriptor.ts` around lines 8 - 23, Update
imageDescriptorFromMetadata so the required ImageModelDescriptor name field uses
an empty-string fallback when meta.imageModelName is nullish, matching the
existing handling of other string properties.
…rd, assert recovery Mounts the real DownloadManagerScreen over the native+fs fakes, taps the actual failed-retry-button on the failed SDXL card, and asserts the card recovers (row no longer 'failed', Retry gone, a fresh native download in flight). Closes the /tests gap: the sibling .redflow.test.ts drives the service directly; this drives the real UI gesture through the full retry wiring. Pins Platform.OS=ios (the device bug) so imageProvider.retry takes the iOS re-download path, not Android's native-resume branch. Red-verified (reverted the reDownloadFromMetadata fallback -> card stuck 'failed'). registerCoreDownloadProviders() before mount, else the service silently refuses retry (no owning provider).
… fix/ios-download-durable-staging
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
__tests__/integration/downloads/iosImageStagingPurgedRedownloads.rendered.redflow.test.tsx (1)
42-42: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse the
modelKeyvariable.Since
makeImageModelKey(modelId)already constructsimage:${modelId}, you can safely reuse themodelKeyvariable here to eliminate the duplicated string interpolation.💡 Proposed refactor
- modelKey, downloadId: 'dl-sdxl', modelId: `image:${modelId}`, fileName, + modelKey, downloadId: 'dl-sdxl', modelId: modelKey, fileName,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@__tests__/integration/downloads/iosImageStagingPurgedRedownloads.rendered.redflow.test.tsx` at line 42, Update the download object in the test to reuse the existing modelKey variable for modelId instead of reconstructing image:${modelId} with a template string. Keep the surrounding downloadId and fileName assignments unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@__tests__/integration/downloads/iosImageStagingPurgedRedownloads.rendered.redflow.test.tsx`:
- Around line 77-82: Move the failed-retry-button and boundary active-row
assertions into the existing waitFor callback alongside the download status
assertion. Keep retrying until the store status, rendered UI, and boundary rows
all reflect successful recovery, while preserving the current assertion
conditions.
---
Nitpick comments:
In
`@__tests__/integration/downloads/iosImageStagingPurgedRedownloads.rendered.redflow.test.tsx`:
- Line 42: Update the download object in the test to reuse the existing modelKey
variable for modelId instead of reconstructing image:${modelId} with a template
string. Keep the surrounding downloadId and fileName assignments unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2c05c7c5-7860-4e80-9c2d-88e165bc251b
📒 Files selected for processing (4)
__tests__/integration/downloads/iosImageStagingPurgedRedownloads.rendered.redflow.test.tsxscripts/release.shscripts/uat.shsrc/screens/ModelsScreen/imageDownloadResume.ts
💤 Files with no reviewable changes (1)
- src/screens/ModelsScreen/imageDownloadResume.ts
Comment on lines
+77
to
+82
| await waitFor(() => { | ||
| expect(useDownloadStore.getState().downloads[modelKey].status).not.toBe('failed'); | ||
| }, { timeout: 5000 }); | ||
| expect(view.queryByTestId('failed-retry-button')).toBeNull(); | ||
| const rows = boundary.download!.active(); | ||
| expect(rows.some(r => r.modelId === `image:${modelId}` || r.fileName === fileName)).toBe(true); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Move all recovery assertions inside waitFor to prevent test flakiness.
React state updates and renders can be batched and asynchronous. Waiting only for the Zustand store to update does not guarantee that React has finished committing the corresponding UI changes. Asserting the DOM synchronously immediately after the store update can lead to a race condition where the UI still reflects the old state.
Moving the UI and boundary assertions into the waitFor block ensures both the state and the DOM are fully updated and synchronized before proceeding. As per coding guidelines, integration tests should "assert what the user sees", and asserting the UI directly inside the asynchronous wait properly aligns with this principle.
💚 Proposed fix to group assertions
await waitFor(() => {
expect(useDownloadStore.getState().downloads[modelKey].status).not.toBe('failed');
- }, { timeout: 5000 });
- expect(view.queryByTestId('failed-retry-button')).toBeNull();
- const rows = boundary.download!.active();
- expect(rows.some(r => r.modelId === `image:${modelId}` || r.fileName === fileName)).toBe(true);
+ expect(view.queryByTestId('failed-retry-button')).toBeNull();
+ const rows = boundary.download!.active();
+ expect(rows.some(r => r.modelId === `image:${modelId}` || r.fileName === fileName)).toBe(true);
+ }, { timeout: 5000 });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| await waitFor(() => { | |
| expect(useDownloadStore.getState().downloads[modelKey].status).not.toBe('failed'); | |
| }, { timeout: 5000 }); | |
| expect(view.queryByTestId('failed-retry-button')).toBeNull(); | |
| const rows = boundary.download!.active(); | |
| expect(rows.some(r => r.modelId === `image:${modelId}` || r.fileName === fileName)).toBe(true); | |
| await waitFor(() => { | |
| expect(useDownloadStore.getState().downloads[modelKey].status).not.toBe('failed'); | |
| expect(view.queryByTestId('failed-retry-button')).toBeNull(); | |
| const rows = boundary.download!.active(); | |
| expect(rows.some(r => r.modelId === `image:${modelId}` || r.fileName === fileName)).toBe(true); | |
| }, { timeout: 5000 }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@__tests__/integration/downloads/iosImageStagingPurgedRedownloads.rendered.redflow.test.tsx`
around lines 77 - 82, Move the failed-retry-button and boundary active-row
assertions into the existing waitFor callback alongside the download status
assertion. Keep retrying until the store status, rendered UI, and boundary rows
all reflect successful recovery, while preserving the current assertion
conditions.
Source: Coding guidelines
|
Collaborator
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug (device, production build)
SDXL (Core ML) image download showed 2.8 GB / 2.8 GB then failed with "…coreml_apple_…xl-base-ios couldn't be opened because there is no such file", and Retry did nothing. The bytes downloaded fine — it died in finalize (
unzipcouldn't find the zip), and retry kept re-running the same dead finalize.CoreML is iOS-only, so the recent Android download-durability work didn't touch CoreML directly — it exposed a latent bug in the shared download-finalize plumbing underneath it.
Root cause (native) + fix
ios/DownloadManagerModule.swiftstaged a completed single-file download inNSTemporaryDirectory(), which iOS reaps across relaunch / under memory pressure. The staged path is persisted (localUri) and finalize can now run after a relaunch (the queued-survival rework) — when the temp file is already gone. A multi-GB zip is the worst case.Fix: stage completed downloads in a durable
Documents/.download-stagingdir (survives relaunch + memory pressure, excluded from iCloud backup).Retry dead-end (JS) + fix
retryImageDownload→tryResumeImageFinalizationre-ranmoveCompletedDownloadon the stale id → same "no such file" every tap, and never fell through to the fresh-download path. Fix: when neither a valid zip nor an extracted dir survives,resumeZipDownloadre-downloads from scratch instead of rethrowing.Android safety
Zero Android-native files changed. Swift is iOS-only; the one shared JS file (
imageDownloadResume.ts) is a strictly-additive graceful fallback (recover-vs-dead-end), which improves Android's zip resume too. Verified:tscclean, download+image suites 174 passed, Android JS bundle builds.Test
iosImageStagingPurgedRedownloads.redflow.test.ts— drives the real retry-finalize path over a faked native boundary (moveCompletedDownload rejects, no artifact survives); asserts a fresh native download is issued + the row goes active. Red-verified (reverted fallback → stuckfailed, no new row) → green. Blast radius: 243 passed.Verification
npm run ios:deviceon a connected iPhone).Summary by CodeRabbit
Also folded in: Slack release announcements (was PR #556)
Merged
chore/slack-release-notifyinto this branch (disjoint file-set — onlyscripts/uat.sh+scripts/release.sh, zero overlap with the download fix, clean merge, no conflicts):uat.sh(beta) andrelease.sh(prod) now post release notes to Slack vianotify-slack-release.mjsafter the GitHub release is cut. Fail-soft (|| true)..env.keygen(scopedsed, one key). No-ops if unset.workflow_dispatch, not the tag these local scripts cut.#556 is now subsumed by this PR and can be closed once this merges.