fix: stage Android receives to SAF folder and guard missing download dir - #4
Conversation
The receiver returned HTTP 500 on Android because scoped storage blocks raw-path writes to SAF-picked folders. Go now stages incoming files into an app-internal dir; on completion the frontend bridges the file into the chosen folder via the SAF tree URI through ContentResolver, then deletes the staging copy. The picker intent now requests WRITE up front so the persistable grant succeeds, and a banner plus auto-reject guard prevents receiving without a configured folder. Also fixes PairModal keyboard overlap and Receive card overflow on mobile.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughThe change adds Android SAF folder persistence and file copying. Mobile transfers use internal staging before copying to the selected folder. The frontend validates folder configuration and reports copy errors. The pair modal now adapts to visual viewport size. ChangesAndroid download folder
Responsive pair modal
Sequence Diagram(s)sequenceDiagram
participant ReceiveView
participant useTransfers
participant WailsJSBridge
participant MainActivity
participant SAFFolder
ReceiveView->>useTransfers: Start Android receive
useTransfers->>WailsJSBridge: Request copyToFolder
WailsJSBridge->>MainActivity: Dispatch copy request
MainActivity->>SAFFolder: Create document and stream staged file
SAFFolder-->>MainActivity: Return copy result
MainActivity-->>useTransfers: Emit android:copyDone
✨ 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 |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| BestPractice | 4 high |
| ErrorProne | 1 medium |
🟢 Metrics 288 complexity · 4 duplication
Metric Results Complexity 288 Duplication 4
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
PR Summary by QodoFix Android SAF receive path with staging dir and missing-folder guard
AI Description
Diagram
High-Level Assessment
Files changed (11)
|
Code Review by Qodo
1. Unchecked docUri strands files
|
| Uri docUri = DocumentsContract.createDocument(getContentResolver(), | ||
| treeUri, mime != null ? mime : "application/octet-stream", fileName); | ||
|
|
||
| try (InputStream in = new FileInputStream(source); |
There was a problem hiding this comment.
1. Unchecked docuri strands files 🐞 Bug ☼ Reliability
MainActivity.copyToFolder() uses the Uri returned by DocumentsContract.createDocument() without validating it and emits low-signal errors on failure, leaving the staged file behind. This can accumulate app-internal “downloads” files when SAF document creation/opening fails (permission/provider/storage issues).
Agent Prompt
### Issue description
`MainActivity.copyToFolder()` doesn’t validate the document Uri returned by `DocumentsContract.createDocument(...)` and doesn’t attempt cleanup when the SAF copy fails. This can strand staged files in app-internal storage (and potentially leave partially created destination documents), while emitting an unhelpful error (e.g., NPE message `null`).
### Issue Context
The Go receiver stages to an internal directory on mobile, and the frontend requests Android to bridge the file into the SAF folder via `copyToFolder`.
### Fix Focus Areas
- build/android/app/src/main/java/com/wails/app/MainActivity.java[430-473]
- Check `docUri == null` and emit a clear error without calling `openOutputStream`.
- If `openOutputStream` fails/returns null or streaming throws, consider deleting the newly created document (best-effort) and/or keep the staging file for retry.
- Emit errors via a safe JSON builder (`JSONObject`) so error messages are always non-null and properly escaped.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| if (data?.uri) { | ||
| local.value.downloadDirUri = data.uri | ||
| } |
There was a problem hiding this comment.
2. Reset keeps stale folder uri 🐞 Bug ≡ Correctness
SettingsView.reset() only resets downloadDir, but not the new downloadDirUri, so the displayed folder can diverge from the SAF destination actually used for Android saves. If the user saves after Reset, incoming files may be copied into a different folder than the UI indicates.
Agent Prompt
### Issue description
`downloadDirUri` is now the authoritative Android destination (used for SAF copies), but `reset()` only restores `downloadDir`. This allows persisting inconsistent settings (path points to A, URI points to B) and saving into an unexpected folder.
### Issue Context
Folder picking sets both `downloadDir` (display path) and `downloadDirUri` (tree URI). Receiving on Android is gated by `downloadDirUri`, and the copy operation uses `destinationUri`.
### Fix Focus Areas
- frontend/src/views/SettingsView.vue[26-52]
- Update `reset()` to also restore `local.value.downloadDirUri = settings.value.downloadDirUri` (or clear both fields if “reset” means unset).
- (Optional) On Android, consider making the `downloadDir` input read-only or clearing `downloadDirUri` when the user edits the path manually to avoid mismatch.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| getContentResolver().takePersistableUriPermission(treeUri, | ||
| Intent.FLAG_GRANT_READ_URI_PERMISSION); | ||
| Intent.FLAG_GRANT_READ_URI_PERMISSION | ||
| | Intent.FLAG_GRANT_WRITE_URI_PERMISSION); |
There was a problem hiding this comment.
3. Persistable grant ignores result flags 🐞 Bug ☼ Reliability
handleFolderPickerResult() calls takePersistableUriPermission() with a hard-coded READ|WRITE mask instead of using the flags actually granted in the result Intent. If a provider grants fewer permissions than requested, folder picking fails (exception path) and Android receiving can’t be configured.
Agent Prompt
### Issue description
`takePersistableUriPermission(treeUri, READ|WRITE)` should only request persistable permissions that were actually granted in the returning `Intent` flags. Hard-coding READ|WRITE risks a `SecurityException` for providers that don’t return WRITE (even if requested).
### Issue Context
The picker is launched with READ|WRITE|PERSISTABLE flags, and the code then immediately attempts to persist both READ and WRITE.
### Fix Focus Areas
- build/android/app/src/main/java/com/wails/app/MainActivity.java[389-421]
- Compute `int takeFlags = data.getFlags() & (Intent.FLAG_GRANT_READ_URI_PERMISSION | Intent.FLAG_GRANT_WRITE_URI_PERMISSION)` and pass `takeFlags`.
- If WRITE isn’t present in `takeFlags`, emit a clear error stating that write access wasn’t granted and copying into that folder won’t work.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
The receiver returned HTTP 500 on Android because scoped storage blocks raw-path writes to SAF-picked folders. Go now stages incoming files into an app-internal dir; on completion the frontend bridges the file into the chosen folder via the SAF tree URI through ContentResolver, then deletes the staging copy. The picker intent now requests WRITE up front so the persistable grant succeeds, and a banner plus auto-reject guard prevents receiving without a configured folder. Also fixes PairModal keyboard overlap and Receive card overflow on mobile.
Summary by CodeRabbit