feat(sync): federated media-collection sync parity + per-category sync integrity#468
Merged
Conversation
…egrity design spec
Tasks 1.3 + 1.4 + minor guard from the federated-media-sync-integrity plan.
- deleteCollection now soft-deletes (marks deleted:true, blanks items/links)
instead of hard-removing the row, so tombstones persist for peer-sync push
- emitRecordDeleted('mediaCollection', id) fires after every soft-delete
- updateCollection guards against patching a tombstone (ERR_NOT_FOUND)
to prevent LWW resurrection via updatedAt churn
- mergeMediaCollectionsFromSync LWW branch propagates deleted/deletedAt from
the winning side and blanks items/universeId/seriesId/coverKey when deleted
- 5 new tests covering: soft-delete visibility, recordDeleted event, tombstone
guard on update, newer remote tombstone wins, older remote tombstone loses,
remote tombstone for unknown id stored correctly
…scribe
- createCollection now emits recordUpdated('mediaCollection') and fires-and-forgets
autoSubscribeRecordToAllPeers via dynamic import (mirrors universe/series pattern)
- PEER_SUBSCRIBABLE_KINDS expanded to include 'mediaCollection'
- KIND_TO_CATEGORY maps 'mediaCollection' → 'mediaCollections'
- PORTOS_SCHEMA_VERSIONS.mediaCollections = 1 replaces the future-comment placeholder
- Tests: createCollection emits updated event; PEER_SUBSCRIBABLE_KINDS includes all three kinds;
schemaVersions declares mediaCollections at v1
… wiring
- Part A: getCollection accepts { includeDeleted } option for tombstone-aware peer push
- Part B: export collectCollectionAssetReferences + private buildCollectionAssetManifest; import getCollection + listCollections from mediaCollections.js
- Part C: buildPushPayload mediaCollection branch (asset manifest + tombstone-aware empty)
- Part D: applyIncomingPush mediaCollection branch routes through mergeMediaCollectionsFromSync
- Part E: mediaCollectionPushSchema added to peerSyncPushSchema discriminated union
- Part F: syncWire mediaCollection case strips/re-adds soft-delete tail for byte-stable checksums
- Part G1: peerSubscribeSchema recordKind enum extended to include mediaCollection
- Part G2: autoSubscribePeerToAllRecords mediaCollection branch uses listCollections({ includeDeleted: false })
- Tests: collectCollectionAssetReferences unit, applyIncomingPush integration (real merge + mock assertion), peerSyncPushSchema parse acceptance
Adds fetchSyncIntegrity/syncRecordToPeer/syncNowForPeer/pullMissingMetadata wrappers to apiPeerSync.js (integrity calls silent, mutations caller-opt-in silent), and useSyncIntegrity hook that fans out to all eligible online peers and reduces results to worst-case statusById + byPeer breakdown maps. Worst-case ranking: assets-missing > diverged > peer-only > local-only > in-parity. Race-guard via generation counter; per-peer errors degrade gracefully to unavailable. Barrel (index.js) and README updated; 22 new tests all green (420 total).
…tion sync badges - Add presentational SyncBadge component (5 integrity statuses + not-syncing + null) - Add reusable SyncDetailDrawer (kind-aware, renders per-peer breakdown, collection preview thumbnails via MediaImage, sync-to-peer and pull-missing-metadata actions) - Add /media/collections/:id/sync route via MediaCollectionSyncView page wrapper - Wire SyncBadge onto each non-synthetic collection card in MediaCollections list - 25 new tests across SyncBadge, SyncDetailDrawer, MediaCollections (445 total, all green)
…ema field, encodeURIComponent, comment)
…sync-integrity # Conflicts: # PLAN.md
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the federated peer-sync system to bring media collections to parity with universes/series, ensures image generation prompts sync via .metadata.json sidecars, and adds per-category sync integrity endpoints plus client UI (badges + deep-linkable drawer) and manual sync controls.
Changes:
- Add
mediaCollectionas a first-class peer-sync record kind (subscriptions, pushes, reverse-subscribe support) with soft-delete tombstones and tombstone GC support. - Sync image gen-params by advertising a canonical
sidecarSha256and pulling.metadata.jsonsidecars alongside images; add a manual backfill endpoint and “Pull missing prompts” UI. - Introduce integrity manifests/diffs (
/api/peer-sync/manifest,/api/peer-sync/integrity), a purecomputeRecordIntegrity, and client UI (SyncBadge,SyncDetailDrawer,useSyncIntegrity) + manual sync routes.
Reviewed changes
Copilot reviewed 54 out of 54 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| server/services/syncOrchestrator.test.js | Adds regression test ensuring snapshot category is skipped when per-record mediaCollection subs exist. |
| server/services/syncOrchestrator.js | Adds mediaCollections to snapshot-skip mapping and tombstone GC logging. |
| server/services/sharing/tombstoneGc.test.js | Extends tombstone sweep tests to include mediaCollection pruning/refusal logic. |
| server/services/sharing/tombstoneGc.js | Adds mediaCollection kind to snapshot mapping, cutoffs, sweep return shape, and refusal calculation. |
| server/services/sharing/sidecarSync.test.js | New tests for sidecar pull + backfill behavior and traversal guards. |
| server/services/sharing/sidecarSync.js | New sidecar pull/backfill helpers for gen-params metadata. |
| server/services/sharing/peerSync.test.js | Adds extensive tests for mediaCollection sync, sidecar hashing/diffing, and manual sync APIs. |
| server/services/sharing/peerSync.js | Adds mediaCollection push/receive, collection asset manifesting, sidecar sync, and manual sync functions. |
| server/services/sharing/integrity.test.js | New tests for manifest building and peer integrity fetch/diff behavior. |
| server/services/sharing/integrity.js | New manifest builder + peer integrity fetcher using pure diff. |
| server/services/sharing/index.js | Re-exports sidecar sync helpers. |
| server/services/sharing/buckets.test.js | Adds tests for shared sanitizeAssetFilename. |
| server/services/sharing/buckets.js | Introduces shared sanitizeAssetFilename and exports it. |
| server/services/mediaCollections.test.js | Adds tests for soft-delete semantics, tombstone merge behavior, and pruning. |
| server/services/mediaCollections.js | Implements soft-delete fields, includeDeleted listing, tombstone merge behavior, and pruning. |
| server/routes/peerSync.test.js | Adds route tests for manifest/integrity and manual sync endpoints. |
| server/routes/peerSync.js | Adds manifest/integrity routes and manual sync endpoints with Zod validation. |
| server/lib/validation.js | Extends peer-sync schemas for mediaCollection + sidecarSha256 + manual sync action schemas. |
| server/lib/syncWire.js | Ensures stable wire output for mediaCollection by stripping/re-adding soft-delete fields. |
| server/lib/syncIntegrity.test.js | New unit tests for computeRecordIntegrity. |
| server/lib/syncIntegrity.js | New pure manifest diff and status classifier. |
| server/lib/schemaVersions.test.js | Adds test asserting mediaCollections schema version exists. |
| server/lib/schemaVersions.js | Declares mediaCollections: 1. |
| server/lib/README.md | Documents new syncIntegrity.js and canonical hashing helper usage. |
| server/lib/objects.test.js | Adds tests for canonicalStringify. |
| server/lib/objects.js | Adds stable canonicalStringify for cross-machine hashing. |
| server/lib/index.js | Re-exports syncIntegrity.js. |
| server/lib/assetHash.test.js | Adds tests for sidecarGenParamsHash convergence behavior. |
| server/lib/assetHash.js | Adds sidecarGenParamsHash using canonical JSON hashing excluding local cache blocks. |
| PLAN.md | Captures deferred follow-ups for the feature area. |
| docs/superpowers/specs/2026-05-23-federated-media-sync-integrity-design.md | New design spec documenting the approach and APIs. |
| client/src/services/apiPeerSync.test.js | New tests for integrity and manual sync client APIs. |
| client/src/services/apiPeerSync.js | Adds integrity fetch + manual sync requests; extends subscribable kinds. |
| client/src/pages/Universes.jsx | Adds SyncBadge per universe row and deep-link to sync drawer. |
| client/src/pages/SyncView.test.jsx | New tests for the generic sync drawer route wrapper. |
| client/src/pages/SyncView.jsx | New wrapper route for deep-linkable SyncDetailDrawer (universe/series). |
| client/src/pages/Pipeline.jsx | Adds SyncBadge per series row and deep-link to sync drawer. |
| client/src/pages/MediaCollectionSyncView.jsx | New deep-linkable sync drawer route for media collections. |
| client/src/pages/MediaCollections.test.jsx | Adds tests ensuring SyncBadge shows on non-synthetic collections only. |
| client/src/pages/MediaCollections.jsx | Adds SyncBadge per collection row and deep-link to sync drawer. |
| client/src/pages/MediaCollectionDetail.test.jsx | Adds tests for “Pull missing prompts” behavior in Unsorted view. |
| client/src/pages/MediaCollectionDetail.jsx | Adds Unsorted-only “Pull missing prompts” action calling backfill endpoint. |
| client/src/pages/Instances.jsx | Updates tombstone GC UI copy/logic to include mediaCollection cohort. |
| client/src/hooks/useSyncIntegrity.test.jsx | New tests for hook peer filtering, reduction, and refresh behavior. |
| client/src/hooks/useSyncIntegrity.js | New hook to poll integrity per peer and reduce to worst-case status maps. |
| client/src/hooks/README.md | Documents useSyncIntegrity. |
| client/src/hooks/index.js | Exports useSyncIntegrity. |
| client/src/components/sync/SyncDetailDrawer.test.jsx | New tests for sync detail drawer actions and per-kind behavior. |
| client/src/components/sync/SyncDetailDrawer.jsx | New sync drawer with per-peer breakdown and manual sync/backfill actions. |
| client/src/components/sync/SyncBadge.test.jsx | New tests for status rendering and click behavior. |
| client/src/components/sync/SyncBadge.jsx | New presentational badge for record sync status. |
| client/src/App.jsx | Registers new /…/:id/sync routes for universes/series/collections. |
| .changelog/NEXT.md | Documents the feature set for the next release. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- SyncDetailDrawer: toast() instead of non-existent toast.info (would throw) - SyncView/MediaCollectionSyncView: drop redundant decodeURIComponent (useParams already decodes; double-decode throws on bad % / mangles %25) - validation: tighten sidecarSha256 to hex-64 regex (matches sha256) - sidecarSync: require + cap content-length before buffering (HTTP path had no streaming cap) + tests - mediaCollections: deleteCollection idempotent on already-tombstoned record (no re-stamp/re-emit) + test - PLAN.md: drop the two follow-ups now fixed
- peerSync.diffAssetManifestAgainstLocal: reuse the sidecar returned by getOrComputeImageSha256 for the sidecarSha256 compare instead of a second readJSONFile — halves sidecar I/O per image - mediaCollections.sanitizeCollection: a tombstone missing deletedAt now falls back to updatedAt (not the older createdAt) so LWW + GC see the real deletion time - integrity.getPeerIntegrity: distinguish peer-unreachable (network failure) from peer-too-old (404) so the UI can message correctly - peerSync route GET /integrity: reject empty/whitespace peerId with 400 - SyncDetailDrawer: guard loadRecord's async setState with a mountedRef to avoid setState-on-unmounted warnings on fast close Tests: +empty-peerId 400 case, +tombstone deletedAt fallback, integrity unreachable-vs-too-old split. Full server suite 7249 green, client build clean.
- pullSidecarForImage: stream the response body with a hard cap (readBodyCapped via res.body.getReader) instead of res.arrayBuffer(). The HTTPS shim enforces maxBytes mid-stream, but the plain-HTTP native-fetch path did not — a peer lying about Content-Length (or chunked transfer) could buffer an unbounded body before the post-read size check. Now aborts mid-stream once the cap is exceeded; falls back to arrayBuffer when no reader (shim already capped, or test mock). (The drawer 'No sync-enabled peers' copy Copilot re-flagged was already fixed in the prior round — its review ran against a stale snapshot.) Tests: +streaming happy-path, +lying-Content-Length abort. Server suite 7276 green.
- deleteCollection: clear coverKey on the tombstone — with items emptied it would otherwise dangle (point at a non-existent item) and leak into the tombstone's wire payload. - buildLocalManifest: hash records sequentially (for...of) instead of Promise.all(map), so a large library can't fan out an unbounded number of concurrent file-hash reads and spike CPU/disk. - SyncDetailDrawer.loadRecord: skip the fetch when recordId is empty (a param-less mount would hit '/media/collections/' and 404/toast); clear recordLoading immediately. - sidecarSync: move the orphaned pullSidecarForImage JSDoc — readBodyCapped now sits above it so each doc attaches to its own function. Tests: +tombstone coverKey cleared, +empty-recordId no-fetch. Server 7277, client 485, build clean.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 54 out of 54 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (3)
server/services/mediaCollections.js:629
- addItem() can still mutate a soft-deleted (tombstoned) collection because it loads with includeDeleted:true but never rejects cur.deleted===true. That allows writes to a record that the main UI/API treats as deleted and can cause unexpected churn in tombstones.
This issue also appears in the following locations of the same file:
- line 688
- line 741
server/services/mediaCollections.js:691
- bulkUpdateCollectionItems() loads collections with includeDeleted:true but does not reject tombstoned records, so callers can still mutate deleted collections (adding/removing items, bumping updatedAt) even though they should behave as not-found after soft-delete.
server/services/mediaCollections.js:744 - removeItem() also allows mutating tombstoned collections because it reads with includeDeleted:true but doesn't block cur.deleted===true. After soft-delete, collection mutators should consistently throw ERR_NOT_FOUND (matching updateCollection).
- mediaCollections item mutators (addItem, removeItem, bulkUpdateCollectionItems) now throw ERR_NOT_FOUND on a soft-deleted collection, matching updateCollection. They loaded with includeDeleted:true but never rejected tombstones, so a caller could resurrect/churn a deleted record's items + updatedAt. - SyncDetailDrawer.loadRecord: also clear the record (setRecord(null)) when skipping the fetch for an empty recordId — otherwise a stale name/preview lingered after navigating to a param-less mount. Tests: +tombstone-rejection for all three item mutators, +stale-record-cleared on empty recordId. Server 7278, client 486, build clean.
- getPeerIntegrity: sanitize the untrusted peer manifest before diffing. computeRecordIntegrity assumes each row is a non-null object with a string id (reads r.id, keys a Map on it); a hostile/malformed response (nulls, scalars, id-less objects) would throw and 500. Filter to well-formed rows first. - SyncDetailDrawer.loadRecord: add a generation-counter guard so only the LATEST in-flight fetch commits state. A rapid recordId change could otherwise let an older fetch resolve last and overwrite the newer record with a stale name/preview. (Complements the existing mountedRef unmount guard.) Tests: +hostile-manifest no-throw/filter, +stale-fetch-dropped race. Server 7279, client 487, build clean.
The round-11 swap of setTimeout(20) → __drainForTests() for the fire-and-forget
initial-push wait passed locally but failed in slower CI ('expected null to be
truthy'): __drainForTests settles the writeTail but the push's peerFetch +
persistPushSuccess chain may not have started when it returns, so lastPushedHash
was still null. Replace with vi.waitFor polling the real condition — deterministic
on fast-local AND slow-CI, and still not a fixed sleep.
Verified: peerSync.test.js green 3x in a row; full server suite 7279.
- findOrCreate{Universe,Series}Collection + findOrCreateCollectionByName now
announce a NEWLY-created collection to the per-record peer-sync pipeline
(emit mediaCollection 'updated' + autoSubscribeRecordToAllPeers), matching
createCollection. Previously only createCollection announced, so a peer with
mediaCollections sync enabled but universe/series sync off missed new
universe/series-linked collections (and their later tombstones). Extracted a
shared announceNewCollection() helper; fires only on create, never on a
find-existing hit.
- peerPullMetadataSchema: .trim() filenames so ' a.png ' normalizes instead of
passing validation then failing disk lookup (a confusing 200 with
attempted>0, recovered=0). Matches the manifest-entry filename handling.
Tests: +announce-on-create/quiet-on-existing for all three upserts, +filename
trim passthrough. Server suite 7283 green.
- peerSyncPushSchema: accept linkedCollection ONLY on universe/series pushes, not mediaCollection. It was in the shared base, so a mediaCollection push could smuggle an arbitrary extra collection that applyIncomingPush merges — a side-channel to overwrite collections outside the explicit per-record subscription. Moved it to a linkedCollectionField spread into just the universe/series branches; the mediaCollection branch's .strict() now rejects it. buildPushPayload never sets it for the mediaCollection kind, so our own pushes are unaffected. Tests: +universe push accepts linkedCollection, +mediaCollection push with linkedCollection 400s. Server suite 7285 green.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Makes media collections federate between peers as reliably as universes/series, ensures synced images carry their generation prompts, and adds at-a-glance sync-integrity visibility — addressing the report that universes/series reached parity but collections didn't, and that some peer-synced images landed in "Unsorted" with no prompts.
mediaCollection, schema-version-gatedmediaCollections: 1): auto-subscribe on create, per-record push on edit, tombstone propagation, orchestrator snapshot-skip, and tombstone GC. Required adding soft-delete to collections (deleted/deletedAt,listCollections({ includeDeleted }), tombstone-aware LWW merge) so a delete can't be resurrected by a reverse-subscribed peer.sidecarSha256(machine-local hash-cache stripped so it converges across peers — avoids a 60s re-pull loop), and the asset-pull worker fetches each image's.metadata.jsonsidecar alongside the bytes. Auto going forward + a manual "Pull missing prompts" repair on the Unsorted view for already-bare images.computeRecordIntegritydiff + Tailnet-onlyGET /api/peer-sync/{manifest,integrity}, surfaced as aSyncBadgeon every Universe / Series / Media-Collection row (in-sync / diverged / assets-missing / local-only / on-peer-only / "not syncing — enable?") and a deep-linkableSyncDetailDrawer(/…/:id/sync) with per-peer breakdown + previews.POST /sync-record, bypasses the unchanged-hash short-circuit), sync-now a peer (POST /sync-now), and pull-metadata backfill (POST /pull-metadata).Collection sync stays opt-in per peer (default off); the badge surfaces a clear "enable?" state. Spec + plan are in
docs/superpowers/{specs,plans}/2026-05-23-federated-media-sync-integrity*; 7 deferred follow-ups captured inPLAN.md.Test Plan
cd server && npm test— 7243 passed, 7 skipped)cd client && npm test— 468 passed) +npm run buildcleanmediaCollectionsfor the peer → create a collection on A → confirm it appears on B with items + prompts; delete on A → confirm tombstone removes it on B; verify the SyncBadge reflects parity; run "Pull missing prompts" on B's Unsorted to repair pre-existing bare images