fix(accounts): retire private memory on removal - #456
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: 0cb85a5c16
ℹ️ 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".
9923bf6 to
8590a8d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8590a8dd08
ℹ️ 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".
e1d3c1e to
0db18d5
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0db18d5065
ℹ️ 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".
0db18d5 to
d21e351
Compare
d21e351 to
d42e69b
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d42e69b65c
ℹ️ 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".
| accounts.remove(scope) | ||
| accounts[scope] = state |
There was a problem hiding this comment.
When a user opens the timeline across many retained accounts, every parsed day index remains in accounts until that account is explicitly removed; the registry permits 64 accounts and each index permits 20,000 days. The previous service-local cache retained only one index, while the new shared cache inserts entries without the four-account LRU used by the other process caches, so normal account switching can retain a large amount of private timeline metadata for the life of the process. Evict older account scopes when publishing a new index.
Useful? React with 👍 / 👎.
d42e69b to
bc3ef02
Compare
…-memory-account-retirement
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf27a2431b
ℹ️ 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".
| accountStates.remove(accountStorageKey)?.let { retired -> | ||
| retired.timeline.value = PhotoTimelineState(pageSize = MAX_PHOTO_TIMELINE_PAGE_SIZE) | ||
| retired.backupStatuses.value = emptyMap() | ||
| retired.initialLoadCompleted.value = false |
There was a problem hiding this comment.
If an in-flight timeline load completes after account retirement but before the old AuthenticatedApp composition is disposed, this purge only clears the retained PhotoTimelineUiState; it does not revoke writes to that object. Fresh evidence in this revision is that loadTimelinePage still assigns the private page and backup statuses directly at NextcloudNativeApp.kt:8922-8948 without an incarnation producer, so a late completion can repopulate the cleared holder and briefly render or retain the removed account's media metadata. Make these state publications gate-aware rather than relying only on removing and resetting the registry entry.
AGENTS.md reference: AGENTS.md:L327-L328
Useful? React with 👍 / 👎.
| "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/AccountPrivateMemoryLifecycle.kt": "36c9267a53d9f6f59fc38862d8a073fca6b6c3c1275730ddc65d138d5e09ce82", | ||
| "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/AccountSettingsScreen.kt": "7b93d571553fa8c364681f172edecde3109b45db9a4ff6f9f6fb12f8f6280a0e", | ||
| "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/AccountWorkspaceMemoryCaches.kt": "11e6f0eab522b4fc799a67bf6e3881f96a62f458a9fff07648bacb5e3eaba9d5", | ||
| "ui/src/commonMain/kotlin/dev/obiente/nextcloudnative/app/AccountPrivateMemoryCleanup.kt": "35c13216a14662f9bc2b47cee36b29f9d7d8602373b937536fcc5cc01da67e4f", |
There was a problem hiding this comment.
This patch updates the manifest's source hashes even though no screenshot asset was regenerated, and the commit's own verification states that a real capture refresh remains pending. Because verify:captures:fresh trusts these declared hashes, the manifest now presents the existing images as current and prevents the refresh workflow from distinguishing a manually blessed stale catalog from one rendered against this revision; run the capture generator and commit its actual output instead.
AGENTS.md reference: AGENTS.md:L99-L101
Useful? React with 👍 / 👎.
1 participant
Outcome
Clear account-private previews, Notes cache entries, support drafts, and related in-memory holders before a removed account can be replaced. Reactivate account holders only after confirming the current account incarnation.
The shared Memories timeline cache now retains at most four account scopes. Cache hits, active reads, and activation update recency. Publishing a fifth scope evicts the least recently used index without changing pending-load identities or account-retirement fences.
Verification
git diff --checkpassed.Compatibility and risk
Android and desktop in-memory account state are affected. Persisted server data and DAV behavior are unchanged. This PR is stacked on #455 and includes
mainat92dd2ae9cec1dc7dac16907e66d7b870694ebbbf.No intended visual changes. The shared production input change still requires a real capture refresh.