Stop releasing the persisted grant the recent list depends on - #523
Merged
Conversation
loadUri took a persistable uri permission and released it four lines later, before the load it had queued onto LoaderService had even run. Every uri written to the recently opened list was therefore unreadable on the next launch, so the one place that offered them - the last row of the file manager chooser - handed back documents that would not open. Grants are now reclaimed by reconciling the ones we hold against the recent list, rather than released on close. That is idempotent, needs no per-removal bookkeeping and mops up the grants earlier versions leaked. The de-duplication in addRecentDocument was a no-op as well: it read the json, called removeRecentDocument - which read the file again, removed and saved - and then wrote back the stale pre-removal array. Duplicates accumulated without bound. The ordering, de-duplication and capping rules move into RecentDocumentList, which is free of android imports so plain JVM tests can cover them, following MimeTypeResolver. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TiSZ4FeptMUsFjAuCewzQZ
andiwand
marked this pull request as ready for review
July 27, 2026 20:33
![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: 6f4b97859e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
andiwand
added a commit
that referenced
this pull request
Jul 27, 2026
#523 stopped releasing the persisted grant on close and made prune() reconcile what we hold against what the stored lists refer to. Between taking a grant and the list referring to it there is a gap, though: loadUri() only queues the load, so the entry is written much later, once MetadataLoader has read the document. MainActivity starts prune() on its own thread from onCreate() and takes the grant from onStart(), so a document opened from another app races them. Losing that race releases the grant of the document currently being loaded: the load itself still succeeds on the transient grant that came with the intent, the entry lands in the recent list, and it is unreadable on the next launch - the failure the reconciliation was written to fix, only through a narrower window. Grants taken this process are therefore held until one of the stored lists names them. The set is empty on a fresh process, so nothing leaks past a restart, and prune() reads the permissions before the list and the list before the pending set, so a grant taken while it runs cannot fall through both. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
First of six, stacked. Base:
main.loadUritook a persistable uri permission and released it four lines later — before the load it had just queued ontoLoaderServicehad run. Every uri written to the recently opened list was therefore unreadable on the next launch, so the one place that surfaced them handed back documents that would not open.Grants are now reclaimed by reconciling what we hold against the recent list, instead of being released on close. Idempotent, no per-removal bookkeeping, and it mops up grants earlier versions leaked.
addRecentDocument's de-duplication was also a no-op: it read the json, calledremoveRecentDocument(which re-read, removed and saved), then wrote back the stale pre-removal array. Duplicates accumulated without bound.Ordering, de-duplication and capping move into
RecentDocumentList, free of android imports so plain JVM tests can cover them — theMimeTypeResolverpattern.Verification
testProDebugUnitTest— 10 new cases over the logic the bug lived inconnectedProDebugAndroidTestgreenThe end-to-end fix is not assertable in the current harness — test fixtures come from the app's own
FileProvider, which cannot grant persistable permissions.