fix(android): bind retained documents to accounts - #447
Conversation
Obiente previewNC Native · Obiente updates this comment as the preview changes. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: f97f74994f
ℹ️ 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".
a80069c to
722eb39
Compare
00333f6 to
8266aab
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8266aabbc2
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6b7830d96
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 888fbf8e00
ℹ️ 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".
dfdae5f to
871e131
Compare
888fbf8 to
74b43bf
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74b43bf122
ℹ️ 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".
a10ff29 to
5bd8032
Compare
74b43bf to
b1fc365
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1fc365e26
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 904fa387ba
ℹ️ 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".
| accountLease: AndroidAccountOperationLease, | ||
| handler: Handler, | ||
| ): ParcelFileDescriptor = try { | ||
| ParcelFileDescriptor.open(content, ParcelFileDescriptor.MODE_READ_ONLY, handler) { accountLease.close() } |
There was a problem hiding this comment.
When a SAF client keeps this cached/offline descriptor open and the provider process is killed before the client closes it, the client retains its duplicated raw file descriptor but accountLease and the lifetime guard disappear with the provider process. After restart, account removal observes no reader and can complete and unlink the cache, while the client continues reading the already-open inode. Use a revocable proxy rather than handing out raw cache descriptors, or otherwise make the removal fence process-restorable.
AGENTS.md reference: AGENTS.md:L457-L461
Useful? React with 👍 / 👎.
| rollbackUnavailableAndroidAccountRemoval( | ||
| active = target.wasActive, recovered = recovered, persistRecovered = { state -> persistState(state) }, | ||
| clearCleanup = { accountRemovalCleanupJournal.clear(accountId.storageKey) }, | ||
| clearCleanup = { | ||
| rollbackAndroidAccountRemoval(appContext, requireNotNull(documentRetirement)) | ||
| accountRemovalCleanupJournal.clear(accountId.storageKey) |
There was a problem hiding this comment.
When the active account's credential slot is unavailable and clearSession(recovered, pendingCleanup, unavailableSession) successfully commits the registry removal but its subsequent roots notification throws, this rollback path does not restore the registry because rollbackUnavailableAndroidAccountRemoval(active = true) skips persistRecovered; it nevertheless restores the old document incarnation and clears the cleanup marker. Re-adding the now-absent account can therefore reuse that active incarnation and reactivate old URI grants. The fresh path here is the post-commit notifyDocumentRootsChanged() failure in clearSession, rather than concurrent reconciliation; make that notification best-effort or retain the tombstone once ownership removal committed.
AGENTS.md reference: AGENTS.md:L397-L398
Useful? React with 👍 / 👎.
904fa38 to
8f53cf8
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f53cf8e18
ℹ️ 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".
| return withAndroidDocumentMutation( | ||
| session = resolved.session, | ||
| loadCurrentSession = { services.loadSession(resolved.session.accountId) }, |
There was a problem hiding this comment.
When a document mutation targets an inactive account while saveSession() reauthenticates that same account using a canonically equivalent server spelling, this path acquires the operation guard under the old raw NextcloudDocumentIds.accountKey. The credential transition guards only the previously active account and the replacement session's new raw key (AndroidAccountCredentialController.kt:434-438), so it can persist the replacement concurrently while create, rename, move, or delete continues with the captured old credentials. Key the mutation fence by accountId.storageKey, or also fence the stored session's prior key, so credential replacement cannot leave an in-flight write operating on stale identity state.
AGENTS.md reference: AGENTS.md:L373-L376
Useful? React with 👍 / 👎.
99f6848 to
7f70d10
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f70d10a98
ℹ️ 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".
| } catch (_: IllegalArgumentException) { | ||
| quarantineMalformedPendingRetirement(accountIdentity) | ||
| null | ||
| } catch (_: ClassCastException) { |
There was a problem hiding this comment.
When a retirement journal contains canonical Base64 that decodes to invalid UTF-8 (for example, _w in a present field), decodeRetirementField() throws CharacterCodingException, but this recovery catches only IllegalArgumentException and ClassCastException. The exception therefore escapes prepareForCredentialReset() before clearCredentials() runs, leaving the explicit credential reset unusable. The fresh evidence beyond the earlier malformed-journal report is this new quarantine handler's exception whitelist, which still excludes the UTF-8 decoder failure; catch that failure or normalize it into the handled malformed-input type.
AGENTS.md reference: AGENTS.md:L325-L326
Useful? React with 👍 / 👎.
1 participant
Summary
Validation