Skip to content

fix(sync): regenerate .mokuro on cloud volume rename (stop silent revert) - #233

Merged
Gnathonic merged 14 commits into
developfrom
fix/cloud-rename-sidecar
Jul 4, 2026
Merged

fix(sync): regenerate .mokuro on cloud volume rename (stop silent revert)#233
Gnathonic merged 14 commits into
developfrom
fix/cloud-rename-sidecar

Conversation

@Gnathonic

@Gnathonic Gnathonic commented Jun 14, 2026

Copy link
Copy Markdown
Owner

The bug

Renaming a cloud-backed volume moved its .mokuro sidecar but never updated the title/series fields inside it. On re-download to another device, processing.ts trusts the .mokuro's internal metadata over the filename — so for OCR volumes the rename silently reverted and series grouping broke. It affects every provider, so the fix lives in the shared unified-cloud-manager.

The fix — renameVolume(old…, new…, volumeUuid)

  • Regenerate, don't move, the .mokuro: generateVolumeSidecarsFromDb(uuid, {seriesTitle, volumeTitle}) gains overrides so the fresh sidecar is built with the new names while the DB still holds the old ones. (The remote rename gates the local commit — a throw skips it — so we never mutate local state first.)
  • Flow: collision gate → upload fresh .mokuro at the new path → move .cbz + cover (content is name-agnostic) → delete the stale .mokuro last → best-effort prune the emptied old series dir (WebDAV removeDirectoryIfEmpty, server-checked emptiness).
  • Retry converges without error-swallowing: every step is idempotent and the only destructive step is last. A file moved by a prior partial run is simply absent from the fresh source listing, so it never re-enters the move loop.

Safety gates (all fire before any remote write)

  • Typed NOT_FOUND, no message sniffing: each provider throws ProviderError('NOT_FOUND') at the boundary where the status is unambiguous. A 404 during a move is a genuine failure (deleted elsewhere / stale id) and aborts before the destructive delete; a 404 during a delete is convergence (absence is the postcondition).
  • Collision gate + overwrite prompt: renaming onto a name another backup occupies throws TARGET_EXISTS before the .mokuro upsert could corrupt the occupant's sidecar. The volume editor then offers overwrite-or-cancel; overwrite deletes the occupant explicitly first.
  • WebDAV idempotent-retry hardening: "destination occupied + source gone" only counts as our prior move when the occupant matches the source's size (identity proxy, paralleling Drive's id / MEGA's nodeId checks). Any other occupant throws.
  • Read-only providers no longer block local-only renames: the READ_ONLY gate fires only when there are actually cloud files to move.
  • SIDECAR_REGEN_FAILED: an OCR volume whose sidecar can't be rebuilt (missing volume_ocr) refuses to rename rather than moving the stale sidecar.

Series rename — per-volume, not all-or-nothing

executeRenameSeries now runs one volume at a time; each volume's cloud rename gates that volume's local commit. Failures are collected per volume (accurate message, no false "kept in sync"), the succeeded volumes stay consistent in both places, and retrying the same rename finishes just the stragglers. On full success the view navigates to the new series.

Cloud-only volumes block the rename with a download-first notice: their .mokuro can't be regenerated without local OCR, and renaming around them would split the series across two cloud folders.

TODO(data-update): the proper fix is downloading a volume's .mokuro/metadata without the full archive — blocked on the metadata-persistence data update (metadata surviving volume deletion, see #201).

Title filesystem-safety sanitization (stacked feature)

A pure sanitizeTitleSegment helper substitutes Windows/OneDrive-illegal characters with fullwidth look-alikes (plus control-strip, dot-leader, reserved-name, and trim rules), applied at the title write points: local import, series rename, and volume rename.

Cloud downloads are exempt (preserveTitles): their titles must keep matching the remote path, or legacy backups with : ? * etc. would read as un-backed-up (duplicate uploads) and renames would miss their files — re-enabling the very revert bug this PR fixes. Legacy titles get sanitized on their next rename, when the cloud files move along with the title.

Verification

  • 753 unit tests, svelte-check clean. New coverage: read-only + local-only rename proceeds, move-failure aborts before the destructive delete, retry convergence via absent sources, TARGET_EXISTS gate + overwrite path, cloud-only block, per-volume failure collection, partial local-commit filtering, WebDAV size-verified idempotency, typed 404 mapping, preserved cloud-download titles.
  • Two review passes (adversarial multi-agent); all confirmed findings folded in.
  • Manual test checklist:
    • Rename a cloud-backed volume (WebDAV/MEGA/Drive) → re-download on second device keeps the new name
    • Rename a volume onto an existing backup's name → overwrite/cancel prompt; cancel changes nothing
    • Rename a series with a volume deleted locally but still in cloud → blocked with download-first notice
    • Rename a series over flaky connection → partial failure reports which volumes failed; retry finishes them
    • Rename a never-backed-up volume while connected to a read-only server → succeeds locally
    • Re-download a legacy backup whose name contains : or ? → still shows as backed up, no duplicate upload

🤖 Generated with Claude Code

…ert)

Renaming a cloud-backed volume MOVED its .mokuro sidecar but never updated
the title/series fields embedded inside it. On re-download to another
device, processing.ts trusts the .mokuro's internal metadata over the
filename, so the rename silently reverted for OCR volumes (and series
grouping broke). The problem is provider-agnostic, so the fix lives in the
shared unified-cloud-manager.

renameVolume(old…, new…, volumeUuid) now:
- regenerates the .mokuro from the DB with the NEW names via overrides
  (generateVolumeSidecarsFromDb gains a {seriesTitle, volumeTitle} arg) —
  the DB still holds the old names because the remote rename GATES the
  local commit (handleSave already runs the cloud rename before the DB
  update, and a throw here skips it);
- uploads the fresh .mokuro at the new path, MOVES the cbz + cover (their
  content is name-agnostic), then deletes the stale .mokuro LAST;
- is idempotent (an already-moved source / already-gone target is treated
  as done) and destructive-last, so a partial failure leaves the old files
  intact and a retry converges — no journal needed;
- throws on a read-only provider (never desyncs local ahead of remote) and
  throws SIDECAR_REGEN_FAILED if an OCR volume's sidecar can't be rebuilt
  (volume_ocr missing) before touching anything remote;
- best-effort prunes the emptied old series directory via a new optional
  provider.removeDirectoryIfEmpty — server-checked emptiness only, never a
  blind recursive delete.

WebDAV renameFile is now idempotent on retry (destination occupied by the
already-moved source = success, matching Drive/MEGA), so a re-run after a
dropped MOVE response converges instead of throwing TARGET_EXISTS.

Edge cases from adversarial review folded in. Follow-up: renameSeries (bulk
folder rename) has the same latent staleness and needs the same treatment.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@vercel

vercel Bot commented Jun 14, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
mokuro-reader Ready Ready Preview, Comment Jul 4, 2026 5:01pm

Request Review

renameSeries now fans out the per-volume rename for OCR series so each
.mokuro's embedded series title is regenerated (a bulk folder move alone
left them stale, silently reverting the rename on re-download). Reuses the
idempotent, destructive-last renameVolumeFiles core so a partial failure
converges on retry; image-only series still take the provider-optimized
folder move. The remote rename gates the local commit — when the cloud
can't be updated (offline, read-only, …) the local rename is aborted and
the user is told why (snackbar / inline error), without surfacing the
low-level cause.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Gnathonic and others added 6 commits June 29, 2026 16:03
Sanitize series/volume titles at their source (rename + import) so the
stored title is safe as a file/folder name on every sink — MEGA, Drive,
WebDAV, OneDrive, the File System Access API, and local export. Fullwidth
look-alike substitution for the Windows-illegal set; going-forward only;
no unified-cloud-manager changes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sanitize the volume title on every save and a newly-created series name.
Existing-series titles are left untouched here (series-wide sanitization
happens in the series-rename flow) to avoid splitting a series across two
cloud folders.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Gnathonic and others added 4 commits July 3, 2026 19:29
…me ordering

Review findings on the rename flow, all pre-destructive-step:

- Replace '404'/'not found' message sniffing with typed ProviderError
  NOT_FOUND thrown at each provider boundary (WebDAV/MEGA/Drive rename +
  delete). A NOT_FOUND during a MOVE is now a genuine failure that aborts
  before the stale-.mokuro delete — an already-moved file is simply absent
  from the fresh source listing, so no error-swallowing is needed for retry
  convergence. Deletes still converge on NOT_FOUND (absence IS the
  postcondition) and drop the stale cache entry.
- WebDAV renameFile's already-at-destination branch now requires the source
  to be gone AND the occupant to match the source's size (identity proxy,
  paralleling Drive's id / MEGA's nodeId checks) instead of adopting any
  occupant.
- Collision gate before any remote write: renaming onto an occupied volume
  name throws TARGET_EXISTS before the .mokuro upsert could corrupt the
  occupant's sidecar. Explicit overwrite option deletes the occupant first.
- READ_ONLY gate moved after the zero-cloud-files check so a read-only
  provider (anonymous session, auto-demoted server) no longer blocks
  purely-local renames.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rite prompt

- executeRenameSeries now commits each volume locally ONLY after its cloud
  rename succeeded, collecting per-volume failures instead of aborting the
  whole series mid-loop with a false 'kept in sync' message. Failed volumes
  keep the old title everywhere; retrying the same rename converges on just
  the stragglers. SeriesView navigates to the new series on full success and
  reports per-volume failures otherwise.
- Series rename is blocked with a download-first notice when the cloud holds
  volumes missing from the local library — renaming around them would split
  the series across two cloud folders.
  TODO(data-update): the proper fix is downloading a volume's .mokuro/
  metadata without the full archive (blocked on the metadata-persistence
  work; see PR #201).
- The rename volume list now comes from the preview (which carries
  volumeTitle) instead of re-running the same Dexie query, closing a
  divergence window between the cloud rename list and the local commit list.
- VolumeEditorModal offers overwrite-or-cancel when the new volume name
  collides with an existing cloud backup (TARGET_EXISTS).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…th identity

Sanitizing titles during a cloud download broke the stored-title ===
cloud-path identity for legacy backups whose names contain now-illegal
characters (':' etc. are legal on Drive/MEGA/most WebDAV): the volume read
as un-backed-up (duplicate uploads) and renames found zero managed files —
committing locally while the stale cloud sidecar survived, the exact silent
revert this branch fixes.

Cloud downloads now save titles verbatim (preserveTitles); sanitization
still applies to local file imports and at rename time, where the rename
machinery moves the cloud files together with the title.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…de it binary)

The spec embedded literal \x00/\x1F/\x7F control bytes where it meant the
printable escape notation, so git treated the file as binary. Prettier
formatting applied to the now-text docs and the sanitize-title test.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Ceynou

Ceynou commented Jul 4, 2026

Copy link
Copy Markdown

Is the renaming function disabled until this is merged?

@Gnathonic

Copy link
Copy Markdown
Owner Author

Is the renaming function disabled until this is merged? @Ceynou

Nah, the bugs that come from renames as it stands are minor and can be fixed with a quick script.
This is to fix the need for the script and other bugs that come from things as they stand.
This fix if done poorly has the ability to make things worse, hence the slowness in finalizing.
Will probably merge in today.

…older prune

Series rename on MEGA left the old folder behind and showed the renamed
series as un-backed-up until a page refresh. Root causes:

- megajs applies moves/deletes to its local tree ONLY via the server-change
  (sc) long-poll, which we ran with keepalive:false — so storage.files went
  permanently stale after every move, and the full cache rebuild inside
  deleteFile clobbered the manager's correct incremental updates with that
  stale tree.
- The reason keepalive was disabled ('sc handler crashes on delete events')
  was self-inflicted: we manually removed nodes from storage.files, so the
  later sc delete packet found no node and crashed reading .parent.

The fix restores the intended architecture — listen to MEGA and update the
GUI reactively:

- Remove the manual storage.files removals (deleteFile, uploadFile replace)
  and re-enable keepalive on login and session restore.
- Subscribe to megajs add/move/delete/update events; a debounced rebuild
  from the now-authoritative tree keeps the reactive cache (and the GUI that
  subscribes to it) current, including changes made from other devices. The
  manager's incremental updates still give instant feedback for our own ops.
- Drop the mid-operation fetch(true) rebuilds (upload/delete/deleteSeries)
  that raced the sc stream.
- Mirror moves into the local tree immediately after moveTo (the sc handler
  no-ops when the parent already matches) so same-operation checks are
  accurate.
- Implement removeDirectoryIfEmpty for MEGA: emptiness is confirmed with a
  live 'f' API query (never the possibly-stale local tree, never a blind
  recursive delete), so series renames now prune the emptied old folder.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Gnathonic
Gnathonic merged commit 2f31a19 into develop Jul 4, 2026
9 checks passed
@Gnathonic
Gnathonic deleted the fix/cloud-rename-sidecar branch July 4, 2026 19:59
@Gnathonic Gnathonic mentioned this pull request Jul 5, 2026
adrian-tompkins pushed a commit to adrian-tompkins/mokuro-reader that referenced this pull request Aug 2, 2026
…-scroll

fix: Prevent browser-level horizontal scrolling in reader
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants