Add offline purge of orphaned received shares to the jsoncs3 share manager

[kobergj]

Summary

The jsoncs3 share manager can accumulate orphaned received-share entries: a shareID stored in a user's received.json whose share no longer exists in the provider cache. These are left behind when a space is deleted but the per-user received-state blocks are not cleaned up (e.g. after a missed SpaceDeleted event).

They are removed inline on ListReceivedShares, but that happens on the request hot path — with many orphans and/or HA replicas this can monopolize the call and race a client deadline (see OCISDEV-859 / the desktop-client disconnect symptom in BYCS school-2638, where ~47 orphans on one deleted space kept affected users in a permanent reconnect loop).

This adds an offline, batched way to drain them.

Changes

• Manager.PurgeOrphanedReceivedShares(ctx, spaceID, dryRun) — enumerates users (metadata.Storage.ReadDir("/users")), and for each received-share entry checks whether the share still exists in the provider cache. Confirmed orphans are removed in a single batched persist per user+space. Supports scoping to one space and a dry-run mode.
• receivedsharecache.Cache.RemoveSpaceShares — removes a set of shareIDs for one user's space in one persist (batched counterpart of Remove), reusing the existing retry + full-jitter backoff + ctx cancellation loop.

Safety

An entry is only removed when the share is confirmed absent from the provider cache after a successful ListSpace prime — exactly the predicate the inline cleanup already uses. If a space's share list cannot be read, the whole space is skipped (counted as an error), so live shares are never removed on a transient failure.

Tests

• Unit tests for RemoveSpaceShares (batched removal, selective removal, persistence, empty-list no-op).
• Existing pkg/share/manager/jsoncs3/... suite passes.

Relates to OCISDEV-859. Paired oCIS CLI command (ocis shares purge-orphaned-received-shares) to follow.

[kw-security]

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues
✅	Code Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

[Copilot]

Pull request overview

This PR adds an offline, batched purge path for orphaned per-user received-share state in the jsoncs3 share manager, avoiding costly inline cleanup during ListReceivedShares when many orphan entries exist.

Changes:

• Add Manager.PurgeOrphanedReceivedShares(ctx, spaceID, dryRun) to scan users’ received-share state and remove entries confirmed missing from the provider cache (after a successful ListSpace prime).
• Add receivedsharecache.Cache.RemoveSpaceShares(...) to remove multiple received-share entries for a given user+space in a single persisted write (with the existing retry/backoff behavior).
• Add unit tests for RemoveSpaceShares and a changelog entry describing the new functionality.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File	Description
pkg/share/manager/jsoncs3/receivedsharecache/receivedsharecache.go	Adds RemoveSpaceShares batched deletion method with retry/sync loop.
pkg/share/manager/jsoncs3/receivedsharecache/receivedsharecache_test.go	Adds test coverage for batched removal behavior and persistence.
pkg/share/manager/jsoncs3/jsoncs3.go	Adds PurgeOrphanedReceivedShares manager method to perform offline orphan cleanup.
changelog/unreleased/purge-orphaned-received-shares.md	Documents the enhancement and links to the PR.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[kobergj]

Thanks for the review. Addressed the feedback in the (amended) commit:

• Space-key format in the docstring — corrected to {storageID}:{spaceID} (joined by shareid.IDDelimiter), matching how the key is actually built at the UserReceivedStates.Add call sites. This was a real correctness issue for the --space-id scoping, good catch.
• Orphan-ID logging — the count now logs at Info; the full share-ID list moved to Debug to avoid large log lines / identifier exposure on big purges.
• Missing tests for PurgeOrphanedReceivedShares — added specs covering: no-op when the share still exists, orphan removal + persistence, dry-run (found but not removed), --space-id scoping, and the "skip the whole space on ListSpace failure" safety predicate.
• "Thas" typo — fixed in the new method.

The span-attributes-after-span.End() note is accurate but is the existing idiom copied from Add/Remove in the same file; I left it as-is to keep the new method consistent with its neighbours rather than diverge in a single spot.

[jvillafanez]

I think this should be moved to the beginning of the function, before the if len(shareIDs) == 0 check, otherwise, calls without "shareIDs" won't be registered.
While it won't add very useful data (the function will return right away), knowing that the method has been called might still be useful.

[jvillafanez]

Do we have guarantees that the spaceID always exists? otherwise, the receivedSpace might be nil, and it could crash the app.

[jvillafanez]

It might be easier to use time.After(...).
Alternatively, you might want to create the timer once (before the attempt loop) and reset the duration here. Docs seems a bit confusing though, you might want to double-check for potential problems.

[jvillafanez]

Any additional information we can extract from the errors? Right now, I don't think it matters that there have been 3 errors or 300, what matters is if there have been any error or none at all. A boolean type provides the same value.

Maybe including the first 25 or 50 error messages (if there are more) might be interesting. We don't want to have all the error messages, but some of them might be useful to have an idea of what's going on.

[kobergj]

Thanks @jvillafanez — addressed most of it in the amended commit:

• Span before the early return (298): moved the RemoveSpaceShares span start above the len(shareIDs) == 0 check so empty-list calls are still traced. Kept a single span for the method (dropped the separate "Grab lock" span) so attributes are set before End(), which also covers the Copilot note on the same lines.
• Possible nil receivedSpace (304): good catch. initializeIfNeeded only creates the space entry for a non-empty spaceID, so I added an explicit if receivedSpace == nil { return nil } guard. (The sibling Remove has the same latent shape; happy to follow up on that separately so this PR stays focused.)
• Errors int value (1300): kept the count but added a capped ErrorSamples []string (first 50) via a small recordError helper, and the oCIS CLI now prints the samples. Gives the "what went wrong" signal without an unbounded dump.

On time.After (353): I'd prefer to keep time.NewTimer + timer.Stop() here. This backoff block was introduced in #611 specifically to fix a time.After timer-GC leak (the timer isn't collected until it fires, which piles up under the retry loop). Switching back to time.After would reintroduce that. Creating one timer before the loop and Reset-ing it is possible but the Reset-after-fire semantics are exactly the footgun the docs warn about, so I'd rather leave the explicit per-attempt timer. Let me know if you feel strongly.

Added manager-level tests for PurgeOrphanedReceivedShares too (orphan removal, dry-run, --space-id scoping, and the skip-on-ListSpace-failure safety path incl. the error sample).

[jvillafanez]

According to the docs, the timer-GC leak should be fixed with go 1.23 (https://pkg.go.dev/time#After). Anyway, the docs aren't very clear with some aspects of the behavior, and I don't think this is a time-critical path.