fix(android): preserve queued upload scheduling - #439
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: 454d73b427
ℹ️ 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".
e2a8728 to
d4e541c
Compare
4482b93 to
c2f0cc7
Compare
c2f0cc7 to
84677b8
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 84677b8a69
ℹ️ 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".
84677b8 to
abcbb66
Compare
abcbb66 to
7b2e575
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b2e575cd6
ℹ️ 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".
7b2e575 to
a86e5ec
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a86e5ec1fa
ℹ️ 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".
a86e5ec to
3ed84f8
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 81e33dc68b
ℹ️ 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 (cancelled: CancellationException) { | ||
| throw cancelled | ||
| } catch (_: Exception) { | ||
| return@synchronized retainCapabilityCleanup(file.selectionId) |
There was a problem hiding this comment.
When persisted capability preferences contain more than MAX_RECOVERABLE_CAPABILITIES and any terminal upload still has cleanup pending, the bounded recovery scan marks the oversized store as quarantined, but that job's subsequent picker.release() calls the unbounded loadCapabilitySnapshot(), whose size check throws into this catch. Returning retainCapabilityCleanup makes reconcileQueuedUploads() fail, so the startup monitor reloads the same oversized store every 60 seconds indefinitely instead of honoring the quarantine; classify this overflow as quarantined in the terminal release path so the job marker can be completed without polling.
AGENTS.md reference: AGENTS.md:L325-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: 261a84cd0d
ℹ️ 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".
| !durableUploadCapabilityPermissionOwnedByAnother( | ||
| capabilities = existing, |
There was a problem hiding this comment.
When a malformed capability retains a valid URI and an app-owned grant, selecting that same document again is allowed because this duplicate check examines only snapshot.capabilities, not snapshot.malformedCapabilities. The new record then observes the existing grant as pre-existing; cleanup of the malformed record retains it because of the new peer, while cleanup of the new record retains it because of the malformed peer, leaving terminal cleanup pending and statuses undismissable indefinitely. Reject only malformed peers with the same cleanup URI, or resolve them before acquiring the new capability.
AGENTS.md reference: AGENTS.md:L457-L459
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bcae385d19
ℹ️ 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 (immediatePending) { | ||
| immediatePending = false | ||
| recover() |
There was a problem hiding this comment.
When a worker has just failed and its 60-second follow-up is pending, any immediate signal reaches this branch and invokes the global reconciler, which re-enqueues the same now-unowned queued job along with unrelated work. Fresh evidence beyond the earlier backoff comments is that opening or refreshing a card calls durableMultipartUploadStatuses, whose unconditional queued-status signal interrupts this delay; another enqueue-recovery signal has the same effect. Persistent worker failures can therefore churn WorkManager and storage at the UI refresh cadence, so immediate recovery should exclude jobs whose failure deadline has not elapsed.
AGENTS.md reference: AGENTS.md:L25-L26
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1caca52102
ℹ️ 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".
| ) | ||
|
|
||
| private fun storedCapabilitySelectionIds(maximumRows: Int? = null): List<String> { | ||
| val selectionIds = preferences.all.keys.asSequence() |
There was a problem hiding this comment.
When a legacy or corrupted capability preference file contains many or very large upload_ entries, preferences.all materializes the complete key/value map before the sequence applies take(limit + 1), so startup recovery can still exhaust memory or stall before detecting and quarantining the oversized store. Fresh evidence beyond the earlier bounded-recovery comment is that the final scanner limits its returned IDs but this newly introduced storage read remains unbounded; enumerate capabilities through bounded storage or enforce a bound before loading all preference values.
AGENTS.md reference: AGENTS.md:L325-L326
Useful? React with 👍 / 👎.
1 participant
Outcome
Failure path
The upload job was persisted before WorkManager scheduling, but any exception from awaiting the scheduling operation deleted that journal and released its source. WorkManager can already have accepted or started the work before its completion signal fails, leaving a server-visible upload without a durable outcome record and allowing the same attachment to be submitted again.
Validation
On the dedicated Linux build host, from exact parent
8d1989297bb99efd8e5bb40bc7e1761c98299ad2::androidApp:testDebugUnitTest --tests dev.obiente.nextcloudnative.AndroidDurableMultipartUploadPolicyTest:androidApp:assembleDebugbash tools/check-kotlin-architecture.shbash tools/check-repository.shThe deterministic tests cover scheduler acceptance followed by a lost completion signal, restart-visible queued state, cancellation after persistence, and persistence failure before scheduling.
Evidence boundaries
This validates the transaction policy and Android debug build on Linux. It does not include an emulator lifecycle run, a live Nextcloud attachment upload, or an injected failure inside Android's real WorkManager database.
This PR is intentionally based on #438 because that ready account-isolation change owns the adjacent queued-account credential lookup.
Advances #52