Fix gallery sync not saving photos to phone camera roll (OS-1472)#3102
Open
nic-olo wants to merge 2 commits into
Open
Fix gallery sync not saving photos to phone camera roll (OS-1472)#3102nic-olo wants to merge 2 commits into
nic-olo wants to merge 2 commits into
Conversation
- MediaLibraryPermissions: check PHOTO_LIBRARY_ADD_ONLY on iOS 14+ so users who granted 'Add Photos Only' are no longer blocked from saving. requestPermission() now requests add-only first (least-privilege per Apple guidelines) then falls back to full PHOTO_LIBRARY. - mediaProcessingQueue: throw after onFileFailed when camera roll save fails so processLoop takes the failure path. Previously processItem called onFileFailed but did not throw, causing processLoop to immediately call onFileProcessed and continue to steps 7-9 (metadata save + glasses deletion). Files now stay on glasses until the user fixes permissions and re-syncs. - gallerySyncService: show a Settings-redirect alert in onSyncComplete when failedFiles is non-empty and camera roll permission is denied, so users know why their photos did not reach the camera roll. - CrustModule.swift: add PHPhotoLibrary.authorizationStatus pre-check before performChanges. Returns a clear error string if the native layer is reached without authorization instead of relying on PHKit's completion-handler error. Co-authored-by: Nicolo Micheletti <michelettinik@gmail.com>
📋 PR Review Helper📱 Mobile App Build✅ Ready to test! (commit 🕶️ ASG Client Build⏳ Waiting for build... 🔀 Test Locallygh pr checkout 3102 |
Member
Author
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db72411db7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| let isAuthorized: Bool | ||
| if #available(iOS 14, *) { | ||
| isAuthorized = authStatus == .authorized || authStatus == .limited || authStatus == .addOnly |
There was a problem hiding this comment.
When building the iOS Crust module, this comparison uses .addOnly as if it were a PHAuthorizationStatus, but Apple's PHAuthorizationStatus cases are statuses like .authorized and .limited; .addOnly is a PHAccessLevel. This will prevent the iOS native module from compiling for the gallery-sync fix path, so the add-only authorizationStatus(for:) result should be accepted via the returned status instead.
Useful? React with 👍 / 👎.
Deploying mentra-live-ota-site with
|
| Latest commit: |
d0bed47
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://3a7f99c0.mentra-live-ota-site.pages.dev |
| Branch Preview URL: | https://cursor-fix-gallery-camera-ro.mentra-live-ota-site.pages.dev |
Deploying mentra-store-dev with
|
| Latest commit: |
d0bed47
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://912f55cf.augmentos-appstore-2.pages.dev |
| Branch Preview URL: | https://cursor-fix-gallery-camera-ro.augmentos-appstore-2.pages.dev |
Deploying dev-augmentos-console with
|
| Latest commit: |
d0bed47
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://28208e4c.dev-augmentos-console.pages.dev |
| Branch Preview URL: | https://cursor-fix-gallery-camera-ro.dev-augmentos-console.pages.dev |
Deploying prod-augmentos-account with
|
| Latest commit: |
d0bed47
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://19baaa3e.augmentos-e84.pages.dev |
| Branch Preview URL: | https://cursor-fix-gallery-camera-ro.augmentos-e84.pages.dev |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
cubic analysis
1 issue found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="mobile/src/services/asg/gallerySyncService.ts">
<violation number="1" location="mobile/src/services/asg/gallerySyncService.ts:1977">
P2: Camera-roll permission alert is triggered from generic `failedFiles`, so unrelated sync failures can be mislabeled as permission problems.</violation>
</file>
Linked issue analysis
Linked issue: OS-1472: [Gallery] Sync shows in Mentra app but not saved to phone camera roll (red ! on thumbnail)
| Status | Acceptance criteria | Notes |
|---|---|---|
| ❌ | Reproduce on Android (Samsung Fold class device) with the user’s settings matrix | PR and diffs focus on iOS permission handling and failure-path behavior; there is no Android-specific repro/test evidence or Samsung-specific fixes in this change set. |
| ✅ | Synced photos appear in device Photos app when “Save to Camera Roll” is enabled — iOS (handles iOS 14+ “Add Photos Only”) | Code now recognizes and requests PHOTO_LIBRARY_ADD_ONLY and treats it as sufficient; native module checks authorization before saving; processing path preserves files when save fails so successful saves are truly saved. |
| ❌ | Synced photos appear in device Photos app when “Save to Camera Roll” is enabled — Android | The PR explicitly leaves Android MediaStore path unchanged and provides no Android-specific fixes or test evidence for the Samsung Fold scenario reported in the incident. |
| ✅ | Failed saves show clear error (not left as misleading in-app-only success) | When saveToLibrary fails the code now surfaces a failure to the store, throws so the process path is the failure path (skips metadata save and deletion), and shows an alert directing users to fix permissions when failedFiles is non-empty. |
| ✅ | Red “!” state maps to an actionable failure reason in logs | The processing path now records file failures with an explicit reason ('camera roll save failed') and avoids marking the file processed/deleting it, so failedFiles will contain a clear, logged reason usable by UI/diagnostics. |
| ✅ | No regression on iOS gallery sync | Changes add explicit authorization checks and least-privilege permission flows (add-only), and add defensive failure handling; diffs show iOS-specific handling and a testing plan for iOS. |
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| // If photos were supposed to be auto-saved to camera roll but some failed, check | ||
| // whether permissions are the cause and direct the user to fix them. | ||
| const syncStore = useGallerySyncStore.getState() | ||
| if (syncStore.failedFiles.length > 0) { |
Contributor
There was a problem hiding this comment.
P2: Camera-roll permission alert is triggered from generic failedFiles, so unrelated sync failures can be mislabeled as permission problems.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At mobile/src/services/asg/gallerySyncService.ts, line 1977:
<comment>Camera-roll permission alert is triggered from generic `failedFiles`, so unrelated sync failures can be mislabeled as permission problems.</comment>
<file context>
@@ -1971,6 +1971,34 @@ class GallerySyncService {
+ // If photos were supposed to be auto-saved to camera roll but some failed, check
+ // whether permissions are the cause and direct the user to fix them.
+ const syncStore = useGallerySyncStore.getState()
+ if (syncStore.failedFiles.length > 0) {
+ try {
+ const autoSaveEnabled = await gallerySettingsService.getAutoSaveToCameraRoll()
</file context>
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
2 participants
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.
What
Fixes OS-1472: gallery photos sync successfully into the Mentra app but are never written to the phone camera roll.
Three bugs combined to produce this symptom:
Bug 1 — iOS
addOnlypermission not recognized (MediaLibraryPermissions.ts)checkPermission()only checkedPERMISSIONS.IOS.PHOTO_LIBRARY(full read-write). On iOS 14+, users who tap "Add Photos Only" when prompted holdPHAuthorizationStatusAddOnly.react-native-permissionsreturnsDENIED/BLOCKEDforPHOTO_LIBRARYin this case, so the permission gate fails and the camera roll write is silently skipped for a large portion of iOS users.Fix:
checkPermission()now also checksPERMISSIONS.IOS.PHOTO_LIBRARY_ADD_ONLY; if that isGRANTED, we treat it as sufficient (it is —PHPhotoLibrary.performChangesworks withaddOnly).requestPermission()now requestsPHOTO_LIBRARY_ADD_ONLYfirst (least-privilege per Apple guidelines), then falls back to fullPHOTO_LIBRARY.Bug 2 — Silent failure:
onFileFailed+onFileProcessedboth fire; file deleted from glasses (mediaProcessingQueue.ts)When
saveToLibraryreturnedfalse,processItemcalledstore.onFileFailedbut did not throw.processLoopsaw no exception, calledstore.onFileProcessed, and continued to steps 7–9: metadata was saved, the photo appeared in the in-app gallery, and the file was deleted from glasses. The user lost their only remaining copy while believing the sync succeeded.Fix: After
onFileFailed, now throwsnew Error("camera roll save failed").processLooptakes the failure path — steps 7 (metadata save), 8 (queue update), and 9 (glasses deletion) are all skipped. The file stays on the glasses until the user fixes permissions and re-syncs.Bug 3 — No user-facing feedback (
gallerySyncService.ts+en.ts)When permissions were denied the user saw no indication. They would check their camera roll, find nothing, and have no way to diagnose the cause.
Fix: In
onSyncComplete, iffailedFilesis non-empty and the camera roll permission is currently denied, ashowAlertis shown directing the user to Settings → Privacy & Security → Photos → Mentra.Bug 4 — Defense-in-depth: no authorization check in native module (
CrustModule.swift)saveToGalleryWithDatejumped directly toPHPhotoLibrary.performChangeswith no authorization pre-check. If called without authorization, the only error path was the PHKit completion-handler error which could be opaque.Fix: Added an explicit
PHPhotoLibrary.authorizationStatus(for: .addOnly)check beforeperformChanges. Returns a clear, logged error immediately if unauthorized.Files Changed
mobile/src/utils/permissions/MediaLibraryPermissions.tsmobile/src/services/asg/mediaProcessingQueue.tsmobile/src/services/asg/gallerySyncService.tsmobile/modules/crust/ios/CrustModule.swiftmobile/src/i18n/en.ts(addedglasses:cameraRollPermissionTitleandglasses:cameraRollPermissionMessage)Testing