fix: do not republish files removed remotely while this client was offline - #120
Draft
petergaultney wants to merge 4 commits into
Draft
fix: do not republish files removed remotely while this client was offline#120petergaultney wants to merge 4 commits into
petergaultney wants to merge 4 commits into
Conversation
A stalled or hibernated session can miss file events while remote state advances. Provider reconnects apply the downloaded head before the handshake, and hibernation wakes can deliver a buffered update without any provider lifecycle event, so event-only arming can leave both idle writes and editor dispatches using a stale disk record. Require verified disk contents where reconnect data is consumed. Production reads the current provider synced level through a dynamic document callback, teardown disconnects while the integration still listens, and hibernation invalidates its retained record before detaching. Pre-gap disk bytes are discarded when a later gap requires a fresh read. Open notes use a bounded verification merge before editor dispatch. Returning clients seed the structured frontmatter map only after that verified merge; unresolved conflicts stay unseeded. Verification preserves saves, does not restart for keystrokes, and falls back without overwriting if the file cannot be read. The behavior is intentionally unconditional: it runs only when reconnect data is consumed, its cost is bounded to one read, and its failure paths are conservative, so a kill switch would weaken the data-loss invariant more than it would reduce operational risk. Tests cover the production factory sensor, provider teardown ordering, pre-gap disk observations, real provider ordering, hibernation wake, active-editor races, post-verification frontmatter seeding, conflict non-seeding, and two-machine reconnect convergence.
The merge kernel computed token LCS with the Hunt-McIlroy candidate algorithm, whose cost scales with the number of matching token pairs. Newline-split tokenization makes every newline its own token, so two versions of an N-line document share at least N^2 newline pairs and every merge of a large document went super-linear regardless of edit size: ~6s of CPU for a two-line merge at 50KB, ~40s at 100KB, minutes at 300KB - long enough to freeze the UI when a large synced document was rewritten wholesale. The kernel also crashed outright on documents containing a line that collides with an Object.prototype member (its equivalence classes live in a plain object), e.g. a lone constructor line. Three changes bound the cost: - One-sided merges short-circuit: when either side is byte-identical to the base, the result is exactly the other side; no diff runs. This is the common shape of a synced document being rewritten remotely while the local copy sits unedited. - Documents above a token limit merge through an anchor kernel: trim the common affixes, chain tokens unique to both sides, diff small gaps exactly, and treat large ambiguous gaps as whole replacements. At or below the limit the exact kernel runs unchanged, so small documents keep byte-identical behavior; the anchor kernel also serves as the fallback for the exact kernel prototype-collision failure. Coarser hunks preserve one-sided edits exactly. In ambiguous regions, either kernel may conflict where the other auto-merges. - Three-way merge editor dispatches skip the character-level diff when more than 64KB changed after affix trimming: diff-match-patch already returns a coarse cut-off result there, at ~1s of main-thread cost; the single spanning replacement produces the same text instantly. Merges measure 30-41s of CPU at 100KB before and under 8ms after; a 300KB wholesale rewrite drops from minutes to under 26ms, and 600KB stays under 30ms. The merge-region combination logic is ported from node-diff3 (MIT) so region shapes and false-conflict suppression match the exact kernel; region joins use index loops because array spread throws past ~64k tokens.
Adds v=<GIT_TAG> to the doc websocket URL on both the initial connect and the token-refresh path. Servers that don't know the param ignore it (y-sweet's HandlerParams tolerates unknown query fields); infrastructure in front of the server (e.g. nginx ingress) logs it, giving per-connection version observability with no server change.
…fline A returning client's local-file scan (addLocalDocs) mints fresh identities for unmapped disk files. When the unmapped path is one whose folder-map entry was tombstoned while this client was offline, the mint republishes the file under a new identity instead of letting cleanupExtraLocalFiles trash it - and the resulting pending-upload hold shields the path from that cleanup indefinitely. Build an offline-removal witness at session start: the folder doc's persisted pre-session state (captured from the raw IDB updates at initial fetch, so it is unaffected by whether the provider merged first) plus the paths of persisted per-document merge records (which survive a lost or reset folder database). At first sync convergence, witness paths absent from the converged map are excluded from scan minting, and any holds already minted for them are routed to the existing remote-deleted-hold discard. Vault-event-driven creates are untouched; local-only and authoritative folders (latch disabled) are untouched; a folder that never connects this session keeps today's behavior (holds park until convergence). Fixes No-Instructions#119
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
2 participants
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.
Fixes #119.
A client that was offline while files were moved or deleted could republish them at their old paths on its next connect: the local-file scan (
addLocalDocs->placeHold) mints create-intent for any unmapped disk file, and it runs atwhenSynced(), which awaits only local persistence - so an unmapped file is ambiguous between "created offline" (upload) and "removed remotely while offline" (trash locally), and the scan always chose upload.This PR gives the client a witness for "this path used to be shared here":
IndexeddbPersistencecapturesinitialStoredState- a merge of the raw update rows read by the initial IDB fetch, i.e. the pre-session persisted state, taken from storage bytes before any live-doc merging. Valid regardless of whether the provider synced before or after persistence load.SyncStore.readCommittedPaths(doc)reads the committed path keys out of such a state snapshot.MergeManager.getPersistedStatePaths()supplies a second witness from the persisted per-file merge-state records, which survives a lost or reset folder database.SharedFoldercombines the witnesses at startup (captureOfflineRemovalWitness). At first converged sync, witness paths with no committed meta in the merged map are routed away from publication (routeOfflineRemovedHolds): any pending-upload holds they acquired are discarded through the existing remote-deletion machinery, and the scan-mint skips them - so the existingcleanupExtraLocalFilespass trashes the stale disk files instead. Both race orderings (scan before convergence, convergence before scan) end in deletion, not republish.Untouched: vault-event-driven creates (live user actions), local-only folders, and sessions that never connect (holds park exactly as today). A machine with a genuinely empty local database has no witness and behaves as before - acknowledged residual, noted in #119.
Field evidence and validation:
tsc -noEmitclean; production esbuild clean. A standalone test coversreadCommittedPaths(both map generations, tombstoned-path exclusion, empty doc). I could not add a SharedFolder-level integration test because__tests__/**is git-crypt-encrypted and I lack the key - happy to describe the repro so one can be added, or adjust the approach to whatever witness source you'd prefer.