Skip to content

making syncing usable#301

Merged
Ethran merged 24 commits into
mainfrom
webdav
Jul 19, 2026
Merged

making syncing usable#301
Ethran merged 24 commits into
mainfrom
webdav

Conversation

@Ethran

@Ethran Ethran commented Jul 19, 2026

Copy link
Copy Markdown
Owner

No description provided.

Ethran and others added 18 commits July 19, 2026 10:47
New per-notebook sync-state entity + DAO + repository, keyed by notebook
id with no foreign key to Notebook (the row must outlive local deletion
so the next sync can propagate a tombstone). Room auto-migration 34->35
creates the table; nothing reads or writes it yet.

This is the source of truth Phase 4b/4c will use for badges, deletion
detection, and (Phase 5) change detection -- the richer replacement for
SyncSettings.syncedNotebookIds.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018YdtTgjpXGQ88y6JJppogu
Write a sync-state row at each commit point (successful manifest PUT on
upload, successful notebook upsert on download) via
NotebookSyncStateRepository.markSynced. Deletion detection and
new-notebook discovery now read the table's ids instead of
SyncSettings.syncedNotebookIds:
- detectAndUploadLocalDeletions: deletedLocally = syncStateIds - localIds;
  drop the row once the tombstone is uploaded.
- downloadNewNotebooks: skip server notebooks already in the table.
- applyRemoteDeletions / uploadDeletion: drop the row when a notebook is
  gone on both sides.
Force download clears the table on wipe; force ops rely on per-notebook
commit writes. Remove the bulk updateSyncedNotebookIds finalize and drop
syncedNotebookIds from SyncSettings entirely (no migration -- the first
sync repopulates the table; an empty table detects no deletions, which
is harmless).

This also lands the deferred 2c: per-notebook success is now recorded
only at real commit points, so a failed notebook never appears synced.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018YdtTgjpXGQ88y6JJppogu
Add NotebookSyncStatusStore exposing Flow<Map<notebookId, SyncBadge>>,
combining the notebook list (new NotebookDao.getAllFlow), the
notebook_sync_state table flow, and the live SyncState:
- no row -> NOT_SYNCED
- row.state == ERROR -> ERROR
- local updatedAt newer than the last committed sync -> NOT_SYNCED
- otherwise SYNCED
While a sync is running, anything not already SYNCED reads as SYNCING.
Nothing is stored; the badge is a pure function of the three sources.
Consumed by the UI in 4d.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018YdtTgjpXGQ88y6JJppogu
Library: LibraryViewModel folds NotebookSyncStatusStore.badges into its
combined UI state; HomeView threads the map into NotebookGrid ->
NotebookCard, which draws a corner icon (CloudDone / CloudOff / Sync /
ErrorOutline) for SYNCED / NOT_SYNCED / SYNCING / ERROR. The param
defaults to null, so nothing shows when sync is unconfigured.

Settings "Last synced" line: the orchestrator now persists lastSyncTime
on every successful full sync (not just manual), so the existing
LastSyncInfo reflects background/periodic syncs too (P8).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018YdtTgjpXGQ88y6JJppogu
Two badge bugs reported on-device:
1. Every notebook showed the spinner during a sync (looked parallel).
   The overlay painted all not-SYNCED notebooks as SYNCING. Now the
   reporter carries the current notebook id (ItemProgress.id), so only
   the notebook actually being transferred spins; the rest show a new
   SCHEDULED badge (clock icon) while queued.
2. Already-synced notebooks showed as not-synced/syncing. Reconciliation
   skips unchanged notebooks ("within tolerance") without writing a row,
   so they never got a SYNCED row. The skip path now calls markSynced,
   so an unchanged notebook reads as SYNCED (CloudDone).

SyncBadge gains SCHEDULED (per WW's suggestion). beginItem takes an
optional id; both loop call sites pass it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018YdtTgjpXGQ88y6JJppogu
Synchronize documentation with recent architectural changes including the new persistence layer, ordering invariants, and UI feedback mechanisms.

- **Commit-marker ordering**: Document the "pages-before-manifest" (upload) and "page-before-timestamp" (download) ordering that ensures interrupted transfers do not leave notebooks in a partially-synced state.
- **`notebook_sync_state` Table**: Introduce the Room-backed per-notebook sync bookkeeping (schema v35) which replaces the legacy `syncedNotebookIds` set.
- **Sync Badges**: Detail the logic for the library cover badges (Synced, Not synced, Scheduled, Syncing, Error) derived by `NotebookSyncStatusStore`.
- **Concurrency & Mutex**: Document the process-wide `syncMutex` that now coordinates full syncs and "Sync on Note Close" operations to prevent races.
- **Error Handling**: Replace the internal `SyncError` enum documentation with the app-wide `AppResult`/`DomainError` pattern, including specific retry policies for `SyncWorker`.
- **Infrastructure Splits**: Reflect the extraction of `WebDavXml` (parsing) and `SyncPaths` (routing), and the transition to a shared `OkHttpClient` connection pool.
- **Atomic Operations**: Document `replaceDownloadedPage` as a single Room transaction to prevent inconsistent page states during download.
- **User Guide**: Add a badge reference table and clarify "Sync on Note Close" behavior for end users.

**Version**: 1.6
**Last Updated**: 2026-07-19
A single failing notebook (e.g. a large one) froze every notebook's badge on
SCHEDULED/SYNCING and stuck the progress bar. Root cause: the badge store derives
SYNCING/SCHEDULED from SyncState==Syncing, but the orchestrator's mid-flow step
failures returned an error without calling reporter.finishError, so the reporter
never left Syncing.

- Terminalize on every abort: mid-flow early returns now go through failStep().
- Per-notebook failures are non-critical: one bad notebook no longer aborts (and
  freezes) the whole run; healthy notebooks still finalize.
- Write the ERROR row: uploadNotebook/downloadNotebook call markError() on failure
  (preserving the prior sync anchor), so the ERROR badge finally lights up; the next
  successful commit clears it back to SYNCED. (P25)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018YdtTgjpXGQ88y6JJppogu
- Add a Copy Log button next to Clear that copies the full log buffer to the
  clipboard (with a Toast confirmation), so it can be pasted into a bug report.
- Fix the progress bar showing 100% while the last notebook is still transferring:
  beginItem(index, total) fires when an item starts, so count completed = index - 1.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018YdtTgjpXGQ88y6JJppogu
Prerequisite for incremental reconciliation (Phase 5a). Adds two WebDAV helpers:
- putFileReturningEtag: PUT that returns the server's new ETag header.
- getFileIfNoneMatch: conditional GET returning null on 304 Not Modified.

uploadNotebook now stores the manifest's post-PUT ETag; downloadNotebook fetches
the manifest with metadata and stores that ETag. Previously both stored null, so
If-None-Match had nothing to send.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018YdtTgjpXGQ88y6JJppogu
Extract the per-notebook upload/download/skip decision into a pure function with no
I/O (NotebookSyncPlanner.decide), driven by local timestamp, the stored sync-state
anchor/ETag, and the remote manifest facts. Being pure, it is finally unit-testable;
adds table-driven tests covering 304/unchanged, local-edited, remote-newer,
both-edited LWW, upload-only skip, and missing-timestamp branches.

Not yet wired into the reconciliation service (next commit).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018YdtTgjpXGQ88y6JJppogu
…Phase 5a/5c)

- One PROPFIND of /notebooks/ is fetched once by the orchestrator and shared by
  reconciliation (existence) and new-notebook discovery, replacing the per-notebook
  HEAD probe (P13).
- Reconciliation drives the pure NotebookSyncPlanner.decide and fetches the manifest
  conditionally (If-None-Match) when a stored ETag exists, so an unchanged notebook
  returns a bodyless 304 instead of a full manifest GET (5a).
- Removed the per-notebook checkClockSkew (a network HEAD each) and checkWifiConstraint
  from reconciliation; preflight runs once in the orchestrator. The single-notebook
  (sync-on-close) path now does its own clock-skew check (5c).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018YdtTgjpXGQ88y6JJppogu
- Reconciliation returns Success for NotebookAction.SkipUploadOnly (remote newer,
  upload-only) instead of DomainError.SyncUploadOnlySkip; local data and the
  sync-state row are left untouched (6a/6b).
- Removed the errors-as-control-flow plumbing: DomainError.SyncUploadOnlySkip,
  isOnlyUploadSkip(), and the finalizeSyncResult special case (finishes P23). The
  Error channel means failure again.
- Added a help caption under the Upload only toggle explaining it never overwrites a
  newer server copy and never modifies local data; documented it in both docs (6c).
- Updated tests that referenced the removed API.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018YdtTgjpXGQ88y6JJppogu
- After a committed upload, delete remote page/image/background files no longer
  referenced by the manifest (authoritative since we just wrote it).
- After a committed download, delete local pages whose ids left the manifest.
- Adds WebDAVClient.listNames (raw child listing, unlike the UUID-filtered
  listCollection) and PageDao.getPageIdsForNotebook. All GC is best-effort: failures
  are logged, never abort the sync.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018YdtTgjpXGQ88y6JJppogu
Opening a notebook now runs a cheap read-only check (SyncOrchestrator.isRemoteNewer,
a conditional manifest GET using the stored ETag) off the load path. If the server's
copy is newer than local, a snack suggests syncing before editing -- preventing the
stale-open-then-edit case that manufactures last-writer-wins conflicts. Best-effort:
never mutates anything, never holds the sync mutex, returns false on any error.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018YdtTgjpXGQ88y6JJppogu
In upload-only mode, a notebook that is newer on the server was left with a stale
SYNCED badge. Now reconciliation records a REMOTE_AHEAD sync-state row for that case,
and the library shows a distinct cloud-download badge. A later local edit supersedes
it (badge flips to pending-upload), since the anchor is the current local updatedAt.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018YdtTgjpXGQ88y6JJppogu
Closes the last interruption window Phase 3 left open: a torn PUT of the manifest
(the commit marker) would leave a corrupt manifest. publishManifest now PUTs the
bytes to a manifest.json.tmp sibling (full write, never touches the live marker),
then MOVEs it over manifest.json (atomic on most servers). If-Match is carried on
the MOVE via an If header so a concurrent remote change still yields a 412 conflict.
Falls back to a direct guarded PUT when the server doesn't support MOVE (405/501) or
the If header; a 412 is propagated as a real conflict. Adds WebDAVClient.move.

Also documents GC, check-on-open, atomic publish, and the REMOTE_AHEAD badge in the
technical doc.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018YdtTgjpXGQ88y6JJppogu
The 'newer version on server' snack now has a Sync now button. It closes the notebook
to its pages list first, then syncs the notebook -- syncing as the editor closes
mirrors the safe sync-on-close path and avoids downloading into the open editor (which
would clobber the freshly downloaded data with stale in-memory state).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018YdtTgjpXGQ88y6JJppogu
@Ethran Ethran linked an issue Jul 19, 2026 that may be closed by this pull request
Ethran and others added 6 commits July 19, 2026 17:57
…hase 8a-8e)

Fixes the on-device download storm where a 404'd media file wedged a notebook forever
(never commits -> every sync retries -> log storm -> OOM):

- 8a-1: classify remote 404 as DomainError.RemoteMissing (permanent, not transient).
- 8a-2: a 404 on a background/image is non-fatal in downloadPage (log + skip), so the
  notebook still commits; only transient errors keep the timestamp stale for retry.
  Once committed, it is no longer re-attempted every sync -> stops the storm/OOM.
- 8a-3: fetch each distinct background once per notebook (a PDF shared by N pages was
  fetched/404'd N times).
- 8b: fix the background path mismatch — download now fetches by basename (flat),
  matching how upload stores it, so managed PDFs/covers stop 404-ing.
- 8c: normalize file:// image URIs before File() on upload (they were silently
  skipped, leaving dangling references that 404 on other devices).
- 8d: skip linked external PDFs (absolute path outside managed storage) on both sides.
- 8e: warn distinctly when a referenced image/background is missing locally at upload.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018YdtTgjpXGQ88y6JJppogu
- Don't swallow CancellationException in syncAllNotebooks: reset the reporter to Idle
  and rethrow, so a cancelled sync no longer wedges the reporter in Error and the
  Sync-now button stays usable (8h-1). Always offer Retry from an Error state.
- Manual sync shows 'A sync is already running' instead of 'Sync failed: sync in
  progress' for SyncInProgress (8i-1).
- The experimental warning dialog is shown only when sync is disabled (8i-2).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018YdtTgjpXGQ88y6JJppogu
- Add SyncSettings.syncOnAppStart / checkOnOpen and gate app-start sync
  (MainActivity) and check-on-open (isRemoteNewer) on them, so each automatic
  trigger has its own switch.
- Add a download-only mode (mirror of upload-only): NotebookSyncPlanner suppresses
  UPLOAD actions (SkipDownloadOnly, leaves the NOT_SYNCED badge honest), and the
  orchestrator skips pushing folders and local deletions. Upload-only and
  download-only are mutually exclusive in the UI.
- New toggles in the Sync behavior section.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018YdtTgjpXGQ88y6JJppogu
- 8j: the check-on-open 'Sync now' now shows a 'Syncing notebook…' -> synced/failed
  snack instead of being silent.
- 8h-2: a 'Cancel sync' button appears while a sync is running; it cancels the manual
  sync job and any WorkManager sync, and resets the reporter (SyncScheduler
  .cancelRunningSync + SettingsViewModel.onCancelSync).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018YdtTgjpXGQ88y6JJppogu
… 9c)

Finishes the P21/P24/P27 trigger-funnel tail:
- Manual 'Sync Now' and force upload/download now go through
  SyncScheduler.triggerImmediateSync (WorkManager) instead of calling the orchestrator
  directly, so every trigger shares constraints/retry and is visible to SyncWorkUiBridge.
- Removed the manual runSyncWithSnack path and its own snack: terminal feedback now
  comes from one place (SyncWorkUiBridge -> SnackDispatcher in MainActivity), per
  docs/snacks.md (domain builds no SnackConf). Live progress still shows in the settings
  panel via SyncProgressReporter.
- Dropped the manual lastSyncTime write (the orchestrator persists it on success, P27).
- SyncWorkUiBridge treats SyncInProgress as informational ('A sync is already running'),
  not a failure (8i-1), so a manual sync landing during a periodic run no longer shows
  'sync failed: sync in progress'.
- onCancelSync now just cancels WorkManager sync + resets the reporter.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018YdtTgjpXGQ88y6JJppogu
@Ethran
Ethran merged commit 8b1fc42 into main Jul 19, 2026
3 of 4 checks passed
@Ethran
Ethran deleted the webdav branch July 19, 2026 17:03
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.

Add option to sync notes

1 participant