Skip to content

fix(file-provider): do not treat a stale-path 404 as a deletion - #11030

Open
juliusvaart wants to merge 1 commit into
nextcloud:masterfrom
juliusvaart:macos/vf/stale-path-404
Open

juliusvaart wants to merge 1 commit into
nextcloud:masterfrom
juliusvaart:macos/vf/stale-path-404

Conversation

@juliusvaart

@juliusvaart juliusvaart commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Resolves

No issue filed. Found while testing on a live account.

Summary

A folder renamed while the working-set scan is reading it is reported as deleted, and macOS removes it from disk while it still exists on the server.

The scan reads from a snapshot of the materialised set taken before any request goes out. If a rename or move commits while the PROPFIND for the old path is in flight, the server answers 404. That 404 is about a path nobody occupies any more, not about the item, but the scan treats it as a deletion:

  • the item is reported deleted, so the system removes the folder locally;
  • its database row is written back with the pre-rename name and deleted set.

survivingOcIds does not catch this. It only spares items that reappear in the pass's updates or creations, and the move has already written the correct row, so the new parent's depth-1 read finds the item unchanged and never reports it.

The fix

Before believing the 404, the scan looks up the item's current row. If the row now points somewhere else, the path was stale and the item is skipped. Otherwise the 404 counts as a deletion, as before.

The change is a guard of about 10 lines in Enumerator+WorkingSetScan.swift.

Tests

New RemoteChangePropagationTests.testA404ForAPathTheItemHasMovedAwayFromIsNotADeletion: the server has already renamed the folder, and the rename reaches the database while the scan's read of the old path is in flight. It fails without the fix and passes with it.

Full suite, run locally with swift test on top of master (edf26f2b1b):

  • 390 XCTest (6 skipped, 0 failures) and 195 swift-testing tests, on master 3527955c09.
  • The only failures were the known timing-flaky EnumeratorTests cases (testWorkingSetChangesBatchMixingUpdatesAndDeletions, testWorkingSetChangesResumeAcrossNewEnumeratorInstances, testTrashChangeEnumeration). They fail the same way on unmodified master: three runs there gave 1, 0 and 3 failures. A rerun of EnumeratorTests on this branch was clean.
  • swiftformat --lint and git diff --check are clean.

Checklist

AI (if applicable)

The code, tests and this description were written by Claude Code (Claude Opus 5.5) at my request, from a diagnosis of my own account's logs and database.

🤖 Generated with Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

@camilasan camilasan added this to the 35.0.2 milestone Oct 27, 2026
@nilsding nilsding modified the milestones: 35.0.2, 35.0.3 Oct 30, 2026
The working-set scan reads from a snapshot of the materialised set taken
before any request goes out. A rename or move that commits while one of
those reads is in flight makes the 404 a statement about a path nothing
occupies any more, not about the item. The scan reported the item
deleted: the system removed a folder from disk seconds after the user
renamed it, while it still existed on the server, and the item's row was
rewritten to its old name with deleted set.

survivingOcIds does not catch this. It only spares items that reappear in
the pass's updates or creations, and the move has already written the
correct row, so the new parent's read finds the item unchanged and never
reports it.

Check the database before believing the 404. If the row has moved
elsewhere since, the path is stale and the item is alive.

Assisted-by: ClaudeCode:claude-opus-5-5
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: Julius van der Vaart <julius@vanderva.art>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants