fix(accounts): retire dynamic memory on removal - #455
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. |
veryCrunchy
changed the title
![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: 39af1ccc0e
ℹ️ 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".
bd62702 to
83f3347
Compare
df787cc to
e946520
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e946520e23
ℹ️ 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".
671af04 to
944dea1
Compare
944dea1 to
9e6259b
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e6259b488
ℹ️ 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".
| if (malformedEntryFound) recordMalformedOnce() | ||
| return when { | ||
| malformedEntryFound -> DesktopAccountActivationBlock.UnknownJournalData | ||
| matchingEntryFound -> DesktopAccountActivationBlock.PendingCleanup |
There was a problem hiding this comment.
When a restart follows a crash after a Prepared journal entry was persisted but before the credential was removed, this now classifies the existing account as blocked, so loadDesktopSessionAfterCleanupGate() throws before loading it. In the inspected desktop startup flow, Main.kt starts the cleanup lifecycle only after two session-dependent startup calls, while NextcloudSessionLoading.kt does not catch this generic IllegalStateException; the concurrent session-loading effect can therefore fail permanently or terminate the application instead of waiting for the background recovery that would detect Present ownership and clear the journal. Recover supported pending entries before session loading, or expose a handled retryable load state.
AGENTS.md reference: AGENTS.md:L447-L449
Useful? React with 👍 / 👎.
| if (previewCacheIdentity == null) { | ||
| DynamicNativeMemoryAccountLifecycle.retireAccount(session.accountId.storageKey) |
There was a problem hiding this comment.
When retrying a two-field legacy removal journal, previewCacheIdentity is null even though this overload has the exact session; this branch therefore retires only process memory and skips both the request-coalescer fence and dynamicApiState.cache.invalidateAccount(). saveSession() can then clear the cleanup journal and reactivate the account while its persisted dynamic responses under NextcloudDocumentIds.cacheAccountId(session) remain available, allowing private data from the prior account incarnation to be shown after the same server/login identity is added again. Derive the full cache identity from session and run the normal fenced cleanup for this legacy case.
AGENTS.md reference: AGENTS.md:L410-L413
Useful? React with 👍 / 👎.
…dynamic-memory-account-retirement
1 participant
Outcome
Clear account-scoped dynamic app memory and pending dynamic mutation state during account removal. Dynamic activation reopens caches only after the exact saved session is still current, so late responses from a removed account cannot repopulate a replacement account.
Pending cleanup blocks account activation until it is safe. Known legacy cleanup entries now block only their recorded account identity, while unknown or malformed entries continue to fail closed. Canonical identity matching is preserved for newer journal formats.
Verification
On the dedicated Linux build host, exact head
944dea1a9c4d6c71c9488fec1517569f4aafcafd, source tree233f61df950656510ec7dbc19574826e028ab1a5:DesktopAccountMemoryRetirementTest: 7 tests passed.DesktopAccountOperationGuardTest: 41 tests passed.bash tools/check-repository.shandgit diff --checkpassed.The latest regression coverage exercises all four legacy cleanup encodings, verifies that the owning account stays blocked, allows unrelated accounts to load and publish, and retains the pending journal.
Hosted Android, Windows, and remaining integration checks are separate merge gates. This focused Linux run does not establish live multi-account behavior on a device.
Restacked head
9e6259b488312612eb6c4198f407d81feaebec17preserves all 11 layer commits and inherits the newer provider-deadlock and terminal-cleanup fixes from #439. Full repository checks andgit diff --checkpass on the integrated tree. The focused desktop result above covers the unchanged #455 behavior, not the newly inherited Android fixes. Latest-head hosted checks remain required.Compatibility and risk
Android and desktop account removal and activation are affected. There is no server schema, DAV, or wire-protocol change. This PR is stacked on #439 and includes
mainat92dd2ae9cec1dc7dac16907e66d7b870694ebbbf.No visual changes.