Sandbox::https://github.com/nextcloud/desktop/pull/11001

claucambra · 2026-10-05T09:51:31Z

Resolves

Second fix for #10579

Summary

Ensures cancellation propagates through File Provider uploads, downloads, and thumbnail fetches. Previously, cancellation did not consistently reach the surrounding operation or its supporting network requests, allowing cancelled operations to continue, update database state, and potentially report success

Checklist

Sign-off message is added to all commits.
The commit messages are following The Conventional Commits specification.
The commit history is clean with no merge commits.
- How to rebase your commits
- How to squash commits with rebase
Uploaded screenshots from before and after for UI changes.
Test(s) added to branch, with before/after results included in PR description, for performance improvements where applicable.
Documentation has been updated or is not required.
Backports requested where applicable (critical bugfixes).
Labels added where applicable (bug/enhancement).
Milestone added for target version.

AI (if applicable)

The content of this PR was partly or fully generated using AI
- I have read the guidelines for AI-assisted contributions.
- I have read Nextcloud's AI Contribution Policy.

i2h3 · 2026-10-06T09:02:54Z

@claucambra Please rebase onto master and resolve the conflicts.

claucambra · 2026-10-06T09:12:07Z

@claucambra Please rebase onto master and resolve the conflicts.

Rebased and resolved now

i2h3

Reviewed at 8336b0f. Main points:

A cancelled fetch is persisted as .downloadError, which blocks later depth-1 metadata updates for that item (Item+Fetch.swift).
A chunked upload cancelled during assembly reports cancellation although the server committed the file (Upload.swift).
Upload cancellation has no tests; the new upload mock hooks are unused.

The rest are edge cases and test-robustness notes. The NetworkOperationCancellation helper and the thumbnail batching look correct to me (Darwin invokes cancellationHandler asynchronously, so no lock re-entrancy; registrations are sequential).

Hygiene: only 2 of 22 commits carry an Assisted-by: trailer while the PR declares AI assistance; please add it where it applies. At ~2.4k changed lines across uploads, downloads, thumbnails, capabilities and account waits, splitting the helper + uploads from the download changes would ease review. The failing classic-client PermissionsTest is unrelated.

Review prepared with AI assistance (Claude Code) and checked against the head commit.

i2h3

b4361ca clears deleted on unchanged rows in every non-paginated depth-1 read, which brings back local-only deletions (inline).
cancelledWaiterDoesNotWaitForAccountTimeout still hangs instead of failing if cancellation regresses (inline).
4 of 40 commits mention Assisted-by: Codex:GPT-6, and in each a blank line separates it from Signed-off-by, so git parses only one of the two as a trailer. Please add the trailer to every commit with AI-assisted content, in the same block as Signed-off-by.

Review prepared with AI assistance (Claude Code) and checked against the head commit.

i2h3

Re-reviewed at 40d5802. The three points from the last round are resolved: the depth-1 un-tombstoning is gone, cancelledWaiterDoesNotWaitForAccountTimeout is bounded, and every commit now carries Assisted-by and Signed-off-by as trailers. swift test passes locally (XCTest: 460 tests, 1 skipped; Swift Testing: 129 tests).

One blocker remains (inline): a package fetch now returns success without the members whose rows are tombstoned.

Notes, not blocking:

directoryDownloadMetadatas also takes the remote fields (serverUrl, fileName, etag, size) from the database row. Depth-1 reads never refresh a member row in .downloadError or .downloading, so if such a member is renamed on the server, every retry requests the old path, gets a 404 and fails the package with noSuchItem. Master used the listing and recovered on the next attempt. Taking the remote fields from the listing fixes this together with the blocker.
A member moved between subfolders during a fetch is listed twice, and the second beginDownload supersedes the fetch's own earlier child operation, so the fetch fails with cannotSynchronize. A retry recovers.
The Lint job fails on new SwiftFormat rules (nestedIf, repeatedGuard, noForceUnwrapInTests). Master fails the same job, but Item+Fetch.swift, FilesDatabaseManager+Downloads.swift, MockRemoteInterface.swift and ItemModifyTests.swift are among the flagged files.

Review prepared with AI assistance (Claude Code) and checked against the head commit.