max / synckit
- Co-Authored-By
- Claude Opus 5 (1M context) <noreply@anthropic.com>
- Claude-Session
- https://claude.ai/code/session_01MptwXZ8k65v19rFmdGAyki
16 files changed,
+122 insertions,
-169 deletions
| @@ -61,8 +61,8 @@ | |||
| 61 | 61 | ||
| 62 | 62 | The Apple targets are checked, not built, by `scripts/check-mobile-targets.sh`. | |
| 63 | 63 | Run it after touching `keystore.rs`, the `keychain` feature, or any | |
| 64 | - | keyring-family dependency: synckit-client was once entirely unbuildable for iOS | |
| 65 | - | while compiling cleanly on every host we build on. | |
| 64 | + | keyring-family dependency. A target can be broken there while every host we | |
| 65 | + | build on compiles cleanly. | |
| 66 | 66 | ||
| 67 | 67 | ## Running the suites | |
| 68 | 68 | ||
| @@ -108,9 +108,8 @@ | |||
| 108 | 108 | - **Unit tests go at the bottom of the file they test**, in one | |
| 109 | 109 | `#[cfg(test)] mod tests`. A unit test that reaches for a database or a mock | |
| 110 | 110 | server is an integration test in the wrong file. | |
| 111 | - | - **No file over ~800 lines.** The integration suite was one 4,201-line file | |
| 112 | - | until it became `tests/integration/`; that is the failure the rule exists to | |
| 113 | - | prevent. | |
| 111 | + | - **No file over ~800 lines.** Split a suite that outgrows it into a directory | |
| 112 | + | of per-domain modules. | |
| 114 | 113 | - **A fixture earns a place in `common.rs` when a second module wants it.** One | |
| 115 | 114 | caller, one module: leave it where it is. | |
| 116 | 115 | - **Every test owns its world.** A fresh `MockServer` per test, a fresh | |
| @@ -133,8 +132,7 @@ | |||
| 133 | 132 | - **Metamorphic relations.** Relate two runs instead of judging one, which needs | |
| 134 | 133 | no table of expected values. Pagination must not change what a pull returns; a | |
| 135 | 134 | rotation must not change the plaintext a pull yields. Both live in the | |
| 136 | - | integration suite and both were written because the pre-existing tests counted | |
| 137 | - | requests without ever looking at the content. | |
| 135 | + | integration suite. Assert on content, not on request counts. | |
| 138 | 136 | ||
| 139 | 137 | ## Writing | |
| 140 | 138 |
| @@ -2,9 +2,8 @@ | |||
| 2 | 2 | ||
| 3 | 3 | End-to-end-encrypted sync: the client SDK and the local config store. | |
| 4 | 4 | ||
| 5 | - | Extracted from the MNW monorepo (2026-07-24) so synckit can version and deploy | |
| 6 | - | independently. Two standalone crates, no root workspace (each builds on its own, | |
| 7 | - | matching the ecosystem convention): | |
| 5 | + | Two standalone crates, no root workspace; each builds on its own, matching the | |
| 6 | + | ecosystem convention. | |
| 8 | 7 | ||
| 9 | 8 | - **`synckit-client`**, the sync SDK: transport, crypto, the declarative | |
| 10 | 9 | `SyncStore` engine over SQLite. Internal; consumed by path or git dependency, | |
| @@ -28,11 +27,6 @@ | |||
| 28 | 27 | purposes freely, but selling hosting on it needs a different arrangement. Pairing | |
| 29 | 28 | this SDK with a server you wrote yourself is outside that perimeter and fine. | |
| 30 | 29 | ||
| 31 | - | The synckit **server** still lives in the MNW server. Splitting it into a | |
| 32 | - | standalone service on its own VPS is a separate, later effort. See the wiki | |
| 33 | - | note `mnw-strategy-synckit-vps-separation`, triggered by the first external | |
| 34 | - | developer. | |
| 35 | - | ||
| 36 | 30 | ## Contributing | |
| 37 | 31 | ||
| 38 | 32 | This project is licensed under MIT. Make Creative, LLC owns the codebase and needs |
| @@ -7,8 +7,6 @@ | |||
| 7 | 7 | (GoingsOn, Balanced Breakfast, audiofiles) use this crate to push and pull | |
| 8 | 8 | changelog entries without the server ever seeing plaintext data. | |
| 9 | 9 | ||
| 10 | - | Version: 0.5.0. | |
| 11 | - | ||
| 12 | 10 | ## Crate structure | |
| 13 | 11 | ||
| 14 | 12 | ``` | |
| @@ -135,10 +133,9 @@ | |||
| 135 | 133 | ||
| 136 | 134 | `__sksv` is the sender's `SyncSchema` storage version, present only when the app | |
| 137 | 135 | declared one (see "The storage-version gate"). It rides inside the sealed | |
| 138 | - | envelope so the server never learns which schema an account is on, and it was | |
| 139 | - | added *within* v2 rather than as a new `__skver`: a reader of this format | |
| 140 | - | addresses `__skhlc` and `data` by name and ignores anything else, so an unstamped | |
| 141 | - | build reads a stamped envelope exactly as it always did. | |
| 136 | + | envelope so the server never learns which schema an account is on. A reader of | |
| 137 | + | this format addresses `__skhlc` and `data` by name and ignores anything else, so | |
| 138 | + | an unstamped build reads a stamped envelope without error. | |
| 142 | 139 | ||
| 143 | 140 | Encrypting the envelope (rather than just the row) is what carries the hybrid | |
| 144 | 141 | logical clock (HLC) inside the E2E ciphertext, so the server can order entries by | |
| @@ -399,35 +396,29 @@ | |||
| 399 | 396 | ||
| 400 | 397 | # The `SyncStore` higher-level helper | |
| 401 | 398 | ||
| 402 | - | > Status: shipped in 0.6 behind the default-on `store` feature (`src/store/`). | |
| 403 | - | > Everything above describes the base SDK (transport + crypto + HLC + conflict | |
| 404 | - | > primitives); this section describes the layer *above* those primitives that | |
| 405 | - | > absorbs the SQLite plumbing the three first-party apps otherwise hand-write. | |
| 406 | - | > The design was reverse-engineered from the GoingsOn, audiofiles, and Balanced | |
| 407 | - | > Breakfast sync services; a consumer that only needs the transport/crypto SDK | |
| 408 | - | > (e.g. mnw-cli) sets `default-features = false` and skips it. | |
| 399 | + | > Lives behind the default-on `store` feature (`src/store/`). Everything above | |
| 400 | + | > describes the base SDK (transport + crypto + HLC + conflict primitives); this | |
| 401 | + | > section describes the layer *above* those primitives that absorbs the SQLite | |
| 402 | + | > plumbing a consuming app would otherwise hand-write. A consumer that only needs | |
| 403 | + | > the transport/crypto SDK (e.g. mnw-cli) sets `default-features = false` and | |
| 404 | + | > skips it. | |
| 409 | 405 | ||
| 410 | - | ## Motivation | |
| 406 | + | ## What it covers | |
| 411 | 407 | ||
| 412 | 408 | Without this layer the SDK stops at "encrypt/decrypt a `Vec<ChangeEntry>` and | |
| 413 | - | move it over the wire," and each consuming app writes ~700–1,000 lines of | |
| 414 | - | *identical* engine on top of it: a `sync_changelog` + `sync_state` schema, per-table triggers, a | |
| 415 | - | drain-loop push, a cursor-loop pull, an FK-ordered apply with JSON→SQL binding, | |
| 416 | - | HLC stamping, an initial snapshot, retention/cleanup, and a scheduler | |
| 417 | - | (tick + SSE + backoff + status events). Measured across the three apps this is | |
| 418 | - | the overwhelming majority of their sync code, and it is mechanical. | |
| 409 | + | move it over the wire," and each consuming app writes the same engine on top of | |
| 410 | + | it: a `sync_changelog` + `sync_state` schema, per-table triggers, a drain-loop | |
| 411 | + | push, a cursor-loop pull, an FK-ordered apply with JSON→SQL binding, HLC | |
| 412 | + | stamping, an initial snapshot, retention/cleanup, and a scheduler (tick + SSE + | |
| 413 | + | backoff + status events). | |
| 419 | 414 | ||
| 420 | - | The single largest boilerplate source is **column-list triplication**: every | |
| 421 | - | syncable table names its synced columns three or four times, in the trigger DDL | |
| 422 | - | (`json_object(...)`), in the initial-snapshot projection, and in the apply-side | |
| 423 | - | whitelist, kept in agreement only by hand and a round-trip drift test. When they | |
| 424 | - | drift, a column silently arrives NULL (this is a real shipped bug class). One | |
| 425 | - | declared column list should generate all of them. | |
| 415 | + | The largest boilerplate source is **column-list triplication**: a syncable table | |
| 416 | + | names its synced columns in the trigger DDL (`json_object(...)`), in the | |
| 417 | + | initial-snapshot projection, and in the apply-side whitelist. When those drift, a | |
| 418 | + | column silently arrives NULL. One declared column list generates all of them. | |
| 426 | 419 | ||
| 427 | - | The goal of `SyncStore` is: an app **declares its tables and policy once**, and | |
| 428 | - | the engine owns everything mechanical. This is the "simple" pillar of the product | |
| 429 | - | (see the private roadmap), the differentiators (private, secure, economical) are | |
| 430 | - | already in the 0.6 primitives; this layer is what makes them easy to adopt. | |
| 420 | + | An app **declares its tables and policy once**, and the engine owns everything | |
| 421 | + | mechanical. | |
| 431 | 422 | ||
| 432 | 423 | ## What is generic (absorbed) vs. what is policy (declared) | |
| 433 | 424 | ||
| @@ -625,8 +616,7 @@ | |||
| 625 | 616 | **This device against its own store.** `sync_state.storage_version` records the | |
| 626 | 617 | version the store was last shaped by. `SyncStore::sync_now` checks it before it | |
| 627 | 618 | registers a device or reads a row, so a refusal has touched nothing. A store that | |
| 628 | - | has never been stamped adopts the declared version instead of refusing: it was | |
| 629 | - | written before the gate existed, not by a version we disagree with. After its own | |
| 619 | + | has never been stamped adopts the declared version instead of refusing. After its own | |
| 630 | 620 | local migration an app calls `SyncStore::stamp_storage_version()`, which is the | |
| 631 | 621 | claim that the store now matches the manifest. | |
| 632 | 622 | ||
| @@ -650,9 +640,8 @@ | |||
| 650 | 640 | ||
| 651 | 641 | ### Adoption | |
| 652 | 642 | ||
| 653 | - | The gate is off until a manifest declares a version, which is what every app | |
| 654 | - | predating it keeps. An app that declares none pushes byte-identical envelopes and | |
| 655 | - | its stores are never stamped. Per-collection version negotiation and a | |
| 643 | + | The gate is off until a manifest declares a version. An app that declares none | |
| 644 | + | pushes byte-identical envelopes and its stores are never stamped. Per-collection version negotiation and a | |
| 656 | 645 | compatibility floor are both deliberately not built: each puts protocol work in | |
| 657 | 646 | front of every feature that adds a record type. | |
| 658 | 647 | ||
| @@ -816,13 +805,13 @@ | |||
| 816 | 805 | ||
| 817 | 806 | | App | Today | After `SyncStore` | | |
| 818 | 807 | |---|---|---| | |
| 819 | - | | **GoingsOn** | ~1,400 non-test LOC across push/pull/apply/hlc/state/blob_sync/scheduler + trigger DDL + drift test | A 20-table `SyncSchema`, `email_accounts` `preserve_local`+`insert_defaults`, `references_unsynced` on `tasks`+`attachments`, a thin attachment `BlobPolicy`, a Tauri `SyncObserver` (see worked example below) | | |
| 820 | - | | **audiofiles** | ~1,400 non-test LOC + the whole blob subsystem + tombstone/hash-id/config-exclusion logic | A 16-table `SyncSchema` (per-table `Hashed` row-ids, `Tombstone` on samples, `exclude_where` on user_config, composite/all-PK tables), the full 5-method `BlobPolicy` impl (kept, genuine policy), an `Arc<Mutex>` `SyncObserver` (see worked example below) | | |
| 821 | - | | **Balanced Breakfast** | ~700 non-test LOC + triplicated columns | A 7-table `SyncSchema` with `feed_items = PartialUpdate{set:[is_read,is_starred]} + DeleteMode::Ignore`, `MAX_CHANGELOG_ENTRIES` in `SyncConfig`, a Tauri `SyncObserver`. Adopts `HybridLogicalClock` | | |
| 808 | + | | **GoingsOn** | push/pull/apply/hlc/state/blob_sync/scheduler + trigger DDL + drift test | A 20-table `SyncSchema`, `email_accounts` `preserve_local`+`insert_defaults`, `references_unsynced` on `tasks`+`attachments`, a thin attachment `BlobPolicy`, a Tauri `SyncObserver` (see worked example below) | | |
| 809 | + | | **audiofiles** | its own engine + the whole blob subsystem + tombstone/hash-id/config-exclusion logic | A 16-table `SyncSchema` (per-table `Hashed` row-ids, `Tombstone` on samples, `exclude_where` on user_config, composite/all-PK tables), the full 5-method `BlobPolicy` impl (kept, genuine policy), an `Arc<Mutex>` `SyncObserver` (see worked example below) | | |
| 810 | + | | **Balanced Breakfast** | its own engine + triplicated columns | A 7-table `SyncSchema` with `feed_items = PartialUpdate{set:[is_read,is_starred]} + DeleteMode::Ignore`, `MAX_CHANGELOG_ENTRIES` in `SyncConfig`, a Tauri `SyncObserver`. Adopts `HybridLogicalClock` | | |
| 822 | 811 | ||
| 823 | 812 | ## Design decisions | |
| 824 | 813 | ||
| 825 | - | Decided (2026-07-12): | |
| 814 | + | Settled: | |
| 826 | 815 | ||
| 827 | 816 | 1. **DB ownership: the engine owns a private `rusqlite` connection to the app's | |
| 828 | 817 | WAL file.** Not a `SyncDb` trait over the app's `sqlx` pool. This is far | |
| @@ -951,26 +940,23 @@ | |||
| 951 | 940 | } | |
| 952 | 941 | ``` | |
| 953 | 942 | ||
| 954 | - | ### What GO proved about the design | |
| 943 | + | ### How GO maps onto the design | |
| 955 | 944 | ||
| 956 | - | - **Confirmed: credential exclusion collapses.** The single most hand-rolled piece of | |
| 957 | - | GO's apply layer (`apply_email_account_upsert`, its own INSERT-with-`password=''` | |
| 958 | - | and 16-column ON CONFLICT) is fully expressed by `preserve_local` + | |
| 959 | - | `insert_defaults`. No per-table override function survives. This was the strongest | |
| 960 | - | signal the enum set is at the right altitude. | |
| 961 | - | - **Confirmed: 17 of 20 tables are one line.** Only `email_accounts`, `tasks`, and | |
| 945 | + | - **Credential exclusion collapses.** GO's `apply_email_account_upsert` (its own | |
| 946 | + | INSERT-with-`password=''` and 16-column ON CONFLICT) is fully expressed by | |
| 947 | + | `preserve_local` + `insert_defaults`. No per-table override function is needed. | |
| 948 | + | - **17 of 20 tables are one line.** Only `email_accounts`, `tasks`, and | |
| 962 | 949 | `attachments` need a builder call; the rest are `t(name, cols)`. | |
| 963 | - | - **Refinement 1: `user_scope` is not an engine concept.** GO never filters the | |
| 964 | - | *changelog* by user; the only `WHERE user_id = ?` is inside its blob queries. | |
| 965 | - | So user scoping is policy-local (it lives in `AttachmentBlobs`), and the engine- | |
| 966 | - | level `user_scope` field was removed. A changelog-level tenant filter is deferred | |
| 967 | - | to the future multi-tenant work. | |
| 968 | - | - **Refinement 2: blob presence hooks must default to no-ops.** GO has no | |
| 969 | - | `cloud_only` equivalent, so `reconcile`/`on_uploaded`/`on_downloaded` gained | |
| 970 | - | default empty bodies; only AF overrides them. | |
| 971 | - | - **Corrected: `references_unsynced` covers `attachments` too**, not just `tasks` | |
| 972 | - | (both carry `source_email_id`). The engine's rule "disable FK for the apply txn if | |
| 973 | - | any batched table is `references_unsynced`" matches GO's existing blanket toggle. | |
| 950 | + | - **`user_scope` is not an engine concept.** GO never filters the *changelog* by | |
| 951 | + | user; the only `WHERE user_id = ?` is inside its blob queries. User scoping is | |
| 952 | + | policy-local, living in `AttachmentBlobs`. A changelog-level tenant filter waits | |
| 953 | + | on the multi-tenant work. | |
| 954 | + | - **Blob presence hooks default to no-ops.** GO has no `cloud_only` equivalent, so | |
| 955 | + | `reconcile`/`on_uploaded`/`on_downloaded` have default empty bodies; only AF | |
| 956 | + | overrides them. | |
| 957 | + | - **`references_unsynced` covers `attachments` as well as `tasks`** (both carry | |
| 958 | + | `source_email_id`). The engine disables FK enforcement for the apply txn if any | |
| 959 | + | batched table is `references_unsynced`. | |
| 974 | 960 | ||
| 975 | 961 | ## Worked example: audiofiles | |
| 976 | 962 | ||
| @@ -1074,32 +1060,31 @@ | |||
| 1074 | 1060 | } | |
| 1075 | 1061 | ``` | |
| 1076 | 1062 | ||
| 1077 | - | ### What AF proved about the design | |
| 1063 | + | ### How AF maps onto the design | |
| 1078 | 1064 | ||
| 1079 | - | - **Confirmed: the hard cases express.** `DeleteMode::Tombstone{column}` captures | |
| 1065 | + | - **The hard cases express.** `DeleteMode::Tombstone{column}` captures | |
| 1080 | 1066 | the samples CASCADE-safety rule; `PrimaryKey::Composite` + the engine's | |
| 1081 | 1067 | all-columns-are-PK → `INSERT OR IGNORE` detection handles `tags`/ | |
| 1082 | 1068 | `classifier_layer_rules`; and `samples` stacking `hashed()` + `tombstone()` + | |
| 1083 | 1069 | a non-`id` PK in one declaration validates that the modifiers compose. | |
| 1084 | - | - **Confirmed: the six-method `BlobPolicy` fits.** AF's `sync_files`/`cloud_only`/ | |
| 1070 | + | - **The six-method `BlobPolicy` fits.** AF's `sync_files`/`cloud_only`/ | |
| 1085 | 1071 | `source_path` machinery maps onto the two queries + `local_path` + `reconcile` + | |
| 1086 | 1072 | `on_downloaded`, with `on_uploaded` correctly a no-op. GO uses 3 methods, AF uses | |
| 1087 | 1073 | 5, the trait spans both without a special case. | |
| 1088 | - | - **Refinement (important), `exclude` must be SQL, not a Rust closure.** AF enforces | |
| 1074 | + | - **`exclude` must be SQL, not a Rust closure.** AF enforces | |
| 1089 | 1075 | the config denylist *symmetrically*: on import (apply guard) AND on export (the | |
| 1090 | 1076 | changelog trigger's WHEN clause), the sensitive key must never enter the | |
| 1091 | - | changelog in the first place (fuzz-2026-07-06 #2). A `fn(&Row)->bool` cannot | |
| 1092 | - | compile into a generated SQL trigger, so the hook became `exclude_where: &str`, a | |
| 1093 | - | SQL predicate the engine drops into both the trigger and the apply-time check. | |
| 1094 | - | This is the one place the GO-derived API was actually wrong. | |
| 1095 | - | - **Refinement: `RowIdScheme::Hashed` is per-table and adds engine duties.** Only | |
| 1077 | + | changelog in the first place. A `fn(&Row)->bool` cannot compile into a generated | |
| 1078 | + | SQL trigger, so the hook is `exclude_where: &str`, a SQL predicate the engine | |
| 1079 | + | drops into both the trigger and the apply-time check. | |
| 1080 | + | - **`RowIdScheme::Hashed` is per-table and adds engine duties.** Only | |
| 1096 | 1081 | content-bearing keys are hashed (samples/analysis/features/tags/tag_policy/ | |
| 1097 | 1082 | collection_members); opaque-integer and config tables stay `PrimaryKey` (M018's | |
| 1098 | 1083 | own split). Hashed obligates the engine to own the per-vault `row_id_salt` | |
| 1099 | 1084 | (generate once in sync_state, never sync), register `hash_row_id`, and emit the | |
| 1100 | 1085 | canonical PK into `data` on generated DELETE triggers so pull reconstructs the | |
| 1101 | 1086 | WHERE from the payload. Documented on the enum. | |
| 1102 | - | - **Boundary noted: tombstone reads/sweep stay app-side.** `Tombstone{column}` | |
| 1087 | + | - **Tombstone reads and the sweep stay app-side.** `Tombstone{column}` | |
| 1103 | 1088 | owns only the apply-time write (`SET deleted_at = COALESCE(deleted_at, now())`). | |
| 1104 | 1089 | The `deleted_at IS NULL` read filter on domain queries and the 30-day hard-delete | |
| 1105 | 1090 | sweep are AF domain code, not the sync engine, the engine just stops the remote |
| @@ -53,7 +53,7 @@ | |||
| 53 | 53 | - Email account passwords and OAuth tokens are excluded from the column whitelist, never leave the device | |
| 54 | 54 | - Tasks with `source_email_id` referencing unsynced emails: FK enforcement relaxed during remote apply | |
| 55 | 55 | ||
| 56 | - | **Location:** `src-tauri/src/sync_service.rs` (1814 lines, 43 tests) | |
| 56 | + | **Location:** `src-tauri/src/syncstore/` | |
| 57 | 57 | ||
| 58 | 58 | ### Balanced Breakfast (5 tables) | |
| 59 | 59 | ||
| @@ -69,7 +69,7 @@ | |||
| 69 | 69 | - `feed_items` deletes are ignored, content re-fetches from source feeds | |
| 70 | 70 | - Changelog retention cap: MAX_CHANGELOG_ENTRIES = 10,000 (prevents unbounded growth) | |
| 71 | 71 | ||
| 72 | - | **Location:** `src-tauri/src/sync_service.rs` (1062 lines, 30 tests) | |
| 72 | + | **Location:** `src-tauri/src/sync_service/` | |
| 73 | 73 | ||
| 74 | 74 | ### audiofiles (9 tables) | |
| 75 | 75 | ||
| @@ -88,7 +88,7 @@ | |||
| 88 | 88 | 3. After push/pull, upload pending blobs (local files in sync-enabled VFS) | |
| 89 | 89 | 4. Download missing blobs (cloud_only samples where file is needed) | |
| 90 | 90 | ||
| 91 | - | **Location:** `crates/audiofiles-sync/src/service.rs` (1438 lines, 48 tests) | |
| 91 | + | **Location:** `crates/audiofiles-sync/src/service/` | |
| 92 | 92 | ||
| 93 | 93 | --- | |
| 94 | 94 |
| @@ -353,14 +353,11 @@ | |||
| 353 | 353 | /// is asked. [`Ordering::Greater`] means `a` wins. | |
| 354 | 354 | /// | |
| 355 | 355 | /// Every site that decides a winner routes through here, so there is one rule | |
| 356 | - | /// rather than one per site. It used to be two. The pull pipeline's collapse | |
| 357 | - | /// step kept the first entry it saw at an exact HLC tie, and it converged for a | |
| 358 | - | /// reason nobody had written down: first-seen order is pull order is the | |
| 359 | - | /// server's sequence order, identical on every device. That is a real property | |
| 360 | - | /// of today's server and a load-bearing assumption to leave implicit, so the | |
| 361 | - | /// collapse now breaks the tie the way the conflict resolver always has, on | |
| 362 | - | /// bytes both devices derive independently. The cost is one canonical encoding | |
| 363 | - | /// per same-row collision within a batch, which is the rare case. | |
| 356 | + | /// rather than one per site, the pull pipeline's collapse step included. An | |
| 357 | + | /// exact HLC tie breaks on bytes both devices derive independently rather than | |
| 358 | + | /// on first-seen order, which would rest on the server's sequence order being | |
| 359 | + | /// identical everywhere. The cost is one canonical encoding per same-row | |
| 360 | + | /// collision within a batch, which is the rare case. | |
| 364 | 361 | /// | |
| 365 | 362 | /// Ordering deletes is the reason this takes whole entries rather than payloads. | |
| 366 | 363 | /// A delete carries no payload and canonicalizes to the empty byte string, which | |
| @@ -1344,8 +1341,8 @@ | |||
| 1344 | 1341 | ||
| 1345 | 1342 | #[test] | |
| 1346 | 1343 | fn lww_newer_update_beats_older_delete() { | |
| 1347 | - | // The data-loss fix: a strictly-newer UPDATE must beat an older DELETE. | |
| 1348 | - | // Under the old "delete always wins" rule this edit was silently lost. | |
| 1344 | + | // A strictly-newer UPDATE must beat an older DELETE, or the edit is | |
| 1345 | + | // silently lost. | |
| 1349 | 1346 | let other = Uuid::new_v4(); | |
| 1350 | 1347 | let mut local = make_entry("tasks", "r1", ChangeOp::Update, Utc::now()); | |
| 1351 | 1348 | local.hlc = Hlc { | |
| @@ -1651,9 +1648,8 @@ | |||
| 1651 | 1648 | assert_eq!(conflicts[0].local.timestamp, t2); | |
| 1652 | 1649 | } | |
| 1653 | 1650 | ||
| 1654 | - | // DELETE vs DELETE under HLC: the higher clock wins like any other pair. | |
| 1655 | - | // (Previously local always won and the timestamp was ignored, now a strictly | |
| 1656 | - | // newer remote delete wins.) | |
| 1651 | + | // DELETE vs DELETE under HLC: the higher clock wins like any other pair, so | |
| 1652 | + | // a strictly newer remote delete wins. | |
| 1657 | 1653 | #[test] | |
| 1658 | 1654 | fn lww_both_delete_newer_wins() { | |
| 1659 | 1655 | let other_device = Uuid::new_v4(); | |
| @@ -1669,11 +1665,11 @@ | |||
| 1669 | 1665 | )); | |
| 1670 | 1666 | } | |
| 1671 | 1667 | ||
| 1672 | - | // F1 (regression): overlapping fields whose two sides carry the *same* | |
| 1673 | - | // wall-ms must resolve convergently, not "ties go to local". The old rule | |
| 1674 | - | // broke ties on local, so device A (local=A) kept A while device B (local=B) | |
| 1675 | - | // kept B, permanent silent divergence. Now the exact tie breaks on the full | |
| 1676 | - | // HLC (distinct node), so both devices land on the same physical value. | |
| 1668 | + | // F1: overlapping fields whose two sides carry the *same* wall-ms must | |
| 1669 | + | // resolve convergently, not "ties go to local". Breaking a tie on local | |
| 1670 | + | // would leave device A holding A and device B holding B, a permanent silent | |
| 1671 | + | // divergence, so the exact tie breaks on the full HLC (distinct node) and | |
| 1672 | + | // both devices land on the same physical value. | |
| 1677 | 1673 | #[test] | |
| 1678 | 1674 | fn field_merge_overlapping_equal_wall_converges() { | |
| 1679 | 1675 | let base = json!({"title": "base"}); | |
| @@ -2070,7 +2066,7 @@ | |||
| 2070 | 2066 | let remote = json!({"a": 2}); | |
| 2071 | 2067 | let base = serde_json::Value::Null; | |
| 2072 | 2068 | let local_ts = Utc::now(); | |
| 2073 | - | // Local strictly newer: must be kept, not silently discarded (the old bug). | |
| 2069 | + | // Local strictly newer: must be kept, not silently discarded. | |
| 2074 | 2070 | let remote_older = local_ts - chrono::Duration::seconds(10); | |
| 2075 | 2071 | assert!(matches!( | |
| 2076 | 2072 | resolve_field_merge( |
| @@ -1849,8 +1849,7 @@ | |||
| 1849 | 1849 | let salt = [7u8; 32]; | |
| 1850 | 1850 | // Inflated memory (OOM DoS from a hostile envelope) rejected before allocation. | |
| 1851 | 1851 | assert!(derive_wrapping_key_with_params("pw", &salt, 4_000_000, 3, 1).is_err()); | |
| 1852 | - | // Mobile-OOM fix: 512 MiB, valid under the old 1 GiB ceiling, is now | |
| 1853 | - | // rejected (before any allocation) by the 256 MiB ceiling. | |
| 1852 | + | // 512 MiB is rejected (before any allocation) by the 256 MiB ceiling. | |
| 1854 | 1853 | assert!(derive_wrapping_key_with_params("pw", &salt, 512 * 1024, 3, 1).is_err()); | |
| 1855 | 1854 | // Weakened memory (KDF downgrade) rejected. | |
| 1856 | 1855 | assert!(derive_wrapping_key_with_params("pw", &salt, 8, 3, 1).is_err()); |
| @@ -184,8 +184,8 @@ | |||
| 184 | 184 | /// sanctioned way to obtain a nil-node clock, used for pre-HLC entries that | |
| 185 | 185 | /// deserialize without an embedded clock (they must lose to any real HLC). It is | |
| 186 | 186 | /// wired as the `serde` default for [`ChangeEntry::hlc`]; everywhere else a clock | |
| 187 | - | /// is minted via [`Hlc::tick`]/[`Hlc::observe`], so the old silent `Hlc::default()` | |
| 188 | - | /// nil-node foot-gun no longer compiles. | |
| 187 | + | /// is minted via [`Hlc::tick`]/[`Hlc::observe`], so a silent nil-node | |
| 188 | + | /// `Hlc::default()` is not expressible. | |
| 189 | 189 | pub(crate) fn hlc_legacy_floor() -> Hlc { | |
| 190 | 190 | Hlc::zero(DeviceId::nil()) | |
| 191 | 191 | } |
| @@ -11,10 +11,8 @@ | |||
| 11 | 11 | //! | |
| 12 | 12 | //! ## Why this is its own crate, and why it is SQLite | |
| 13 | 13 | //! | |
| 14 | - | //! Four apps stored settings four ways (a synced SQLite table, an | |
| 15 | - | //! unconditionally-synced one, `localStorage`, a TOML file) and the theme | |
| 16 | - | //! extraction was about to make it five. The unifying substrate is SQLite rows, | |
| 17 | - | //! not a text file, because config syncs **per key**: two devices editing | |
| 14 | + | //! The substrate is SQLite rows, not a text file, because config syncs | |
| 15 | + | //! **per key**: two devices editing | |
| 18 | 16 | //! different keys must both survive (row-level HLC merge, which a whole-file blob | |
| 19 | 17 | //! cannot do), and some keys must never sync at all (see posture). Rows are what | |
| 20 | 18 | //! buy both. A text file would need a projection layer and a sidecar for the | |
| @@ -23,11 +21,9 @@ | |||
| 23 | 21 | //! | |
| 24 | 22 | //! ## Posture, and why it fails closed | |
| 25 | 23 | //! | |
| 26 | - | //! [`Posture`] is the one security-load-bearing type here. It descends from | |
| 27 | - | //! audiofiles' `DeviceLocal`/`Replicated`, which exists because a hostile server | |
| 28 | - | //! steered a local filesystem path across the sync boundary through an | |
| 29 | - | //! unclassified config key (fuzz-2026-07-06 #2, -07-20 #1, -07-21 #3). Two rules | |
| 30 | - | //! carried over intact: | |
| 24 | + | //! [`Posture`] is the one security-load-bearing type here. An unclassified | |
| 25 | + | //! config key is how a hostile server steers a local filesystem path across the | |
| 26 | + | //! sync boundary, so two rules hold: | |
| 31 | 27 | //! | |
| 32 | 28 | //! - The default is [`Posture::Local`]. A key crosses only when a spec declares | |
| 33 | 29 | //! it [`Posture::Synced`]. |
| @@ -450,8 +450,7 @@ | |||
| 450 | 450 | /// entry's `key_id`. Generic over the decrypt step so both the plain-pull | |
| 451 | 451 | /// (`ChangeEntry`) and rich-pull (`PulledChange`) paths share exactly this | |
| 452 | 452 | /// selection logic, including the unknown-`key_id` fallback that tries the | |
| 453 | - | /// primary key then the pending key. Previously the live pull path | |
| 454 | - | /// reimplemented a fallback-less variant while the tested one sat unused. | |
| 453 | + | /// primary key then the pending key. | |
| 455 | 454 | pub(super) fn decrypt_with_rotation_keys<T, F>( | |
| 456 | 455 | entry: PullChangeEntry, | |
| 457 | 456 | primary_key: &[u8; 32], |
| @@ -130,8 +130,7 @@ | |||
| 130 | 130 | /// | |
| 131 | 131 | /// **Nothing here may be load-bearing.** A resume store that errors, or that | |
| 132 | 132 | /// returns a record which turns out not to fit, must only cost a restart from | |
| 133 | - | /// zero, which is what the caller did before this existed. The upload path | |
| 134 | - | /// treats every method as best-effort for that reason. | |
| 133 | + | /// zero. The upload path treats every method as best-effort for that reason. | |
| 135 | 134 | pub trait BlobResumeStore: Send + Sync { | |
| 136 | 135 | /// The record for `hash`, if a session is on file. | |
| 137 | 136 | fn load(&self, hash: &str) -> crate::Result<Option<ResumeRecord>>; |
| @@ -19,10 +19,9 @@ | |||
| 19 | 19 | //! | |
| 20 | 20 | //! An **allowlist**, not a denylist, and that is the security property. A key | |
| 21 | 21 | //! with no policy row does not match, so it never enters the changelog: an | |
| 22 | - | //! unclassified key stays on the device. audiofiles reached the same predicate | |
| 23 | - | //! the hard way: its earlier `key != 'loose_files'` denylist never covered | |
| 24 | - | //! `mirror_path`/`mirror_enabled`, letting a hostile server steer a local write | |
| 25 | - | //! root across the boundary (fuzz-2026-07-21 #3). The policy table is seeded from | |
| 22 | + | //! unclassified key stays on the device. A denylist cannot hold this line, since | |
| 23 | + | //! a key nobody thought to name (`mirror_path`, `mirror_enabled`) lets a hostile | |
| 24 | + | //! server steer a local write root across the boundary. The policy table is seeded from | |
| 26 | 25 | //! the spec, whose default posture is `Local`, so the allowlist is | |
| 27 | 26 | //! [`ConfigSpec`]'s fail-closed rule carried into SQL. | |
| 28 | 27 | ||
| @@ -151,8 +150,7 @@ | |||
| 151 | 150 | } | |
| 152 | 151 | ||
| 153 | 152 | // The allowlist's reason for being: a key nobody classified must not sync, | |
| 154 | - | // even though the table syncs by default. This is the fuzz-hardened rule in | |
| 155 | - | // SQL rather than in Rust. | |
| 153 | + | // even though the table syncs by default. The rule lives in SQL, not in Rust. | |
| 156 | 154 | #[test] | |
| 157 | 155 | fn an_undeclared_key_is_never_captured() { | |
| 158 | 156 | let (conn, store) = wired(); | |
| @@ -241,7 +239,8 @@ | |||
| 241 | 239 | } | |
| 242 | 240 | ||
| 243 | 241 | // The server does not get to write a Local key. `mirror_path` is a filesystem | |
| 244 | - | // root; accepting one from the wire is the write-root steer (fuzz-2026-07-21 #3). | |
| 242 | + | // root, so accepting one from the wire would let the server steer a local | |
| 243 | + | // write root. | |
| 245 | 244 | #[test] | |
| 246 | 245 | fn an_inbound_local_key_is_filtered_not_applied() { | |
| 247 | 246 | let (mut conn, store) = wired(); |
| @@ -19,12 +19,11 @@ | |||
| 19 | 19 | //! | |
| 20 | 20 | //! # It costs no request | |
| 21 | 21 | //! | |
| 22 | - | //! The sync loop has always asked the server which groups to sync, once per | |
| 23 | - | //! cycle, and the answer has always carried the name and the admin. Until | |
| 24 | - | //! 2026-08-24 [`SyncTransport::list_group_directory`](super::sync::SyncTransport::list_group_directory) | |
| 25 | - | //! was `list_group_scopes` and mapped each record down to `(GroupId, i32)`, | |
| 26 | - | //! dropping the rest. Writing it down instead is the whole mechanism: the read | |
| 27 | - | //! already happens, on a schedule, outside any request loop. | |
| 22 | + | //! The sync loop asks the server which groups to sync, once per cycle, and | |
| 23 | + | //! [`SyncTransport::list_group_directory`](super::sync::SyncTransport::list_group_directory) | |
| 24 | + | //! carries the name and the admin flag with each record. Writing that down is | |
| 25 | + | //! the whole mechanism: the read already happens, on a schedule, outside any | |
| 26 | + | //! request loop. | |
| 28 | 27 | //! | |
| 29 | 28 | //! Members and invitations are the parts that are a new request, made only for | |
| 30 | 29 | //! groups this user administers, since the server refuses a non-admin. Worth | |
| @@ -469,8 +468,7 @@ | |||
| 469 | 468 | -- | |
| 470 | 469 | -- A DIRECTORY, not synced state. Every table here is the server's answer written | |
| 471 | 470 | -- down: the sync loop already asks the server which groups to sync on every | |
| 472 | - | -- cycle, and until 2026-08-24 it kept `(id, gck_version)` and dropped the rest | |
| 473 | - | -- of each record on the floor. | |
| 471 | + | -- cycle, and the answer carries the whole record. | |
| 474 | 472 | -- | |
| 475 | 473 | -- WHY IT HAS TO BE LOCAL. A described screen's handler is a synchronous | |
| 476 | 474 | -- function, so a screen that needs the group list cannot fetch it. That is not a | |
| @@ -625,7 +623,7 @@ | |||
| 625 | 623 | ||
| 626 | 624 | /// Whether this device knows the user to be a member of `group_id`. | |
| 627 | 625 | /// | |
| 628 | - | /// The local answer to the question `share_project` used to ask the server. A | |
| 626 | + | /// Answered from the local directory, with no request. A | |
| 629 | 627 | /// scope the sync engine holds no key for routes a whole subtree into a changelog | |
| 630 | 628 | /// that goes nowhere, so the check is worth making; making it against the | |
| 631 | 629 | /// directory means it can be made from a described handler. | |
| @@ -915,7 +913,7 @@ | |||
| 915 | 913 | assert_eq!(listed[1].email, "late@localhost"); | |
| 916 | 914 | } | |
| 917 | 915 | ||
| 918 | - | /// The question `share_project` used to ask the server, answered locally. | |
| 916 | + | /// Membership is answerable from the local directory, with no request. | |
| 919 | 917 | #[test] | |
| 920 | 918 | fn membership_is_answerable_without_a_request() { | |
| 921 | 919 | let mut conn = db(); | |
| @@ -979,9 +977,7 @@ | |||
| 979 | 977 | /// Both routes into the directory run [`DDL`] itself: `migration_sql` | |
| 980 | 978 | /// concatenates it rather than restating it, so an app that builds a | |
| 981 | 979 | /// `SyncStore` and one that only calls `ensure_tables` get the same tables by | |
| 982 | - | /// construction. This holds the concatenation in place; before 2026-08-24 the | |
| 983 | - | /// statements were spelled out in both files and this test was the only thing | |
| 984 | - | /// standing between them and drift. | |
| 980 | + | /// construction. This test holds the concatenation in place. | |
| 985 | 981 | #[test] | |
| 986 | 982 | fn the_standalone_ddl_agrees_with_the_migration() { | |
| 987 | 983 | let conn = Connection::open_in_memory().expect("a database"); |
| @@ -169,12 +169,10 @@ | |||
| 169 | 169 | /// Changes that have been through [`resolve_pull`], and the only thing | |
| 170 | 170 | /// [`apply_remote_changes`](super::apply::apply_remote_changes) accepts. | |
| 171 | 171 | /// | |
| 172 | - | /// The point is what it makes unrepresentable. Applying a batch straight off | |
| 173 | - | /// the wire, skipping conflict detection, the committed-HLC gate and the | |
| 174 | - | /// collapse, used to be a plain function call that compiled; the invariant | |
| 175 | - | /// lived in the order the pipeline's steps happened to be written in, and | |
| 176 | - | /// nothing carried it to the consumers that depend on it. Now the only way to | |
| 177 | - | /// obtain one is to run the pipeline. Same technique as | |
| 172 | + | /// The point is what it makes unrepresentable. The only way to obtain one is to | |
| 173 | + | /// run the pipeline, so applying a batch straight off the wire, skipping | |
| 174 | + | /// conflict detection, the committed-HLC gate and the collapse, does not | |
| 175 | + | /// compile. Same technique as | |
| 178 | 176 | /// [`CleanChanges`](crate::conflict::CleanChanges) one layer up, and as MNW's | |
| 179 | 177 | /// `S3DeleteAuthority`. | |
| 180 | 178 | /// | |
| @@ -925,9 +923,9 @@ | |||
| 925 | 923 | /// Aimed at [`resolve_pull`] with a real `Connection`, deliberately, and not | |
| 926 | 924 | /// at the pure conflict layer one step down. The one-entry-per-row invariant | |
| 927 | 925 | /// only exists after the collapse, so `resolve_pull` is the lowest layer | |
| 928 | - | /// where a max-HLC-wins specification is an honest thing to assert; an | |
| 929 | - | /// earlier attempt at this aimed at `CleanChanges::gated_at`, which only ever | |
| 930 | - | /// promised committed-clock filtering, and failed three of four properties | |
| 926 | + | /// where a max-HLC-wins specification is an honest thing to assert. | |
| 927 | + | /// `CleanChanges::gated_at`, one layer down, promises only committed-clock | |
| 928 | + | /// filtering, so three of these four properties do not hold there even | |
| 931 | 929 | /// against correct code. See wiki `testing-posture`, Phase 3. | |
| 932 | 930 | /// | |
| 933 | 931 | /// The four properties are the ones a sync engine lives or dies on, and each |