feat(accounts): add platform credential slots - #436
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: b5e6014e2e
ℹ️ 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: d0bb0e52ff
ℹ️ 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: 6dabe16e75
ℹ️ 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: 5849f1719f
ℹ️ 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".
d6d0017 to
4daca37
Compare
4daca37 to
f7931ce
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f7931ce047
ℹ️ 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: 0b1e4cb658
ℹ️ 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".
0b1e4cb to
c0526df
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c0526df17f
ℹ️ 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".
c0526df to
bbc70fc
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bbc70fc564
ℹ️ 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".
28965a7 to
1d310d0
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31c7c7e245
ℹ️ 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".
| check(snapshot.malformedEntryCount == 0) { | ||
| "Reset the malformed account-removal cleanup state before signing in again." |
There was a problem hiding this comment.
When the journal contains even one malformed entry, startup session restoration returns null and every subsequent account selection or sign-in reaches this unconditional failure. Neither clearSession() nor the recovery worker removes or quarantines malformed entries, so the prescribed reset cannot be performed in the app and the installation remains locked out until the user wipes all app data; provide an explicit safe purge/recovery flow for this state.
AGENTS.md reference: AGENTS.md:L447-L449
Useful? React with 👍 / 👎.
| val reference = requireReference(documentId, session) | ||
| if (reference.isRoot) throw SecurityException("The Nextcloud root cannot be renamed.") | ||
| val account = resolveAccount(session) | ||
| val file = findDocument(session, account, reference.path) |
There was a problem hiding this comment.
For every DocumentsProvider rename or delete, and for moves and creates below the root, withAndroidDocumentMutation already holds this account's non-reentrant operation mutex when findDocument() calls services.listFiles(). The newly guarded listFilesWithSource() then tries to acquire the same mutex through withRetainedAndroidAccountFileRead, so the provider call waits on its own lease before issuing DAV and never completes; perform the metadata lookup before taking the mutation lease or propagate that the lease is already held.
Useful? React with 👍 / 👎.
| val reference = requireReference(documentId, session) | ||
| if (reference.isRoot) throw SecurityException("The Nextcloud root cannot be renamed.") | ||
| val account = resolveAccount(session) | ||
| val file = findDocument(session, account, reference.path) |
There was a problem hiding this comment.
For every DocumentsProvider rename, delete, or move, and for creation below a non-root folder, withAndroidDocumentMutation already holds this account's non-reentrant operation lease when findDocument() calls services.listFiles(). The newly guarded listFilesWithSource() then waits for the same lease through withRetainedAndroidAccountFileRead, so the provider operation deadlocks before issuing its DAV mutation; perform the metadata lookup before acquiring the mutation lease or propagate that the lease is already held.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eadef3dc98
ℹ️ 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".
| synchronized(ANDROID_HOME_WORKSPACE_STORAGE_LOCK) { | ||
| val editor = preferences.edit() | ||
| keys.forEach(editor::remove) | ||
| check(editor.commit()) { "The home workspace account settings could not be removed." } |
There was a problem hiding this comment.
When active-account removal overlaps installed-app discovery, the still-composed shell can call appPinsRepository.save after this one-shot deletion; the storage lock only orders the operations and has no retired-account generation check, so a late save recreates apps:pins and re-adding the account restores metadata that removal reported deleted. Fresh evidence beyond the earlier missing-purge report is that the added deletion still accepts writes from pre-removal UI incarnations; fence both platform storage implementations against those stale writes.
AGENTS.md reference: AGENTS.md:L327-L329
Useful? React with 👍 / 👎.
| fun stateFor(session: NextcloudSession): PhotoTimelineUiState { | ||
| val accountKey = previewCacheDigest(session) | ||
| accountStates.remove(accountKey)?.let { existing -> | ||
| accountStates[accountKey] = existing |
There was a problem hiding this comment.
When account removal overlaps composition of the old account shell, stateFor neither consults sharedAccountPrivateMemoryGate nor synchronizes with removeAccount, so a call after the purge can recreate the stable account key and retain the removed account's timeline and backup-status state for a later same-account sign-in. Fresh evidence beyond the earlier memory-cache findings is that this newly extracted repository remains outside the incarnation fence used by the adjacent Calendar, Activity, and Talk caches.
AGENTS.md reference: AGENTS.md:L327-L329
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a64f83b249
ℹ️ 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".
| listOf( | ||
| { fenceAndroidDynamicApiStateForRemoval(cacheIdentity, dynamicApiState.coalescer, dynamicApiState.cache) }, | ||
| { dynamicDiscoveryCache.retireAccount(session.accountId.storageKey, cacheIdentity) }, | ||
| { removeSupportAccount(accountIdentity) }, |
There was a problem hiding this comment.
When an Android account has recorded support events, this callback removes support-intake submissions only; the shared AndroidSupportDiagnostics instance still retains the account's events in memory and in events-v1.jsonl. Because JvmSupportDiagnostics.visibleEvents() derives a stable scope from the account identity, signing into the same server/login again makes the removed account's old operations and timestamps visible and exportable. Add account-targeted diagnostic-history deletion with stale-publication fencing before clearing the removal journal; the ROADMAP account-removal gate includes metadata.
AGENTS.md reference: AGENTS.md:L40-L41
Useful? React with 👍 / 👎.
| providerPreferenceAccountId = currentAccountId | ||
| val key = virtualFileProviderPreferenceKey(currentAccountId) | ||
| providerWasEnabledBeforeRemoval = preferences.getBoolean(key, false) | ||
| setDesktopVirtualFileProviderPreference(preferences, currentAccountId, enabled = false) |
There was a problem hiding this comment.
If the desktop process exits after this preference flush but before credential removal commits, the account remains in the registry while providerWasEnabledBeforeRemoval is lost with the process. This can occur before the cleanup journal is even prepared; on restart restoreVirtualFileProviderIfEnabled() sees the persisted false value, so a previously enabled Linux or Windows virtual-file provider never returns. Persist the prior enablement as recovery state, or defer this durable preference change until the credential-removal commit.
AGENTS.md reference: AGENTS.md:L327-L329
Useful? React with 👍 / 👎.
1 participant
Outcome
Advances #172. This PR depends on #429.
Latest review revision
Commit
eadef3dc989977bcb507865ed2bfb6743f9e9136fixes DocumentsProvider metadata lookups reacquiring the non-reentrant account lease already held by rename, delete, move, and nested-create operations. Ordinary file reads still acquire and validate their own lease. Focused regressions cover held-lease lookups, stale ordinary reads, cache/error/cancellation behavior, and parent-directory validation.One correctness finding remains open: malformed account-removal cleanup journals can block all account activation without a usable in-app recovery flow. An entry may contain no recoverable ownership identity. Ignoring it could revive pending-delete private state, while a blanket purge could erase offline files or drafts. No automatic journal discard or private-data purge has been added; recovery policy and explicit user consent need a maintainer decision before merge.
Validation
31c7c7e2458c3aaf0f78568e122cb951742d7642.Compatibility and evidence limits
Android and desktop JVM targets are affected. No device, emulator, live Nextcloud server, macOS keychain, Windows Credential Manager, or Linux Secret Service end-to-end verification is claimed for this revision. Corrupt, mismatched, and unsupported credential state remains fail-closed rather than being overwritten.
No visual changes in the provider-deadlock revision.