Fix macOS File Provider Synchronization Drops

[juliusvaart]

Summary

First of eight PRs splitting #10728 into self-contained changes, as requested in review.
This one is correctness only — five fixes for cases where remote changes silently never
reach the Mac, plus the log noise that made those cases hard to diagnose.

No performance claims here, so there is nothing to benchmark. The figures quoted below
are observations from live runs that motivated each fix, not measurements of an improvement.
The benchmark harness the later PRs use is #.

What is fixed

Locally created directories are never scanned — a folder created on the Mac never got
visitedDirectory, which is what puts a directory into the materialised set the working-set
scan reads. Items added to it on the server (web UI, public upload link, another user) never
surfaced, and no later enumerateItems existed to repair it.
Resolves #9688.

Trashed rows break the working-set scan — trashing rewrites an item's serverUrl to the
trashbin but leaves deleted false, and a folder keeps visitedDirectory. The scan then
PROPFINDed the trashed folder through the ordinary DAV path, which 404s, and read that 404 as
"the item is gone" — reporting it deleted and hard-removing the row the trash reconciliation
derives permanent deletions from. Trash has its own path via listingTrashAsync.

Requests arriving before the account does — the system starts the extension and asks it
for work before the main app has handed the account across. fetchContents and item(for:)
answered notAuthenticated outright and the framework treated those downloads as failed. On
one relaunch, 17 fetches arrived in the first 0.6 s, 1.7 s before the account landed, and most
were never re-requested. Both now wait for the account, turning the race into a short delay.
createItem / modifyItem / deleteItem are deliberately left alone — they are
user-initiated — and enumerator(for:) already self-heals.

Expected noSuchItem logged as an error — pinning a subtree asks the framework to download
every descendant, but that addresses the framework's own item store, which only knows what
enumeration has handed it. Every never-browsed descendant answers noSuchItem: 15,229 of
15,849 on one pinned tree, roughly half the log volume. Nothing is lost by skipping them, since
the database flag is written first and contentPolicy reports
.downloadEagerlyAndKeepDownloaded when the item is first enumerated. Genuine failures still
log as errors.

Regression tests for the path a notify_push or root-ETag signal actually drives:
enumerateChanges(.workingSet) → scanMaterialisedItemsForRemoteChanges() →
pendingWorkingSetChanges(since:) → the change observer. Three of them reproduced real silent
drops before the fixes and now guard them.

Verification

• 357 tests pass, 7 of them new (350 on stable-34.0)
• Every commit builds and tests standalone, so the history bisects
• Each fix has a test that fails without it

TODO

• Close Fix file provider issues with directory handling and performance #10728 once the split is agreed
• Rebase after the scan PRs land — Enumerator+WorkingSetScan.swift is touched by both
• Labels and milestone need a maintainer; I have no triage rights on this repo

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.
• Uploaded screenshots from before and after for UI changes. — n/a, no UI changes
• Test(s) added to branch, with before/after results included in PR description, for performance improvements where applicable. — tests added; no performance claims in this PR
• Documentation has been updated or is not required. — not required
• Backports requested where applicable (critical bugfixes). — n/a, targets stable-34.0
• Labels added where applicable (bug/enhancement). — needs maintainer
• Milestone added for target version. — needs maintainer

[github-actions]

Artifact containing the AppImage: nextcloud-appimage-pr-10734.zip

Digest: sha256:63356841deb6323528368223b3088d1b6fff3b81ca93fe00e5b447de37a1b6c5

To test this change/fix you can download the above artifact file, unzip it, and run it.

Please make sure to quit your existing Nextcloud app and backup your data.

[i2h3]

Looks good to me overall.

[i2h3]

This is the only thing I consider worth changing. Waiting for a fixed time will result in flaky tests because hosted GitHub action runners sometimes are horribly slow. I fought this problem so often already. Changing the pattern of awaitAccount() to have a return value or throw an error likely mitigates the issue.

Implementing tests with Swift Testing would enable @Test(.timeLimit(.seconds(5)). Otherwise waitForExpectations(timeout:handler:) in XCTest.

[juliusvaart]

Hi @i2h3,

According to Claude, removing the timing assertion was the right call (sorry, Swift is beyond me, but i understand the logic).

Summary:

Rather than loosening the bound I took your suggestion about the return value.
awaitAccount now throws NSFileProviderError(.notAuthenticated) on timeout
instead of answering nil. Returning an optional conflated "no account arrived"
with "the account was already there", which is why a clock was the only way to
tell the two apart. Now the outcome carries that information, so the test calls
it with timeoutNanoseconds: 0: the fast path is the only way to get an account
back, since parking would throw. No elapsed time involved.

I also moved the file to Swift Testing, as you suggested — #expect(throws:) is
much nicer than the XCTest dance for an async throwing call.

15 consecutive runs pass, and 5 more with 12 CPU-saturating processes running
alongside. More to the point, there's no upper-bound time assertion left in the
file at all, so that failure mode isn't just less likely, it's gone.

Two small notes, neither of them disagreements:

The timeout test still checks that at least 300 ms elapsed. That one is a lower
bound, so a slow runner can only overshoot it — and it's the only thing left
distinguishing "waited and gave up" from "failed instantly". Happy to drop it if
you'd rather have no clock in there whatsoever.

On @Test(.timeLimit(...)) — I tried it, and Swift Testing only accepts minutes:
'seconds' is unavailable: Time limit must be specified in minutes. I think that
restriction is deliberate, since a time limit is really a backstop against hangs
rather than a promptness assertion. It also wouldn't have caught this particular
bug: a broken fast path would have waited the full 5 s and still finished well
inside any limit. Useful to know for the flaky tests you mentioned elsewhere,
though.

The 200 ms sleeps in the park-and-release tests I've left as they are. They only
order the two halves and pass either way round — if the sleep is too short the
account is published first, the fast path runs, and the same assertion holds.
Same pattern as RetrievedCapabilitiesActorTests. Shout if you'd still prefer
them gone and I'll make the parking observable instead.

Sorry about the lost line anchor — I amended rather than adding a fixup commit
to keep the history clean.

[i2h3]

I will review next week.