From ab961f13cebba4966a9192466eadae39a973fd7a Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sat, 25 Jul 2026 20:14:55 -0400 Subject: [PATCH 01/86] desktop: cross-compiled Windows NSIS installer lane (task 2015) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a `windows` job to desktop.yml on the new ci-tauri-win image, producing a Windows -setup.exe without any Windows hardware. A Windows container can't run on a Linux host, so cross-compilation is the only route: --runner cargo-xwin supplies the MSVC CRT/SDK (pre-warmed into the image) and links with lld-link, and makensis builds the installer. NSIS only. .msi needs WiX v3, a Windows program — per Tauri, ".msi installers can only be created on Windows". It comes back if a Windows node ever exists. Kept as a separate job so a Windows-side failure can never block the Linux artifacts, which are the primary product today. publish-release.sh now globs the windows target root too; nullglob means each job uploads only what its own workspace contains, and the release is created once and reused via the 409 path, so both jobs can publish to the same release safely. No app code changes were needed. The AppImage self-integration UI already gates on is_appimage (AccountView.vue:131, DesktopIntegrationPrompt.vue:22), and $APPIMAGE is never set on Windows, so the OOBE prompt and Settings toggle hide themselves. Recorded plainly in ci-requirements.md that this is the weakest-verified lane we have: Tauri calls Linux->Windows cross-compilation "not tested as much" and a last resort, and a Linux runner cannot execute a Windows binary. Green means it built. A real Windows machine check is mandatory before trusting a release, and installers are unsigned until a certificate exists. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01SreJkbxB4gx8pPsu8QbLPi --- .forgejo/workflows/desktop.yml | 57 ++++++++++++++++++++++++++++ ci-requirements.md | 24 ++++++++++++ desktop/packaging/publish-release.sh | 8 ++++ 3 files changed, 89 insertions(+) diff --git a/.forgejo/workflows/desktop.yml b/.forgejo/workflows/desktop.yml index e5ff048..25a53ac 100644 --- a/.forgejo/workflows/desktop.yml +++ b/.forgejo/workflows/desktop.yml @@ -126,3 +126,60 @@ jobs: env: GITHUB_TOKEN: ${{ github.token }} run: bash desktop/packaging/publish-release.sh + + # Windows installer, CROSS-COMPILED from Linux — there is no Windows build host. + # A Windows container can't run on a Linux host (containers share the host + # kernel), so cross-compiling is the only route without Windows hardware: + # cargo-xwin + LLVM's lld-link + makensis are Linux programs that emit Windows + # PE output. That toolchain is why this needs its own image rather than ci-tauri. + # + # NSIS only. `.msi` needs WiX v3, which is a Windows program — Tauri: ".msi + # installers can only be created on Windows". It returns if a Windows node does. + # + # A separate job, so a Windows-side failure never blocks the Linux artifacts that + # are the primary product today. Tauri calls this path "not tested as much" and a + # last resort, and nothing here can LAUNCH a Windows binary — green means it + # built, not that it runs. A real-machine check stays mandatory before trusting it. + windows: + name: Windows installer (cross-compiled) + if: github.ref == 'refs/heads/dev' || github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/tags/v') + runs-on: python-ci + container: + image: git.fabledsword.com/bvandeusen/ci-tauri-win:1.97 + steps: + - uses: actions/checkout@v6 + + # Same reason as the Linux job: generate_context! embeds the built frontend + # at compile time, so it must exist before cargo runs. + - name: Build the shared frontend + run: npm ci && npm run build + working-directory: frontend + + # --runner cargo-xwin swaps cargo for the cross-compiling driver (it supplies + # the MSVC CRT/SDK, pre-warmed into the image, and links with lld-link). + # Frontend already built above; skip the beforeBuildCommand rebuild. + - name: Tauri build (NSIS installer) + run: | + cargo tauri build \ + --runner cargo-xwin \ + --target x86_64-pc-windows-msvc \ + --bundles nsis \ + --config '{"build":{"beforeBuildCommand":""}}' + working-directory: desktop/src-tauri + + - name: Upload installer + continue-on-error: true + uses: actions/upload-artifact@v3 + with: + name: thoughtsync-windows + path: desktop/src-tauri/target/x86_64-pc-windows-msvc/release/bundle/nsis/*.exe + if-no-files-found: warn + + # Publishes to the SAME release as the Linux job. Safe to run twice: the + # script reuses an existing release (409) and nullglob means each job uploads + # only the bundles present in its own workspace. + - name: Publish release + if: startsWith(github.ref, 'refs/tags/v') + env: + GITHUB_TOKEN: ${{ github.token }} + run: bash desktop/packaging/publish-release.sh diff --git a/ci-requirements.md b/ci-requirements.md index 081daa1..d75e9bd 100644 --- a/ci-requirements.md +++ b/ci-requirements.md @@ -82,5 +82,29 @@ backend/frontend push. `pacman -U`-tested here. That step logs `.PKGINFO` + the full file listing so the package is auditable from the run log; a real Arch install is the operator's confirm. + +### Windows lane — second job, second image + +`desktop.yml` also runs a `windows` job that cross-compiles the NSIS installer. + +- **Image:** `git.fabledsword.com/bvandeusen/ci-tauri-win:1.97` (Rust + Node + + `cargo-xwin` + LLVM/`lld` + NSIS). A separate image from `ci-tauri` per + CI-Runner's `docs/process.md` fork rule — the MSVC CRT/SDK cache alone is >1 GB. + Its pins are held in lockstep with `ci-tauri`; bump them together, since both + lanes compile the same source. +- **Why cross-compile:** there is no Windows build host, and a Windows container + cannot run on a Linux host (containers share the host kernel). `cargo-xwin`, + `lld-link` and `makensis` are Linux programs that emit Windows PE output. +- **NSIS only.** `.msi` requires WiX v3, a Windows program — per Tauri, "`.msi` + installers can only be created on Windows." +- **Separate job on purpose:** a Windows failure must not block the Linux + artifacts, which are the primary product today. +- **Weakest verification of any lane.** Tauri documents this path as "not as + straight forward as compiling on Windows directly and is not tested as much", + to be used "only as a last resort" — and a Linux runner cannot execute a + Windows binary. Green means it *built*. A real Windows machine check is + mandatory before trusting a release. +- **Unsigned.** Installers will trip SmartScreen until a code-signing + certificate exists; that is a purchasing decision, not a CI one. - No Postgres lane (unchanged): the desktop app's local store + sync behavior is verified on the operator's machine, not in CI. diff --git a/desktop/packaging/publish-release.sh b/desktop/packaging/publish-release.sh index 32a1c7d..0e79bae 100755 --- a/desktop/packaging/publish-release.sh +++ b/desktop/packaging/publish-release.sh @@ -17,6 +17,7 @@ # desktop/src-tauri/target/release/bundle/appimage/*.AppImage (de-bundled) # desktop/src-tauri/target/release/bundle/deb/*.deb # desktop/src-tauri/target/release/bundle/arch/*.pkg.tar.* (prebuilt pacman) +# desktop/src-tauri/target/x86_64-pc-windows-msvc/release/bundle/nsis/*.exe # # Instance-agnostic: server + repo come from the runner's github.* context # (Forgejo populates them for compatibility), so nothing is hardcoded to one host. @@ -37,13 +38,20 @@ AUTH=(-H "Authorization: token $GITHUB_TOKEN") SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" REPO_ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)" BUNDLE_ROOT="$REPO_ROOT/desktop/src-tauri/target/release/bundle" +# Cross-compiled Windows output lands under the target triple, not the host root. +WIN_BUNDLE_ROOT="$REPO_ROOT/desktop/src-tauri/target/x86_64-pc-windows-msvc/release/bundle" # --- collect the assets to upload ------------------------------------------- shopt -s nullglob +# nullglob (set above) drops the patterns that didn't match, which is what lets the +# Linux job and the Windows job each run this script against the SAME release and +# upload only what they actually built — they run in separate workspaces, so neither +# can see the other's bundles. The release is created once and reused (409 path). ASSETS=( "$BUNDLE_ROOT"/appimage/*.AppImage "$BUNDLE_ROOT"/deb/*.deb "$BUNDLE_ROOT"/arch/*.pkg.tar.* + "$WIN_BUNDLE_ROOT"/nsis/*.exe ) if [ ${#ASSETS[@]} -eq 0 ]; then echo "ERROR: no bundles under $BUNDLE_ROOT — did the tauri build run?" >&2 -- 2.54.0 From 5b471f5dd46129311004226c6451c6700c6abe8b Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sat, 25 Jul 2026 20:37:33 -0400 Subject: [PATCH 02/86] desktop: generate the Windows icon set in the cross-compile job MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit tauri-build needs icons/icon.ico to emit the Windows Resource file, and the repo only carries the PNG set the Linux bundles use — run 2881 failed with "icons/icon.ico not found". Generated in-job from the committed 1024px app-icon.png rather than committing a hand-made .ico, so there stays one icon of record that can't silently drift from the brand art. Scoped to the windows job; the Linux bundles don't need it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01SreJkbxB4gx8pPsu8QbLPi --- .forgejo/workflows/desktop.yml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.forgejo/workflows/desktop.yml b/.forgejo/workflows/desktop.yml index 25a53ac..005e5c3 100644 --- a/.forgejo/workflows/desktop.yml +++ b/.forgejo/workflows/desktop.yml @@ -155,6 +155,15 @@ jobs: run: npm ci && npm run build working-directory: frontend + # tauri-build generates a Windows Resource file and needs `icons/icon.ico`, + # which the repo doesn't carry — only the PNG set the Linux bundles use. + # Generating it from the committed 1024px source keeps one icon of record + # instead of a hand-made .ico that could silently drift from the brand art. + # Linux doesn't need this step, which is why it lives here and not in `build`. + - name: Generate the Windows icon set + run: cargo tauri icon app-icon.png + working-directory: desktop/src-tauri + # --runner cargo-xwin swaps cargo for the cross-compiling driver (it supplies # the MSVC CRT/SDK, pre-warmed into the image, and links with lld-link). # Frontend already built above; skip the beforeBuildCommand rebuild. -- 2.54.0 From fbbe877c460bbdcb1e26a998ca2c557b8e61d213 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sat, 25 Jul 2026 22:40:25 -0400 Subject: [PATCH 03/86] =?UTF-8?q?M10.6:=20client=E2=86=94server=20sync=20p?= =?UTF-8?q?rotocol=20handshake=20(task=201995)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Version the sync WIRE PROTOCOL separately from either program's release version, so a self-hosted server and the desktop app can sit on different releases and still work out whether they can talk. Each side declares two numbers — what it speaks, and the oldest counterpart it accepts. Either side can therefore mark a change breaking without the other shipping in step, which is the whole point: no app↔server lockstep. Server advertises on the existing public /api/config (a client must be able to ask "can I talk to you?" before it holds a device token, or even has an account): sync_protocol_version, min_client_protocol_version, sync_features. sync_features exists because a version number can only say newer/older. An ADDITIVE change earns a capability name instead of a minimum bump, so a newer client meeting an older server drops that one feature and syncs the rest, rather than refusing. Raising a minimum is reserved for genuinely breaking changes — it's the switch that hard-blocks the other side. Client half is pure decision logic (sync/compat.rs), no I/O, so every branch is unit-testable — there's no live-server lane in CI. Three outcomes: ok / degraded{unavailable} / incompatible{reason, client_must_update}. The last names which side can fix it, so the message is actionable. A server that predates the handshake sends no protocol fields at all; that reads as "update the server", deliberately not as a parse error, which would look to the user like they mistyped the URL. normalize_base_url defaults a bare host to https://, never http:// — silently downgrading would put a long-lived device token on the wire in cleartext because someone omitted five characters. Plain HTTP on a trusted LAN stays supported; the user types http:// and thereby chooses it. Transport (the actual fetch) lands next, separately: it needs an HTTP/TLS stack, and that's a real risk to the Windows cross-compile lane, so it gets its own CI run to bisect against rather than riding along with this. No UI here by design — the link/settings surface it feeds is M10.7's, per this task's own sequencing. Policy documented in docs/sync.md. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01SreJkbxB4gx8pPsu8QbLPi --- desktop/src-tauri/src/lib.rs | 3 + desktop/src-tauri/src/sync/compat.rs | 384 +++++++++++++++++++++++++++ desktop/src-tauri/src/sync/mod.rs | 10 + docs/sync.md | 61 +++++ src/thoughtsync/app.py | 6 +- src/thoughtsync/sync.py | 42 +++ tests/test_sync.py | 29 ++ 7 files changed, 534 insertions(+), 1 deletion(-) create mode 100644 desktop/src-tauri/src/sync/compat.rs create mode 100644 desktop/src-tauri/src/sync/mod.rs diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index cfec12a..d15798a 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -8,6 +8,9 @@ mod integration; mod local; +// `pub` (unlike the modules above) because nothing calls into it yet — the sync +// engine that will is M10.7. A private module's unused items read as dead code. +pub mod sync; #[cfg_attr(mobile, tauri::mobile_entry_point)] pub fn run() { diff --git a/desktop/src-tauri/src/sync/compat.rs b/desktop/src-tauri/src/sync/compat.rs new file mode 100644 index 0000000..9092afd --- /dev/null +++ b/desktop/src-tauri/src/sync/compat.rs @@ -0,0 +1,384 @@ +//! Client<->server compatibility handshake (M10.6). +//! +//! The desktop app is local-first: it never *needs* a server. When the user links +//! one, this module decides whether the two can actually talk — before a single +//! note moves. The sync engine (M10.7) consults it on link and on every sync. +//! +//! The contract is two integers per side, versioning the WIRE PROTOCOL separately +//! from either program's release version: +//! +//! | | this client | the server advertises | +//! |---|---|---| +//! | speaks | `CLIENT_PROTOCOL_VERSION` | `sync_protocol_version` | +//! | accepts down to | `MIN_SERVER_PROTOCOL_VERSION` | `min_client_protocol_version` | +//! +//! Each side declaring its own floor is what avoids app<->server lockstep: either +//! side can mark a change breaking without the other needing to ship in step. See +//! `docs/sync.md` for the policy that governs when those numbers move. + +use serde::{Deserialize, Serialize}; + +/// The sync wire protocol this client speaks. +pub const CLIENT_PROTOCOL_VERSION: u32 = 1; + +/// The oldest server protocol this client can drive — the symmetric half of the +/// server's `min_client_protocol_version`. +pub const MIN_SERVER_PROTOCOL_VERSION: u32 = 1; + +/// Capabilities without which syncing is meaningless, so their absence BLOCKS the +/// link rather than degrading it. +pub const REQUIRED_FEATURES: &[&str] = &["notes", "labels"]; + +/// Capabilities whose absence costs a feature but not the link. Listing these +/// explicitly (rather than diffing against whatever the server happens to send) is +/// what lets the UI name exactly what the user will be missing. +pub const OPTIONAL_FEATURES: &[&str] = &["attachments", "tombstones", "revisions"]; + +/// The handshake fields of `GET /api/config`. +/// +/// Every protocol field is optional because a server predating M10.6 simply won't +/// send them. That case has to read as "this server is too old to sync", not as a +/// parse failure — which would look to the user like they mistyped the URL. +#[derive(Debug, Clone, Default, Deserialize, Serialize)] +pub struct ServerInfo { + #[serde(default)] + pub site_name: Option, + /// The server's release version, for display only — never gate on it. + #[serde(default)] + pub version: Option, + #[serde(default)] + pub sync_protocol_version: Option, + #[serde(default)] + pub min_client_protocol_version: Option, + #[serde(default)] + pub sync_features: Vec, +} + +impl ServerInfo { + fn has_feature(&self, name: &str) -> bool { + self.sync_features.iter().any(|f| f.as_str() == name) + } + + fn missing(&self, from: &[&str]) -> Vec { + from.iter() + .copied() + .filter(|f| !self.has_feature(f)) + .map(String::from) + .collect() + } +} + +/// The verdict the link/settings UI renders and the sync engine obeys. +/// +/// Serialized tagged so the frontend can `switch` on `status` directly. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +#[serde(tag = "status", rename_all = "snake_case")] +pub enum Compatibility { + /// Full parity — sync everything. + Ok, + /// Safe to sync, but these named capabilities aren't available here. + Degraded { unavailable: Vec }, + /// Do not sync. `client_must_update` points the user at the side that can fix + /// it, so the message can be actionable instead of just "incompatible". + Incompatible { + reason: String, + client_must_update: bool, + }, +} + +fn incompatible(reason: &str, client_must_update: bool) -> Compatibility { + Compatibility::Incompatible { + reason: reason.to_string(), + client_must_update, + } +} + +/// Decide whether this client can sync with the described server. +/// +/// Pure: the transport fetches `ServerInfo`, this decides what it means. Keeping +/// the decision free of I/O is what makes every branch below unit-testable, which +/// matters because there is no Postgres/live-server lane in CI. +pub fn evaluate(info: &ServerInfo) -> Compatibility { + // Ordered most-fundamental first, so the user sees the root problem rather than + // a downstream symptom of it. + let Some(server_proto) = info.sync_protocol_version else { + return incompatible( + "This server doesn't support device sync — it predates the sync protocol. \ + Update the server, then link again.", + false, + ); + }; + + if server_proto < MIN_SERVER_PROTOCOL_VERSION { + return incompatible( + &format!( + "This server speaks sync protocol v{server_proto}, but this app needs \ + at least v{MIN_SERVER_PROTOCOL_VERSION}. Update the server." + ), + false, + ); + } + + // The server's floor is what hard-blocks an old client. Absent => no floor: a + // server that advertises a protocol but no minimum accepts anything. + let floor = info.min_client_protocol_version.unwrap_or(0); + if CLIENT_PROTOCOL_VERSION < floor { + return incompatible( + &format!( + "This server requires client protocol v{floor} or newer; this app \ + speaks v{CLIENT_PROTOCOL_VERSION}. Update ThoughtSync." + ), + true, + ); + } + + // A version match still isn't enough: a server can speak the protocol with a + // core capability compiled out or disabled. + let missing_required = info.missing(REQUIRED_FEATURES); + if !missing_required.is_empty() { + return incompatible( + &format!( + "This server is missing sync capabilities this app requires: {}.", + missing_required.join(", ") + ), + false, + ); + } + + let unavailable = info.missing(OPTIONAL_FEATURES); + if unavailable.is_empty() { + Compatibility::Ok + } else { + Compatibility::Degraded { unavailable } + } +} + +/// Headers this client puts on every request to a linked server, so the server can +/// log or gate on client identity without a separate handshake round-trip. +pub fn client_headers() -> [(&'static str, String); 2] { + let agent = format!("thoughtsync-desktop/{}", env!("CARGO_PKG_VERSION")); + [ + ("X-ThoughtSync-Client", agent), + ("X-ThoughtSync-Protocol", CLIENT_PROTOCOL_VERSION.to_string()), + ] +} + +/// Turn what a user typed into a base URL we can build request paths on, or `None` +/// if there's nothing usable in it. +/// +/// A bare host gets **`https://`**, never `http://`. Silently downgrading would put +/// a long-lived device token on the wire in cleartext because someone omitted five +/// characters. Plain HTTP on a trusted LAN stays fully supported — the user just +/// has to type `http://` and thereby choose it. +pub fn normalize_base_url(raw: &str) -> Option { + let trimmed = raw.trim(); + if trimmed.is_empty() { + return None; + } + // Resolve the scheme BEFORE touching trailing slashes — stripping them first + // turns a bare "https://" into "https:", which then reads as a hostname. + let with_scheme = match trimmed.split_once("://") { + Some((scheme, rest)) => { + // Anything that isn't HTTP(S) (ftp://, file://, a stray "foo://") can't + // be a ThoughtSync server; reject rather than fail confusingly later. + let scheme = scheme.to_ascii_lowercase(); + if scheme != "http" && scheme != "https" { + return None; + } + format!("{scheme}://{rest}") + } + None => format!("https://{trimmed}"), + }; + let (scheme, rest) = with_scheme.split_once("://")?; + let rest = rest.trim_end_matches('/'); + // Reject a scheme with no authority ("https://", "http:///path"). + if rest.split(['/', '?', '#']).next().unwrap_or("").is_empty() { + return None; + } + Some(format!("{scheme}://{rest}")) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// A server matching this client exactly, which each test then degrades. + fn current_server() -> ServerInfo { + ServerInfo { + site_name: Some("ThoughtSync".into()), + version: Some("0.1.0".into()), + sync_protocol_version: Some(CLIENT_PROTOCOL_VERSION), + min_client_protocol_version: Some(CLIENT_PROTOCOL_VERSION), + sync_features: REQUIRED_FEATURES + .iter() + .chain(OPTIONAL_FEATURES.iter()) + .copied() + .map(String::from) + .collect(), + } + } + + #[test] + fn current_server_is_fully_compatible() { + assert_eq!(evaluate(¤t_server()), Compatibility::Ok); + } + + #[test] + fn server_without_protocol_fields_is_too_old() { + // A pre-M10.6 server: /api/config parses, but carries no protocol block. + let info = ServerInfo { + site_name: Some("ThoughtSync".into()), + version: Some("0.0.9".into()), + ..Default::default() + }; + match evaluate(&info) { + Compatibility::Incompatible { + client_must_update, .. + } => assert!(!client_must_update, "the SERVER is the old side here"), + other => panic!("expected incompatible, got {other:?}"), + } + } + + #[test] + fn client_older_than_the_servers_floor_must_update() { + let info = ServerInfo { + sync_protocol_version: Some(CLIENT_PROTOCOL_VERSION + 5), + min_client_protocol_version: Some(CLIENT_PROTOCOL_VERSION + 5), + ..current_server() + }; + match evaluate(&info) { + Compatibility::Incompatible { + client_must_update, .. + } => assert!(client_must_update), + other => panic!("expected incompatible, got {other:?}"), + } + } + + #[test] + fn newer_server_within_our_floor_still_works() { + // The whole point of the two-number contract: a server can move ahead + // additively without locking out a client that predates the change. + let info = ServerInfo { + sync_protocol_version: Some(CLIENT_PROTOCOL_VERSION + 3), + min_client_protocol_version: Some(CLIENT_PROTOCOL_VERSION), + ..current_server() + }; + assert_eq!(evaluate(&info), Compatibility::Ok); + } + + #[test] + fn server_with_no_declared_floor_accepts_us() { + let info = ServerInfo { + min_client_protocol_version: None, + ..current_server() + }; + assert_eq!(evaluate(&info), Compatibility::Ok); + } + + #[test] + fn missing_optional_feature_degrades_rather_than_blocks() { + let info = ServerInfo { + sync_features: current_server() + .sync_features + .into_iter() + .filter(|f| f.as_str() != "attachments") + .collect(), + ..current_server() + }; + assert_eq!( + evaluate(&info), + Compatibility::Degraded { + unavailable: vec!["attachments".to_string()] + } + ); + } + + #[test] + fn missing_required_feature_blocks() { + let info = ServerInfo { + sync_features: vec!["labels".to_string()], + ..current_server() + }; + match evaluate(&info) { + Compatibility::Incompatible { reason, .. } => assert!(reason.contains("notes")), + other => panic!("expected incompatible, got {other:?}"), + } + } + + #[test] + fn version_mismatch_outranks_a_missing_feature() { + // Both wrong → report the version, the root cause of the missing feature. + let info = ServerInfo { + sync_protocol_version: Some(CLIENT_PROTOCOL_VERSION + 2), + min_client_protocol_version: Some(CLIENT_PROTOCOL_VERSION + 2), + sync_features: vec![], + ..current_server() + }; + match evaluate(&info) { + Compatibility::Incompatible { + client_must_update, .. + } => assert!(client_must_update), + other => panic!("expected incompatible, got {other:?}"), + } + } + + #[test] + fn verdict_serializes_tagged_for_the_frontend() { + let verdict = Compatibility::Degraded { + unavailable: vec!["attachments".into()], + }; + let json = serde_json::to_string(&verdict).expect("verdict serializes"); + assert!(json.contains("\"status\":\"degraded\""), "got {json}"); + } + + #[test] + fn server_info_tolerates_unknown_and_absent_fields() { + // Forward compatibility: a NEWER server sending fields we've never heard of + // must not break the handshake. + let info: ServerInfo = serde_json::from_str( + r#"{"site_name":"S","sync_protocol_version":1, + "min_client_protocol_version":1, + "sync_features":["notes","labels","attachments","tombstones","revisions"], + "some_future_field":{"nested":true}}"#, + ) + .expect("unknown fields are ignored"); + assert_eq!(evaluate(&info), Compatibility::Ok); + } + + #[test] + fn client_headers_identify_app_and_protocol() { + let headers = client_headers(); + assert_eq!(headers[0].0, "X-ThoughtSync-Client"); + assert!(headers[0].1.starts_with("thoughtsync-desktop/")); + assert_eq!(headers[1].1, CLIENT_PROTOCOL_VERSION.to_string()); + } + + #[test] + fn base_url_defaults_to_https_and_trims() { + assert_eq!( + normalize_base_url(" notes.example.com/ "), + Some("https://notes.example.com".to_string()) + ); + assert_eq!( + normalize_base_url("https://notes.example.com///"), + Some("https://notes.example.com".to_string()) + ); + } + + #[test] + fn base_url_keeps_an_explicit_http_choice() { + // Plain HTTP on a LAN is supported — the user just has to ask for it. + assert_eq!( + normalize_base_url("http://192.168.1.10:8000"), + Some("http://192.168.1.10:8000".to_string()) + ); + } + + #[test] + fn base_url_rejects_junk() { + assert_eq!(normalize_base_url(""), None); + assert_eq!(normalize_base_url(" "), None); + assert_eq!(normalize_base_url("https://"), None); + assert_eq!(normalize_base_url("ftp://files.example.com"), None); + } +} diff --git a/desktop/src-tauri/src/sync/mod.rs b/desktop/src-tauri/src/sync/mod.rs new file mode 100644 index 0000000..62c37f3 --- /dev/null +++ b/desktop/src-tauri/src/sync/mod.rs @@ -0,0 +1,10 @@ +//! Talking to a ThoughtSync server — entirely opt-in. +//! +//! The app is local-first: `local` is the source of truth and everything works +//! unlinked. Nothing in here runs until the user links a server. +//! +//! `compat` owns the version/capability handshake (M10.6) that decides whether a +//! given server can be talked to at all. The engine that then moves notes — push, +//! pull, the revision cursor, last-write-wins — lands in M10.7 and consults it. + +pub mod compat; diff --git a/docs/sync.md b/docs/sync.md index 5d0e965..ec4597e 100644 --- a/docs/sync.md +++ b/docs/sync.md @@ -13,6 +13,67 @@ All sync endpoints live under `/api/sync`. Everything is **owner-scoped** and > operator on deploy, not in CI. Pure logic (LWW comparator, paging cursor, > token hashing) is unit-tested. +## Protocol versioning — the compatibility handshake + +Clients and servers update on their own schedules; a self-hosted server can sit on +an older release than the desktop app for months. So the wire protocol is +versioned **separately from either program's release version**, and each side +declares two numbers: what it speaks, and the oldest counterpart it accepts. + +| | server (`src/thoughtsync/sync.py`) | client (`desktop/src-tauri/src/sync/compat.rs`) | +|---|---|---| +| speaks | `SYNC_PROTOCOL_VERSION` | `CLIENT_PROTOCOL_VERSION` | +| accepts down to | `MIN_CLIENT_PROTOCOL_VERSION` | `MIN_SERVER_PROTOCOL_VERSION` | + +The server publishes its half on the **public, unauthenticated** `GET /api/config` +— a client must be able to ask "can I talk to you?" before it holds a device +token, or even has an account: + +```json +{ "site_name": "...", "version": "0.1.0", + "sync_protocol_version": 1, + "min_client_protocol_version": 1, + "sync_features": ["notes", "labels", "attachments", "tombstones", "revisions"] } +``` + +The client identifies itself on every request with +`X-ThoughtSync-Client: thoughtsync-desktop/` and +`X-ThoughtSync-Protocol: `. + +### `sync_features` — why versions alone aren't enough + +A version number can only say "newer" or "older". `sync_features` names +capabilities, so a client tests for the one it needs instead of inferring it from +a number. That is what keeps an **additive** change from forcing a lockstep +upgrade: a newer client meeting an older server drops the missing feature and +syncs everything else. + +### The policy + +- **Any wire change** → bump `SYNC_PROTOCOL_VERSION`. +- **Additive change** (a new field, a new capability) → add a `sync_features` + name. Do **not** raise a minimum. Old clients keep working. +- **Breaking change only** → raise `MIN_CLIENT_PROTOCOL_VERSION` (or the client's + `MIN_SERVER_PROTOCOL_VERSION`). This is the switch that hard-blocks the other + side, so it is the one to be stingy with. +- Never gate behavior on the *release* version (`version`) — it's for display. + +### The three outcomes + +The client evaluates the advertisement (`compat::evaluate`) and gets exactly one +of: + +- **ok** — full parity; sync everything. +- **degraded** — safe to sync, but named capabilities are unavailable here; the UI + says which. +- **incompatible** — do not sync. Carries `client_must_update` so the message can + point at the side that can actually fix it, rather than just saying + "incompatible". + +A server that predates this handshake sends no protocol fields at all. That is +treated as **incompatible (update the server)** — deliberately not as a parse +error, which would look to the user like they mistyped the URL. + ## Authentication — device bearer tokens Native clients authenticate with a long-lived **device token**, not a session diff --git a/src/thoughtsync/app.py b/src/thoughtsync/app.py index 59eb8e1..07b032e 100644 --- a/src/thoughtsync/app.py +++ b/src/thoughtsync/app.py @@ -18,7 +18,7 @@ from .notes import bp as notes_bp from .saved_filters import bp as saved_filters_bp from .settings import get_public_config, get_setting, load_or_create_secret_key from .settings_api import bp as settings_bp -from .sync import bp as sync_bp +from .sync import bp as sync_bp, protocol_advertisement STATIC_DIR = os.path.join(os.path.dirname(__file__), "static") @@ -91,6 +91,10 @@ def create_app() -> Quart: async with session_scope() as db: data = await get_public_config(db) data["version"] = app.config["APP_VERSION"] + # The sync-protocol handshake (M10.6). A native client reads this BEFORE + # linking — while it still has no token and possibly no account — to decide + # whether it can talk to this server, and which optional features to offer. + data.update(protocol_advertisement()) return jsonify(data) @app.get("/", defaults={"path": ""}) diff --git a/src/thoughtsync/sync.py b/src/thoughtsync/sync.py index d11de7b..1889149 100644 --- a/src/thoughtsync/sync.py +++ b/src/thoughtsync/sync.py @@ -46,6 +46,48 @@ DEFAULT_LIMIT = 500 MAX_LIMIT = 1000 MAX_PUSH = 1000 # per-batch change cap +# --- protocol versioning (M10.6) -------------------------------------------- +# +# The client<->server compatibility contract. These integers version the WIRE +# PROTOCOL, deliberately separate from the app's release version, so a client and +# server on different releases can still work out whether they can talk. Without +# that separation every protocol change would force app<->server lockstep. +# +# SYNC_PROTOCOL_VERSION what this server speaks. +# MIN_CLIENT_PROTOCOL_VERSION the oldest client protocol it still accepts. +# +# Bump SYNC_PROTOCOL_VERSION for ANY wire change. Raise +# MIN_CLIENT_PROTOCOL_VERSION only for a genuinely BREAKING one: it is the switch +# that hard-blocks older clients, so additive changes must leave it alone. +SYNC_PROTOCOL_VERSION = 1 +MIN_CLIENT_PROTOCOL_VERSION = 1 + +# Named capabilities beyond the base protocol. An ADDITIVE change earns a name +# here rather than a min-version bump, so a newer client meeting an older server +# can degrade to "some features unavailable" instead of refusing to sync. Clients +# test for the name, never infer a capability from a version number — that's what +# keeps feature gating independent of release lockstep. +SYNC_FEATURES: tuple[str, ...] = ( + "notes", # note delta sync (pull + push) + "labels", # the label catalog as its own entity + "attachments", # blob upload/download, deduped by sha256 + "tombstones", # purge propagates as a content-less row + "revisions", # an overwritten version snapshots into note history +) + + +def protocol_advertisement() -> dict: + """What the server publishes about the sync protocol, merged into `/api/config`. + + DB-free and unauthenticated on purpose: a client has to be able to ask "can I + talk to you at all?" before it holds a device token — or even has an account. + """ + return { + "sync_protocol_version": SYNC_PROTOCOL_VERSION, + "min_client_protocol_version": MIN_CLIENT_PROTOCOL_VERSION, + "sync_features": list(SYNC_FEATURES), + } + def _parse_since(raw: str | None) -> int: """The pull cursor: a non-negative revision watermark. Bad/absent → 0 (full sync).""" diff --git a/tests/test_sync.py b/tests/test_sync.py index 2ce5b40..51ee4e3 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -6,10 +6,14 @@ from thoughtsync.app import create_app from thoughtsync.sync import ( DEFAULT_LIMIT, MAX_LIMIT, + MIN_CLIENT_PROTOCOL_VERSION, + SYNC_FEATURES, + SYNC_PROTOCOL_VERSION, _clamp_limit, _page_cursor, _parse_since, client_wins, + protocol_advertisement, ) @@ -82,3 +86,28 @@ def test_page_cursor_both_full_uses_min_boundary(): cursor, more = _page_cursor([1, 2, 10], [3, 4, 5], since=0, limit=3) assert cursor == 5 assert more is True + + +# --- protocol handshake (M10.6) --------------------------------------------- + + +def test_protocol_advertisement_shape(): + ad = protocol_advertisement() + assert ad["sync_protocol_version"] == SYNC_PROTOCOL_VERSION + assert ad["min_client_protocol_version"] == MIN_CLIENT_PROTOCOL_VERSION + # A list, not a tuple — it has to survive jsonify as a JSON array. + assert isinstance(ad["sync_features"], list) + assert ad["sync_features"] == list(SYNC_FEATURES) + + +def test_protocol_floor_never_exceeds_current(): + # A server can't demand a client protocol newer than the one it speaks itself — + # that would lock out every client, including a perfectly current one. + assert MIN_CLIENT_PROTOCOL_VERSION <= SYNC_PROTOCOL_VERSION + + +def test_protocol_features_are_unique_nonempty_names(): + # Clients match capabilities by exact name, so duplicates or blanks would make + # a feature check silently meaningless. + assert all(f and f.strip() == f for f in SYNC_FEATURES) + assert len(set(SYNC_FEATURES)) == len(SYNC_FEATURES) -- 2.54.0 From 4b4bfe67adaf89301fd52913e857c62301ed586b Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sat, 25 Jul 2026 22:42:57 -0400 Subject: [PATCH 04/86] desktop: rustfmt the client-header tuple Applied verbatim from run 2886's cargo fmt --check diff. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01SreJkbxB4gx8pPsu8QbLPi --- desktop/src-tauri/src/sync/compat.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/desktop/src-tauri/src/sync/compat.rs b/desktop/src-tauri/src/sync/compat.rs index 9092afd..b9fdd03 100644 --- a/desktop/src-tauri/src/sync/compat.rs +++ b/desktop/src-tauri/src/sync/compat.rs @@ -159,7 +159,10 @@ pub fn client_headers() -> [(&'static str, String); 2] { let agent = format!("thoughtsync-desktop/{}", env!("CARGO_PKG_VERSION")); [ ("X-ThoughtSync-Client", agent), - ("X-ThoughtSync-Protocol", CLIENT_PROTOCOL_VERSION.to_string()), + ( + "X-ThoughtSync-Protocol", + CLIENT_PROTOCOL_VERSION.to_string(), + ), ] } -- 2.54.0 From 4eb92942d017930439174bccb963a634927d5a98 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sat, 25 Jul 2026 22:47:10 -0400 Subject: [PATCH 05/86] M10.6: HTTP transport for the handshake (task 1995) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the client's first outbound call: GET {server}/api/config, carrying X-ThoughtSync-Client and X-ThoughtSync-Protocol, feeding compat::evaluate. Deliberately its OWN commit. This introduces the first HTTP+TLS stack into a crate that cross-compiles to Windows from Linux via cargo-xwin — the lane that has already broken once on a transitive C dependency (libsqlite3-sys needing llvm-lib). Landing it alone means a failure here has exactly one possible cause, instead of surfacing mid-way through M10.7's much larger change where it would be expensive to bisect. TLS backend is native-tls, NOT rustls, and that is the whole point of the choice: on x86_64-pc-windows-msvc native-tls resolves to `schannel`, which is pure-Rust bindings to the OS TLS stack, so nothing C or assembly has to cross-compile on the fragile lane. rustls would pull in ring/aws-lc-rs and their assembler. On Linux native-tls uses OpenSSL, whose headers ci-tauri already ships (libssl-dev, part of Tauri's own Linux prerequisites). Verified from run 2884's log rather than assumed: tokio and http are already in the Windows tree via tauri, but no HTTP client and no TLS stack were — so this genuinely is new surface there, not a no-op. probe() distinguishes "never got a usable answer" (Err) from "answered, but we can't work with it" (Ok + verdict). Those need very different messages: one is "check what you typed", the other is "update something". Transport errors are translated out of reqwest's Display, which is accurate but reads like a stack trace. Still no UI — M10.7 owns the link/settings surface that calls server_probe. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01SreJkbxB4gx8pPsu8QbLPi --- desktop/src-tauri/Cargo.toml | 10 ++ desktop/src-tauri/src/lib.rs | 6 +- desktop/src-tauri/src/sync/client.rs | 135 +++++++++++++++++++++++++++ desktop/src-tauri/src/sync/mod.rs | 1 + 4 files changed, 150 insertions(+), 2 deletions(-) create mode 100644 desktop/src-tauri/src/sync/client.rs diff --git a/desktop/src-tauri/Cargo.toml b/desktop/src-tauri/Cargo.toml index b60ecfc..df86d99 100644 --- a/desktop/src-tauri/Cargo.toml +++ b/desktop/src-tauri/Cargo.toml @@ -28,6 +28,16 @@ chrono = { version = "0.4", default-features = false, features = ["clock"] } # issues are diagnosable from any environment. `log` is the facade the code uses. tauri-plugin-log = "2" log = "0.4" +# HTTP for the opt-in server handshake (M10.6) and, next, the sync engine (M10.7). +# +# native-tls, NOT rustls, deliberately: on x86_64-pc-windows-msvc native-tls +# resolves to `schannel` — pure-Rust bindings to the OS TLS stack — so nothing C or +# assembly has to cross-compile on the Windows lane, which is the fragile one (it +# builds on Linux via cargo-xwin, and a C dependency there is what broke it before). +# rustls would instead pull in ring/aws-lc-rs and their assembler. On Linux +# native-tls uses OpenSSL, whose headers (libssl-dev) ci-tauri already ships. +# default-features off drops http2/charset we don't need for a JSON API. +reqwest = { version = "0.12", default-features = false, features = ["json", "native-tls"] } # Tauri's default release profile: smaller, faster shipped binaries. [profile.release] diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index d15798a..b37b070 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -8,8 +8,9 @@ mod integration; mod local; -// `pub` (unlike the modules above) because nothing calls into it yet — the sync -// engine that will is M10.7. A private module's unused items read as dead code. +// `pub` (unlike the modules above) because most of it has no in-crate caller yet — +// the sync engine that will consume it is M10.7, and a private module's unreachable +// items read as dead code. pub mod sync; #[cfg_attr(mobile, tauri::mobile_entry_point)] @@ -88,6 +89,7 @@ pub fn run() { local::commands::saved_filters_create, local::commands::saved_filters_remove, local::commands::saved_filters_rename, + sync::client::server_probe, ]) .run(tauri::generate_context!()) .expect("error while running the ThoughtSync desktop app"); diff --git a/desktop/src-tauri/src/sync/client.rs b/desktop/src-tauri/src/sync/client.rs new file mode 100644 index 0000000..fd94e58 --- /dev/null +++ b/desktop/src-tauri/src/sync/client.rs @@ -0,0 +1,135 @@ +//! HTTP transport to a ThoughtSync server. +//! +//! Today it performs exactly one call: the compatibility handshake (M10.6). The +//! sync engine (M10.7) grows push/pull on top of the same client, which is why the +//! auth headers, timeout and error vocabulary are established here rather than +//! inline in the probe. +//! +//! Nothing here runs unless the user has linked a server — the app is local-first +//! and fully usable with no network at all. + +use std::time::Duration; + +use serde::Serialize; + +use super::compat::{self, Compatibility, ServerInfo}; + +/// Handshake timeout. Short on purpose: a user is watching a "Connect" button while +/// this runs, and the most common mistake — a wrong host on a LAN — fails by +/// hanging rather than refusing, so an unbounded wait would just look frozen. +const PROBE_TIMEOUT: Duration = Duration::from_secs(10); + +/// What the link UI needs after a successful handshake: where we ended up (the +/// normalized URL, which may differ from what was typed), who answered, and whether +/// we can work with them. +#[derive(Debug, Serialize)] +pub struct ProbeResult { + pub base_url: String, + pub server: ServerInfo, + pub compatibility: Compatibility, +} + +/// The public, unauthenticated endpoint carrying the handshake. +fn config_url(base_url: &str) -> String { + format!("{base_url}/api/config") +} + +/// Ask a server who it is and whether we can sync with it. +/// +/// `Err` means we never got a usable answer (bad address, unreachable, not a +/// ThoughtSync server). A server that answers but is *incompatible* is `Ok` with a +/// verdict — that distinction matters, because the two need very different messages: +/// one is "check what you typed", the other is "update something". +pub async fn probe(raw_url: &str) -> Result { + let base_url = compat::normalize_base_url(raw_url) + .ok_or("Enter a server address, like https://notes.example.com")?; + + let client = reqwest::Client::builder() + .timeout(PROBE_TIMEOUT) + .build() + .map_err(|e| format!("Could not start the network client: {e}"))?; + + let mut request = client.get(config_url(&base_url)); + for (name, value) in compat::client_headers() { + request = request.header(name, value); + } + + let response = request + .send() + .await + .map_err(|e| describe_transport_error(&base_url, &e))?; + + let status = response.status(); + if !status.is_success() { + return Err(format!( + "{base_url} answered with HTTP {}. Check the address — a reverse proxy or \ + a different site may be answering there.", + status.as_u16() + )); + } + + // Something answered 200 that isn't a ThoughtSync server (a router login page, a + // captive portal). Report the address, not the parse error, which would mean + // nothing to the person reading it. + let server: ServerInfo = response.json().await.map_err(|_| { + format!( + "{base_url} responded, but not with ThoughtSync's configuration. \ + Is that the right address?" + ) + })?; + + let compatibility = compat::evaluate(&server); + Ok(ProbeResult { + base_url, + server, + compatibility, + }) +} + +/// Turn a transport failure into something a person can act on. reqwest's own +/// Display is accurate but reads like a stack trace. +fn describe_transport_error(base_url: &str, err: &reqwest::Error) -> String { + if err.is_timeout() { + format!( + "{base_url} didn't respond within {} seconds. It may be offline, or \ + unreachable from this network.", + PROBE_TIMEOUT.as_secs() + ) + } else if err.is_connect() { + format!( + "Couldn't reach {base_url}. Check the address and that the server is \ + running. If it uses plain HTTP, include http:// explicitly." + ) + } else { + format!("Couldn't reach {base_url}: {err}") + } +} + +/// Probe a server from the link/settings UI. +#[tauri::command] +pub async fn server_probe(url: String) -> Result { + probe(&url).await +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn config_url_joins_without_doubling_slashes() { + // normalize_base_url has already stripped any trailing slash, so a plain + // concatenation is correct — this pins that assumption. + assert_eq!( + config_url("https://notes.example.com"), + "https://notes.example.com/api/config" + ); + } + + #[test] + fn config_url_preserves_a_port_and_subpath() { + assert_eq!( + config_url("http://192.168.1.10:8000/thoughtsync"), + "http://192.168.1.10:8000/thoughtsync/api/config" + ); + } +} diff --git a/desktop/src-tauri/src/sync/mod.rs b/desktop/src-tauri/src/sync/mod.rs index 62c37f3..acd26c0 100644 --- a/desktop/src-tauri/src/sync/mod.rs +++ b/desktop/src-tauri/src/sync/mod.rs @@ -7,4 +7,5 @@ //! given server can be talked to at all. The engine that then moves notes — push, //! pull, the revision cursor, last-write-wins — lands in M10.7 and consults it. +pub mod client; pub mod compat; -- 2.54.0 From 9118680bb1f46ba16af591b45eeb676903dc316b Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sat, 25 Jul 2026 22:53:31 -0400 Subject: [PATCH 06/86] docs: record the CI consequences of the M10.6 TLS dependency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ci-requirements.md is the contract with CI-Runner (rule 39), so the two things a future image change could silently break belong in it: libssl-dev + pkg-config in ci-tauri are now load-bearing — native-tls compiles against OpenSSL on Linux, so a slim-down of that image would fail the Rust build at openssl-sys rather than anywhere obvious. The TLS backend choice is a property of the WINDOWS lane, not a dependency detail: native-tls resolves to schannel on windows-msvc, keeping C/assembly out of the cross-compile. Swapping to rustls would pull in ring/aws-lc-rs and their assembler — the same class of dependency that broke that lane before. Flagged so it's treated as a lane change, not a version bump. Also documented why libssl3 is left covered TRANSITIVELY rather than declared. dpkg-shlibdeps now lists it, and verify.sh passes it through webkit's recursive closure. Declaring it directly would be worse, not better: the package name is release-dependent (libssl3 on bookworm, libssl3t64 after the time_t transition), so hardcoding it freezes the .deb to the build distro, whereas webkit's closure adapts. verify.sh fails loudly if webkit ever stops pulling OpenSSL, which is what makes that safe. Docs only — triggers no workflow. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01SreJkbxB4gx8pPsu8QbLPi --- ci-requirements.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/ci-requirements.md b/ci-requirements.md index d75e9bd..d167837 100644 --- a/ci-requirements.md +++ b/ci-requirements.md @@ -78,6 +78,22 @@ backend/frontend push. `pacman -Qkk` file verification needs `.MTREE`. Adding `libarchive-tools` + `zstd` + a docker CLI to `ci-tauri` would upgrade these paths; none of them block a green build. +- **`libssl-dev` + `pkg-config` are load-bearing** (both already in `ci-tauri`). + Since M10.6 the desktop crate depends on `reqwest` with the **`native-tls`** + backend, which on Linux compiles against OpenSSL. Do NOT drop either package + from `ci-tauri` in a future slim-down — the Rust build fails at `openssl-sys`. + (They're part of Tauri's own documented Linux prerequisites, so they should + stay regardless.) +- **`libssl3` is covered transitively, on purpose — don't "fix" it.** Since + M10.6 `dpkg-shlibdeps` lists `libssl3` among the binary's needs, but the + `.deb` declares only `libwebkit2gtk-4.1-0` + `libgtk-3-0`. `verify.sh` passes + it because webkit's own recursive dependency closure includes OpenSSL, so apt + installs it either way. Declaring it explicitly would be *worse*: the package + name is release-dependent (`libssl3` on bookworm, `libssl3t64` after the + 64-bit-time_t transition in trixie/Ubuntu 24.04), so a hardcoded name freezes + the package to the build distro. Leaning on webkit's closure adapts. If webkit + ever stops pulling OpenSSL, `verify.sh` fails the build loudly — that guard is + what makes the indirection safe. - **Not verifiable in CI:** the runner is Debian, so the pacman package cannot be `pacman -U`-tested here. That step logs `.PKGINFO` + the full file listing so the package is auditable from the run log; a real Arch install is the operator's @@ -106,5 +122,12 @@ backend/frontend push. mandatory before trusting a release. - **Unsigned.** Installers will trip SmartScreen until a code-signing certificate exists; that is a purchasing decision, not a CI one. +- **TLS backend is chosen for this lane's sake.** The desktop crate pins + `reqwest` to `native-tls`, which on `x86_64-pc-windows-msvc` resolves to + `schannel` — pure-Rust bindings to the OS TLS stack. That keeps C/assembly out + of the cross-compile entirely. Switching to `rustls` would pull in + `ring`/`aws-lc-rs` and their assembler, which is exactly the class of + dependency that broke this lane before (`libsqlite3-sys` → `llvm-lib`). Treat + a TLS-backend change as a change to *this lane*, not just a dependency bump. - No Postgres lane (unchanged): the desktop app's local store + sync behavior is verified on the operator's machine, not in CI. -- 2.54.0 From bbb2fd9b1c2f6974dc7ed15efb2ec3633ba09256 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sat, 25 Jul 2026 23:35:51 -0400 Subject: [PATCH 07/86] =?UTF-8?q?M10.7a:=20link/unlink=20a=20server=20?= =?UTF-8?q?=E2=80=94=20device=20auth=20+=20sync=5Fstate=20(task=202104)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pairing step. Nothing else in the sync arc can move until this works. sync/state.rs owns the link record in the sync_state row M10.4 already put in the local schema. Two safety properties are the reason it isn't just three setters: - Linking a DIFFERENT server resets the change-feed cursor. A cursor is only meaningful against the server that issued it; carrying one across would silently skip every change on the new server below that watermark — data loss wearing the costume of a successful sync. Re-linking the SAME server (a token refresh) keeps it, so a routine re-auth doesn't force a full re-download. - Unlink clears the cursor too, so a later link can't inherit a watermark from a server that never issued it. An unparseable or absent cursor reads as 0 (full sync). That direction is always safe: a redundant re-sync costs time, a too-high cursor costs notes. Likewise a half-written row (server but no token) reports NOT linked. state::Status deliberately has no device_token field — it crosses into the webview, and a long-lived bearer token has no business reachable from page scripts. A test asserts the token never appears in its serialization. Token lives in the app-data SQLite file, not an OS keyring: the keyring crate needs libsecret/DBus on Linux, which adds a C dependency to a binary that has to cross-compile and fails outright on headless/minimal-WM setups — the same class of environment assumption behind the black-window bug. sync_link runs the M10.6 handshake FIRST and refuses an incompatible server before any credential is sent. Two credential paths, because neither covers everyone: device-login (a fresh install has no session to mint a token from) and a pasted token (some users would rather not type a password into a desktop app). A pasted token is verified against /api/auth/me before being stored — auth.py's login_required accepts bearer — since an unverified paste would turn a copy/paste slip into a failure surfacing at the next sync, far from its cause. The store lock is taken only after all network work: a std MutexGuard isn't Send so it cannot cross an await, and holding the store for a round-trip would freeze every note operation in the UI. Unlink is LOCAL only — the token stays valid server-side until revoked under Account -> Linked devices. A pasted token arrives without its device id, so a reliable remote revoke isn't possible from here; the UI must say so rather than imply a revoke that didn't happen. Follow-up filed. No UI yet — that's M10.7e. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01SreJkbxB4gx8pPsu8QbLPi --- desktop/src-tauri/src/lib.rs | 9 +- desktop/src-tauri/src/sync/client.rs | 198 ++++++++++++++++----- desktop/src-tauri/src/sync/commands.rs | 124 +++++++++++++ desktop/src-tauri/src/sync/mod.rs | 13 +- desktop/src-tauri/src/sync/state.rs | 233 +++++++++++++++++++++++++ 5 files changed, 526 insertions(+), 51 deletions(-) create mode 100644 desktop/src-tauri/src/sync/commands.rs create mode 100644 desktop/src-tauri/src/sync/state.rs diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index b37b070..dfe1352 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -8,8 +8,8 @@ mod integration; mod local; -// `pub` (unlike the modules above) because most of it has no in-crate caller yet — -// the sync engine that will consume it is M10.7, and a private module's unreachable +// `pub` (unlike the modules above) because parts of it have no in-crate caller yet — +// the engine that will consume them is M10.7b/c, and a private module's unreachable // items read as dead code. pub mod sync; @@ -89,7 +89,10 @@ pub fn run() { local::commands::saved_filters_create, local::commands::saved_filters_remove, local::commands::saved_filters_rename, - sync::client::server_probe, + sync::commands::sync_probe, + sync::commands::sync_link, + sync::commands::sync_unlink, + sync::commands::sync_status, ]) .run(tauri::generate_context!()) .expect("error while running the ThoughtSync desktop app"); diff --git a/desktop/src-tauri/src/sync/client.rs b/desktop/src-tauri/src/sync/client.rs index fd94e58..b75fc6c 100644 --- a/desktop/src-tauri/src/sync/client.rs +++ b/desktop/src-tauri/src/sync/client.rs @@ -1,27 +1,29 @@ //! HTTP transport to a ThoughtSync server. //! -//! Today it performs exactly one call: the compatibility handshake (M10.6). The -//! sync engine (M10.7) grows push/pull on top of the same client, which is why the -//! auth headers, timeout and error vocabulary are established here rather than -//! inline in the probe. +//! Covers the compatibility handshake (M10.6) and device-token auth (M10.7a). The +//! engine that moves notes — push, pull, cursor — grows on top of the same client, +//! which is why the timeout, identity headers and error vocabulary live here rather +//! than inline at each call site. //! -//! Nothing here runs unless the user has linked a server — the app is local-first -//! and fully usable with no network at all. +//! Nothing here runs unless the user has linked a server; the app is local-first and +//! fully usable with no network at all. use std::time::Duration; -use serde::Serialize; +use reqwest::{RequestBuilder, StatusCode}; +use serde::{Deserialize, Serialize}; use super::compat::{self, Compatibility, ServerInfo}; -/// Handshake timeout. Short on purpose: a user is watching a "Connect" button while -/// this runs, and the most common mistake — a wrong host on a LAN — fails by -/// hanging rather than refusing, so an unbounded wait would just look frozen. -const PROBE_TIMEOUT: Duration = Duration::from_secs(10); +/// Timeout for the short request/response calls in this module. Kept tight because a +/// user is watching a button while they run, and the most common mistake — a wrong +/// host on a LAN — fails by hanging rather than refusing, so an unbounded wait would +/// just look frozen. The sync engine's bulk transfers will need their own, longer one. +const REQUEST_TIMEOUT: Duration = Duration::from_secs(10); -/// What the link UI needs after a successful handshake: where we ended up (the -/// normalized URL, which may differ from what was typed), who answered, and whether -/// we can work with them. +/// What the link UI needs after a handshake: where we ended up (the normalized URL, +/// which may differ from what was typed), who answered, and whether we can work +/// with them. #[derive(Debug, Serialize)] pub struct ProbeResult { pub base_url: String, @@ -29,31 +31,62 @@ pub struct ProbeResult { pub compatibility: Compatibility, } -/// The public, unauthenticated endpoint carrying the handshake. -fn config_url(base_url: &str) -> String { - format!("{base_url}/api/config") +/// The account a device token belongs to. Surfaced after linking so the user can +/// confirm they linked the account they meant to — easy to get wrong on a server +/// hosting more than one. +#[derive(Debug, Clone, Deserialize, Serialize)] +pub struct Identity { + pub id: String, + pub email: String, + #[serde(default)] + pub display_name: String, +} + +#[derive(Deserialize)] +struct DeviceLoginResponse { + token: String, + user: Identity, +} + +fn http() -> Result { + reqwest::Client::builder() + .timeout(REQUEST_TIMEOUT) + .build() + .map_err(|e| format!("Could not start the network client: {e}")) +} + +/// Attach the client-identity headers every request carries, plus a bearer token +/// when we hold one. +fn prepare(builder: RequestBuilder, token: Option<&str>) -> RequestBuilder { + let mut builder = builder; + for (name, value) in compat::client_headers() { + builder = builder.header(name, value); + } + match token { + Some(t) => builder.bearer_auth(t), + None => builder, + } +} + +fn unexpected_status(base_url: &str, status: StatusCode) -> String { + format!( + "{base_url} answered with HTTP {}. Check the address — a reverse proxy or a \ + different site may be answering there.", + status.as_u16() + ) } /// Ask a server who it is and whether we can sync with it. /// /// `Err` means we never got a usable answer (bad address, unreachable, not a -/// ThoughtSync server). A server that answers but is *incompatible* is `Ok` with a -/// verdict — that distinction matters, because the two need very different messages: -/// one is "check what you typed", the other is "update something". +/// ThoughtSync server). A server that answers but is *incompatible* comes back `Ok` +/// with a verdict — that distinction matters, because the two need very different +/// messages: one is "check what you typed", the other is "update something". pub async fn probe(raw_url: &str) -> Result { let base_url = compat::normalize_base_url(raw_url) .ok_or("Enter a server address, like https://notes.example.com")?; - let client = reqwest::Client::builder() - .timeout(PROBE_TIMEOUT) - .build() - .map_err(|e| format!("Could not start the network client: {e}"))?; - - let mut request = client.get(config_url(&base_url)); - for (name, value) in compat::client_headers() { - request = request.header(name, value); - } - + let request = prepare(http()?.get(config_url(&base_url)), None); let response = request .send() .await @@ -61,11 +94,7 @@ pub async fn probe(raw_url: &str) -> Result { let status = response.status(); if !status.is_success() { - return Err(format!( - "{base_url} answered with HTTP {}. Check the address — a reverse proxy or \ - a different site may be answering there.", - status.as_u16() - )); + return Err(unexpected_status(&base_url, status)); } // Something answered 200 that isn't a ThoughtSync server (a router login page, a @@ -86,6 +115,83 @@ pub async fn probe(raw_url: &str) -> Result { }) } +/// Exchange email + password for a device bearer token. +/// +/// The fresh-install path: it needs no existing session, which is what lets a brand +/// new desktop install link without visiting the web app first. +pub async fn device_login( + base_url: &str, + email: &str, + password: &str, + device_name: &str, +) -> Result<(String, Identity), String> { + let body = serde_json::json!({ + "email": email, + "password": password, + "name": device_name, + }); + let request = prepare(http()?.post(device_login_url(base_url)), None).json(&body); + let response = request + .send() + .await + .map_err(|e| describe_transport_error(base_url, &e))?; + + let status = response.status(); + if status == StatusCode::UNAUTHORIZED { + return Err("That email and password didn't match an account on this server.".to_string()); + } + if !status.is_success() { + return Err(unexpected_status(base_url, status)); + } + + let parsed: DeviceLoginResponse = response + .json() + .await + .map_err(|_| format!("{base_url} signed us in but sent an unexpected reply."))?; + Ok((parsed.token, parsed.user)) +} + +/// Validate a token by asking whom it belongs to. +/// +/// Used when the user pastes a token issued from the web app. Storing it unverified +/// would turn a copy/paste slip into a failure that only surfaces at the next sync, +/// far from the thing that caused it. +pub async fn fetch_identity(base_url: &str, token: &str) -> Result { + let request = prepare(http()?.get(me_url(base_url)), Some(token)); + let response = request + .send() + .await + .map_err(|e| describe_transport_error(base_url, &e))?; + + let status = response.status(); + if status == StatusCode::UNAUTHORIZED { + let message = "That token isn't valid on this server — it may have been revoked. \ + Issue a new one from the web app under Account → Linked devices."; + return Err(message.to_string()); + } + if !status.is_success() { + return Err(unexpected_status(base_url, status)); + } + + response + .json() + .await + .map_err(|_| format!("{base_url} accepted the token but sent an unexpected reply.")) +} + +/// The public, unauthenticated endpoint carrying the handshake. +fn config_url(base_url: &str) -> String { + format!("{base_url}/api/config") +} + +fn device_login_url(base_url: &str) -> String { + format!("{base_url}/api/auth/device-login") +} + +fn me_url(base_url: &str) -> String { + format!("{base_url}/api/auth/me") +} + /// Turn a transport failure into something a person can act on. reqwest's own /// Display is accurate but reads like a stack trace. fn describe_transport_error(base_url: &str, err: &reqwest::Error) -> String { @@ -93,7 +199,7 @@ fn describe_transport_error(base_url: &str, err: &reqwest::Error) -> String { format!( "{base_url} didn't respond within {} seconds. It may be offline, or \ unreachable from this network.", - PROBE_TIMEOUT.as_secs() + REQUEST_TIMEOUT.as_secs() ) } else if err.is_connect() { format!( @@ -105,28 +211,30 @@ fn describe_transport_error(base_url: &str, err: &reqwest::Error) -> String { } } -/// Probe a server from the link/settings UI. -#[tauri::command] -pub async fn server_probe(url: String) -> Result { - probe(&url).await -} - #[cfg(test)] mod tests { use super::*; #[test] - fn config_url_joins_without_doubling_slashes() { - // normalize_base_url has already stripped any trailing slash, so a plain + fn urls_join_without_doubling_slashes() { + // normalize_base_url has already stripped any trailing slash, so plain // concatenation is correct — this pins that assumption. assert_eq!( config_url("https://notes.example.com"), "https://notes.example.com/api/config" ); + assert_eq!( + device_login_url("https://notes.example.com"), + "https://notes.example.com/api/auth/device-login" + ); + assert_eq!( + me_url("https://notes.example.com"), + "https://notes.example.com/api/auth/me" + ); } #[test] - fn config_url_preserves_a_port_and_subpath() { + fn urls_preserve_a_port_and_subpath() { assert_eq!( config_url("http://192.168.1.10:8000/thoughtsync"), "http://192.168.1.10:8000/thoughtsync/api/config" diff --git a/desktop/src-tauri/src/sync/commands.rs b/desktop/src-tauri/src/sync/commands.rs new file mode 100644 index 0000000..1d7a0c9 --- /dev/null +++ b/desktop/src-tauri/src/sync/commands.rs @@ -0,0 +1,124 @@ +//! Tauri commands for pairing with a server (M10.7a). +//! +//! Linking is opt-in and reversible; the app is fully usable having never touched +//! any of this. The Settings UI (M10.7e) drives these. + +use serde::{Deserialize, Serialize}; +use tauri::State; + +use crate::local::Db; +use crate::sync::client::{self, Identity, ProbeResult}; +use crate::sync::compat::Compatibility; +use crate::sync::state; + +/// Ask a server who it is, without committing to anything. The UI calls this as the +/// user finishes typing an address, so they see what answered before handing over +/// credentials. +#[tauri::command] +pub async fn sync_probe(url: String) -> Result { + client::probe(&url).await +} + +/// Either a password login or a token pasted from the web app. Both are offered +/// because neither covers everyone: a fresh install has no session to mint a token +/// from, while someone using a password manager or SSO may prefer not to type a +/// password into a desktop app at all. +#[derive(Deserialize)] +pub struct LinkInput { + pub url: String, + #[serde(default)] + pub email: Option, + #[serde(default)] + pub password: Option, + #[serde(default)] + pub token: Option, + /// How this device is labelled in the server's device list. + #[serde(default)] + pub name: Option, +} + +#[derive(Serialize)] +pub struct LinkResult { + pub status: state::Status, + pub identity: Identity, + /// Carried through so the UI can warn about a `degraded` server right after + /// linking, instead of staying silent until a feature quietly does nothing. + pub compatibility: Compatibility, +} + +/// A recognizable default, so a server's device list doesn't fill up with "Device". +fn default_device_name() -> String { + format!("ThoughtSync desktop ({})", std::env::consts::OS) +} + +fn trimmed(value: &Option) -> Option<&str> { + value.as_deref().map(str::trim).filter(|s| !s.is_empty()) +} + +#[tauri::command] +pub async fn sync_link(input: LinkInput, db: State<'_, Db>) -> Result { + // 1. Handshake FIRST. Never hand credentials to a server we've established we + // can't sync with — and an incompatible server is exactly the case where a + // later failure would be hardest to attribute. + let probe = client::probe(&input.url).await?; + if let Compatibility::Incompatible { reason, .. } = &probe.compatibility { + return Err(reason.clone()); + } + let base_url = probe.base_url; + + // 2. Obtain a credential. + let (token, identity) = match trimmed(&input.token) { + Some(token) => { + // Verify before storing: an unverified paste turns a copy/paste slip + // into a failure that only surfaces at the next sync. + let identity = client::fetch_identity(&base_url, token).await?; + (token.to_string(), identity) + } + None => { + let (Some(email), Some(password)) = (trimmed(&input.email), trimmed(&input.password)) + else { + return Err("Enter your email and password, or paste a device token.".to_string()); + }; + let name = trimmed(&input.name) + .map(str::to_string) + .unwrap_or_else(default_device_name); + client::device_login(&base_url, email, password, &name).await? + } + }; + + // 3. Persist. The lock is taken only now, for two reasons: a std MutexGuard + // isn't Send so it cannot be held across an await, and holding the store + // locked for a network round-trip would freeze every note operation in the UI. + let status = { + let conn = db.0.lock().map_err(|e| e.to_string())?; + state::set_link(&conn, &base_url, &token).map_err(|e| e.to_string())?; + state::status(&conn).map_err(|e| e.to_string())? + }; + + log::info!("linked to {} as {}", base_url, identity.email); + Ok(LinkResult { + status, + identity, + compatibility: probe.compatibility, + }) +} + +/// Stop syncing and forget the server. +/// +/// Local only: the device token remains valid on the SERVER until revoked there +/// (Account → Linked devices). We can't reliably revoke it from here — a pasted +/// token arrives without its device id — so the UI must say so rather than imply a +/// remote revoke that didn't happen. Tracked for follow-up. +#[tauri::command] +pub fn sync_unlink(db: State<'_, Db>) -> Result { + let conn = db.0.lock().map_err(|e| e.to_string())?; + state::clear_link(&conn).map_err(|e| e.to_string())?; + log::info!("unlinked from server"); + state::status(&conn).map_err(|e| e.to_string()) +} + +#[tauri::command] +pub fn sync_status(db: State<'_, Db>) -> Result { + let conn = db.0.lock().map_err(|e| e.to_string())?; + state::status(&conn).map_err(|e| e.to_string()) +} diff --git a/desktop/src-tauri/src/sync/mod.rs b/desktop/src-tauri/src/sync/mod.rs index acd26c0..a7d403b 100644 --- a/desktop/src-tauri/src/sync/mod.rs +++ b/desktop/src-tauri/src/sync/mod.rs @@ -3,9 +3,16 @@ //! The app is local-first: `local` is the source of truth and everything works //! unlinked. Nothing in here runs until the user links a server. //! -//! `compat` owns the version/capability handshake (M10.6) that decides whether a -//! given server can be talked to at all. The engine that then moves notes — push, -//! pull, the revision cursor, last-write-wins — lands in M10.7 and consults it. +//! - `compat` — the version/capability handshake (M10.6): whether a given server can +//! be talked to at all. Pure decision logic, no I/O. +//! - `client` — HTTP transport: the handshake call and device-token auth. +//! - `state` — the persisted link record (server, token, change-feed cursor). +//! - `commands` — the Tauri surface the Settings UI drives. +//! +//! The engine that moves notes — push, pull, last-write-wins — lands in M10.7b/c and +//! consults `compat` before it does anything. pub mod client; +pub mod commands; pub mod compat; +pub mod state; diff --git a/desktop/src-tauri/src/sync/state.rs b/desktop/src-tauri/src/sync/state.rs new file mode 100644 index 0000000..c8ddf06 --- /dev/null +++ b/desktop/src-tauri/src/sync/state.rs @@ -0,0 +1,233 @@ +//! The link record: which server this app is paired with, the device token that +//! authenticates to it, and how far it has consumed that server's change feed. +//! +//! One row, enforced by `CHECK (id = 1)` and seeded during migration, so every +//! operation here is an UPDATE — there is no create-or-missing case to handle. +//! +//! The token lives in the app-data SQLite file rather than an OS keyring on purpose: +//! the `keyring` crate needs libsecret/DBus on Linux, which adds a C dependency to a +//! binary that has to cross-compile, and fails outright on headless or minimal-WM +//! setups. Protecting the database file is the portable trade. + +use rusqlite::{params, Connection}; +use serde::Serialize; + +/// The full link record, token included. Internal to the Rust side. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct SyncState { + pub server_url: Option, + pub device_token: Option, + pub last_cursor: i64, +} + +impl SyncState { + /// Linked means BOTH a server and a credential for it. Either one alone is a + /// half-written link that nothing can act on, so it must not read as linked. + pub fn is_linked(&self) -> bool { + self.server_url.is_some() && self.device_token.is_some() + } +} + +/// What the UI is allowed to see. +/// +/// Deliberately has no `device_token` field: this crosses into the webview, and a +/// long-lived bearer token has no business being reachable from page scripts. +#[derive(Debug, Clone, Serialize, PartialEq, Eq)] +pub struct Status { + pub linked: bool, + pub server_url: Option, + pub last_cursor: i64, +} + +impl From<&SyncState> for Status { + fn from(s: &SyncState) -> Self { + Status { + linked: s.is_linked(), + server_url: s.server_url.clone(), + last_cursor: s.last_cursor, + } + } +} + +/// Treat a blank string as absent, so a half-cleared row can't masquerade as linked. +fn present(value: Option) -> Option { + value.filter(|s| !s.trim().is_empty()) +} + +pub fn read(conn: &Connection) -> rusqlite::Result { + conn.query_row( + "SELECT server_url, device_token, last_cursor FROM sync_state WHERE id = 1", + [], + |row| { + let cursor: Option = row.get(2)?; + Ok(SyncState { + server_url: present(row.get(0)?), + device_token: present(row.get(1)?), + // Stored TEXT (schema) but used as an integer watermark. Absent or + // unparseable means "start from the beginning" — always the safe + // reading, because a redundant full sync costs time, never data, + // whereas a too-high cursor silently skips changes. + last_cursor: cursor.and_then(|c| c.trim().parse().ok()).unwrap_or(0), + }) + }, + ) +} + +/// Record a link. +/// +/// Resets the change-feed cursor whenever the server differs from the one previously +/// linked. A cursor is only meaningful against the server that issued it; carrying +/// one across would silently skip every change on the new server below that +/// watermark — data loss that looks like a successful sync. Re-linking the SAME +/// server (after a token refresh, say) keeps the cursor, so a routine re-auth doesn't +/// force a full re-download. +pub fn set_link(conn: &Connection, server_url: &str, device_token: &str) -> rusqlite::Result<()> { + let keep_cursor = read(conn)?.server_url.as_deref() == Some(server_url); + conn.execute( + "UPDATE sync_state + SET server_url = ?1, + device_token = ?2, + last_cursor = CASE WHEN ?3 THEN last_cursor ELSE NULL END + WHERE id = 1", + params![server_url, device_token, keep_cursor], + )?; + Ok(()) +} + +/// Forget the server entirely. +/// +/// Clears the cursor as well as the credentials: a cursor left behind would, on the +/// next link, be interpreted against a server that never issued it. +pub fn clear_link(conn: &Connection) -> rusqlite::Result<()> { + conn.execute( + "UPDATE sync_state + SET server_url = NULL, device_token = NULL, last_cursor = NULL + WHERE id = 1", + [], + )?; + Ok(()) +} + +/// Advance the consumed-change watermark. Called by the pull loop (M10.7b) only +/// after a page has been fully applied. +pub fn set_cursor(conn: &Connection, cursor: i64) -> rusqlite::Result<()> { + conn.execute( + "UPDATE sync_state SET last_cursor = ?1 WHERE id = 1", + params![cursor.to_string()], + )?; + Ok(()) +} + +pub fn status(conn: &Connection) -> rusqlite::Result { + Ok(Status::from(&read(conn)?)) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::local::schema; + + fn db() -> Connection { + let conn = Connection::open_in_memory().expect("in-memory db"); + schema::migrate(&conn).expect("migrate"); + conn + } + + #[test] + fn fresh_store_is_unlinked() { + let conn = db(); + let state = read(&conn).expect("read"); + assert_eq!(state, SyncState::default()); + assert!(!state.is_linked()); + assert_eq!(state.last_cursor, 0); + } + + #[test] + fn link_round_trips() { + let conn = db(); + set_link(&conn, "https://notes.example.com", "tok-1").expect("link"); + let state = read(&conn).expect("read"); + assert!(state.is_linked()); + assert_eq!(state.server_url.as_deref(), Some("https://notes.example.com")); + assert_eq!(state.device_token.as_deref(), Some("tok-1")); + } + + #[test] + fn relinking_the_same_server_keeps_the_cursor() { + let conn = db(); + set_link(&conn, "https://a.example.com", "tok-1").expect("link"); + set_cursor(&conn, 4242).expect("cursor"); + // e.g. the token was revoked and the user re-authenticated. + set_link(&conn, "https://a.example.com", "tok-2").expect("relink"); + let state = read(&conn).expect("read"); + assert_eq!(state.last_cursor, 4242, "a re-auth shouldn't force a full re-sync"); + assert_eq!(state.device_token.as_deref(), Some("tok-2")); + } + + #[test] + fn linking_a_different_server_resets_the_cursor() { + let conn = db(); + set_link(&conn, "https://a.example.com", "tok-1").expect("link"); + set_cursor(&conn, 4242).expect("cursor"); + set_link(&conn, "https://b.example.com", "tok-2").expect("relink"); + assert_eq!( + read(&conn).expect("read").last_cursor, + 0, + "a cursor from another server would skip everything below it" + ); + } + + #[test] + fn unlink_clears_the_cursor_too() { + let conn = db(); + set_link(&conn, "https://a.example.com", "tok-1").expect("link"); + set_cursor(&conn, 99).expect("cursor"); + clear_link(&conn).expect("unlink"); + let state = read(&conn).expect("read"); + assert!(!state.is_linked()); + assert_eq!(state.last_cursor, 0); + assert!(state.server_url.is_none()); + assert!(state.device_token.is_none()); + } + + #[test] + fn half_written_link_is_not_linked() { + let conn = db(); + conn.execute( + "UPDATE sync_state SET server_url = 'https://a.example.com' WHERE id = 1", + [], + ) + .expect("partial write"); + assert!(!read(&conn).expect("read").is_linked()); + } + + #[test] + fn blank_strings_count_as_absent() { + let conn = db(); + conn.execute( + "UPDATE sync_state SET server_url = ' ', device_token = '' WHERE id = 1", + [], + ) + .expect("blank write"); + let state = read(&conn).expect("read"); + assert!(!state.is_linked()); + assert!(state.server_url.is_none()); + } + + #[test] + fn unparseable_cursor_falls_back_to_a_full_sync() { + let conn = db(); + conn.execute("UPDATE sync_state SET last_cursor = 'garbage' WHERE id = 1", []) + .expect("bad cursor"); + assert_eq!(read(&conn).expect("read").last_cursor, 0); + } + + #[test] + fn status_never_carries_the_token() { + let conn = db(); + set_link(&conn, "https://a.example.com", "super-secret").expect("link"); + let json = serde_json::to_string(&status(&conn).expect("status")).expect("serialize"); + assert!(!json.contains("super-secret"), "token leaked to the webview: {json}"); + assert!(json.contains("\"linked\":true"), "got {json}"); + } +} -- 2.54.0 From 7d9a6509f38cc3fbf7b8ae39f9017d7fc791556e Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sat, 25 Jul 2026 23:38:49 -0400 Subject: [PATCH 08/86] desktop: rustfmt the M10.7a state tests Four macro-argument splits, applied verbatim from run 2895's cargo fmt --check diff. No logic change. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01SreJkbxB4gx8pPsu8QbLPi --- desktop/src-tauri/src/sync/state.rs | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/desktop/src-tauri/src/sync/state.rs b/desktop/src-tauri/src/sync/state.rs index c8ddf06..ba90c4f 100644 --- a/desktop/src-tauri/src/sync/state.rs +++ b/desktop/src-tauri/src/sync/state.rs @@ -148,7 +148,10 @@ mod tests { set_link(&conn, "https://notes.example.com", "tok-1").expect("link"); let state = read(&conn).expect("read"); assert!(state.is_linked()); - assert_eq!(state.server_url.as_deref(), Some("https://notes.example.com")); + assert_eq!( + state.server_url.as_deref(), + Some("https://notes.example.com") + ); assert_eq!(state.device_token.as_deref(), Some("tok-1")); } @@ -160,7 +163,10 @@ mod tests { // e.g. the token was revoked and the user re-authenticated. set_link(&conn, "https://a.example.com", "tok-2").expect("relink"); let state = read(&conn).expect("read"); - assert_eq!(state.last_cursor, 4242, "a re-auth shouldn't force a full re-sync"); + assert_eq!( + state.last_cursor, 4242, + "a re-auth shouldn't force a full re-sync" + ); assert_eq!(state.device_token.as_deref(), Some("tok-2")); } @@ -217,8 +223,11 @@ mod tests { #[test] fn unparseable_cursor_falls_back_to_a_full_sync() { let conn = db(); - conn.execute("UPDATE sync_state SET last_cursor = 'garbage' WHERE id = 1", []) - .expect("bad cursor"); + conn.execute( + "UPDATE sync_state SET last_cursor = 'garbage' WHERE id = 1", + [], + ) + .expect("bad cursor"); assert_eq!(read(&conn).expect("read").last_cursor, 0); } @@ -227,7 +236,10 @@ mod tests { let conn = db(); set_link(&conn, "https://a.example.com", "super-secret").expect("link"); let json = serde_json::to_string(&status(&conn).expect("status")).expect("serialize"); - assert!(!json.contains("super-secret"), "token leaked to the webview: {json}"); + assert!( + !json.contains("super-secret"), + "token leaked to the webview: {json}" + ); assert!(json.contains("\"linked\":true"), "got {json}"); } } -- 2.54.0 From dc8b2d360d20ca65a327f715eff47c272c0c33ac Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 26 Jul 2026 00:05:27 -0400 Subject: [PATCH 09/86] M10.7b: pull the change feed into the local store (task 2105) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Server -> local. sync/wire.rs mirrors the delta-feed JSON exactly as notes/serialize.py sends it; sync/pull.rs applies it. ATOMICITY IS THE POINT. The cursor is written in the SAME transaction as the page it describes. A cursor committed ahead of its data would skip those rows forever while reporting a clean sync — the worst kind of failure, because nothing looks wrong. A test forces a mid-page failure and asserts the cursor stayed put. Every degradation leans toward re-downloading rather than skipping: an unparseable cursor means full sync, wire fields are all defaulted so a newer server adding a field (or an older one omitting one) yields a partial note instead of a rejected page, and a page that fails rolls back whole. Labels are applied before notes so a membership never references a row that doesn't exist. A note also carries enough of its labels to materialize them, because notes and labels page from ONE shared sequence and a note can arrive referencing a label whose own delta landed in an earlier page. via_tag is applied verbatim rather than re-deriving #tags from the body. The server already reconciled them on save, and re-deriving would go through the local find-or-create path, which marks new labels dirty — pushing them straight back. Sync churn manufactured out of nothing. Duplicate-label merge, the subtle one: a label created offline can collide by name with one the server already had under a different id. Both sides enforce one label per name, so the server's row has to win — but simply deleting the local duplicate would CASCADE its note_labels away, stripping the label off notes this pull never mentions, with no later page to repair it. So we free the name, insert the server's row, re-point the memberships, then drop the husk. Tested. Children (items/attachments/previews/labels) are replaced wholesale rather than diffed: a delta carries the note's FULL state, so what arrived IS the complete set, and diffing could strand a row the server no longer has. The loop trusts the data over the flag — a server claiming has_more without advancing its cursor stops with an error instead of spinning forever. Pull can overwrite a row with unpushed local edits. The documented cycle is push-then-pull (M10.7c), so that should never happen; when it does it's counted as clobbered_dirty and logged rather than hidden. 17 tests, all against an in-memory database. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01SreJkbxB4gx8pPsu8QbLPi --- desktop/src-tauri/src/lib.rs | 1 + desktop/src-tauri/src/sync/client.rs | 56 +- desktop/src-tauri/src/sync/commands.rs | 24 + desktop/src-tauri/src/sync/mod.rs | 2 + desktop/src-tauri/src/sync/pull.rs | 699 +++++++++++++++++++++++++ desktop/src-tauri/src/sync/wire.rs | 161 ++++++ 6 files changed, 938 insertions(+), 5 deletions(-) create mode 100644 desktop/src-tauri/src/sync/pull.rs create mode 100644 desktop/src-tauri/src/sync/wire.rs diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index dfe1352..3b176cc 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -93,6 +93,7 @@ pub fn run() { sync::commands::sync_link, sync::commands::sync_unlink, sync::commands::sync_status, + sync::commands::sync_pull, ]) .run(tauri::generate_context!()) .expect("error while running the ThoughtSync desktop app"); diff --git a/desktop/src-tauri/src/sync/client.rs b/desktop/src-tauri/src/sync/client.rs index b75fc6c..bf68a84 100644 --- a/desktop/src-tauri/src/sync/client.rs +++ b/desktop/src-tauri/src/sync/client.rs @@ -14,6 +14,7 @@ use reqwest::{RequestBuilder, StatusCode}; use serde::{Deserialize, Serialize}; use super::compat::{self, Compatibility, ServerInfo}; +use super::wire; /// Timeout for the short request/response calls in this module. Kept tight because a /// user is watching a button while they run, and the most common mistake — a wrong @@ -21,6 +22,15 @@ use super::compat::{self, Compatibility, ServerInfo}; /// just look frozen. The sync engine's bulk transfers will need their own, longer one. const REQUEST_TIMEOUT: Duration = Duration::from_secs(10); +/// Bulk transfers get much longer: a first full sync can be thousands of notes, and +/// failing one at ten seconds would make a large store impossible to ever pull. +const SYNC_TIMEOUT: Duration = Duration::from_secs(120); + +/// Shared by every call that presents a token, so a revoked one reads the same way +/// wherever it surfaces. +const TOKEN_REJECTED: &str = "This server rejected the device token — it may have been \ + revoked. Unlink and link again to issue a new one."; + /// What the link UI needs after a handshake: where we ended up (the normalized URL, /// which may differ from what was typed), who answered, and whether we can work /// with them. @@ -48,13 +58,17 @@ struct DeviceLoginResponse { user: Identity, } -fn http() -> Result { +fn http_with(timeout: Duration) -> Result { reqwest::Client::builder() - .timeout(REQUEST_TIMEOUT) + .timeout(timeout) .build() .map_err(|e| format!("Could not start the network client: {e}")) } +fn http() -> Result { + http_with(REQUEST_TIMEOUT) +} + /// Attach the client-identity headers every request carries, plus a bearer token /// when we hold one. fn prepare(builder: RequestBuilder, token: Option<&str>) -> RequestBuilder { @@ -179,6 +193,36 @@ pub async fn fetch_identity(base_url: &str, token: &str) -> Result Result { + let url = format!("{base_url}/api/sync/changes?since={since}"); + let request = prepare(http_with(SYNC_TIMEOUT)?.get(url), Some(token)); + let response = request + .send() + .await + .map_err(|e| describe_transport_error(base_url, &e))?; + + let status = response.status(); + if status == StatusCode::UNAUTHORIZED { + return Err(TOKEN_REJECTED.to_string()); + } + if !status.is_success() { + return Err(unexpected_status(base_url, status)); + } + + response + .json() + .await + .map_err(|e| format!("Couldn't read the change feed from {base_url}: {e}")) +} + /// The public, unauthenticated endpoint carrying the handshake. fn config_url(base_url: &str) -> String { format!("{base_url}/api/config") @@ -196,10 +240,12 @@ fn me_url(base_url: &str) -> String { /// Display is accurate but reads like a stack trace. fn describe_transport_error(base_url: &str, err: &reqwest::Error) -> String { if err.is_timeout() { + // No specific duration here: these calls run under two different budgets + // (interactive vs bulk sync), and naming the wrong one is worse than naming + // none. format!( - "{base_url} didn't respond within {} seconds. It may be offline, or \ - unreachable from this network.", - REQUEST_TIMEOUT.as_secs() + "{base_url} didn't respond in time. It may be offline, or unreachable \ + from this network." ) } else if err.is_connect() { format!( diff --git a/desktop/src-tauri/src/sync/commands.rs b/desktop/src-tauri/src/sync/commands.rs index 1d7a0c9..b7cbf2f 100644 --- a/desktop/src-tauri/src/sync/commands.rs +++ b/desktop/src-tauri/src/sync/commands.rs @@ -9,6 +9,7 @@ use tauri::State; use crate::local::Db; use crate::sync::client::{self, Identity, ProbeResult}; use crate::sync::compat::Compatibility; +use crate::sync::pull; use crate::sync::state; /// Ask a server who it is, without committing to anything. The UI calls this as the @@ -122,3 +123,26 @@ pub fn sync_status(db: State<'_, Db>) -> Result { let conn = db.0.lock().map_err(|e| e.to_string())?; state::status(&conn).map_err(|e| e.to_string()) } + +/// The server URL + token, or a plain "not linked" error. Every networked sync +/// command needs exactly this, and none of them may hold the lock past it. +fn credentials(db: &State<'_, Db>) -> Result<(String, String), String> { + let conn = db.0.lock().map_err(|e| e.to_string())?; + let current = state::read(&conn).map_err(|e| e.to_string())?; + match (current.server_url, current.device_token) { + (Some(url), Some(token)) => Ok((url, token)), + _ => Err("This app isn't linked to a server yet.".to_string()), + } +} + +/// Pull the server's changes into the local store. +/// +/// Standalone for now; M10.7c wraps push-then-pull into a single `sync_now`, which +/// is the ordering the protocol assumes. Run alone against unpushed local edits, the +/// server's version lands on top of them — the returned `clobbered_dirty` count +/// reports that rather than hiding it. +#[tauri::command] +pub async fn sync_pull(db: State<'_, Db>) -> Result { + let (base_url, token) = credentials(&db)?; + pull::run(db.inner(), &base_url, &token).await +} diff --git a/desktop/src-tauri/src/sync/mod.rs b/desktop/src-tauri/src/sync/mod.rs index a7d403b..b95d473 100644 --- a/desktop/src-tauri/src/sync/mod.rs +++ b/desktop/src-tauri/src/sync/mod.rs @@ -15,4 +15,6 @@ pub mod client; pub mod commands; pub mod compat; +pub mod pull; pub mod state; +pub mod wire; diff --git a/desktop/src-tauri/src/sync/pull.rs b/desktop/src-tauri/src/sync/pull.rs new file mode 100644 index 0000000..f6c026a --- /dev/null +++ b/desktop/src-tauri/src/sync/pull.rs @@ -0,0 +1,699 @@ +//! Pull: bring a server's changes into the local store (M10.7b). +//! +//! The feed is a single monotonic sequence shared by notes and labels, so one +//! integer cursor is a total-order watermark over both (docs/sync.md). We loop pages +//! until the server says there are no more, persisting the cursor **in the same +//! transaction** as the page it describes — a cursor committed ahead of its data +//! would silently skip those rows forever, which reads as a clean sync. + +use chrono::{SecondsFormat, Utc}; +use rusqlite::{params, Connection, OptionalExtension}; +use serde::Serialize; + +use super::client; +use super::state; +use super::wire; +use crate::local::Db; + +/// Backstop against a server that never stops saying `has_more`. At the server's +/// 1000-row page cap this is 10M rows — far past any real store, so hitting it means +/// something is wrong, not that someone has a lot of notes. +const MAX_PAGES: usize = 10_000; + +/// What a pull did — for the UI, and for the log when something looks off. +#[derive(Debug, Clone, Default, Serialize, PartialEq, Eq)] +pub struct PullSummary { + pub pages: usize, + pub notes_applied: usize, + pub notes_deleted: usize, + pub labels_applied: usize, + pub labels_deleted: usize, + pub cursor: i64, + /// Rows that still held unpushed local edits when the server's version landed on + /// top. Should be 0 in the normal cycle, because push runs first; anything higher + /// means local work was overwritten, which is worth saying out loud. + pub clobbered_dirty: usize, +} + +impl PullSummary { + fn absorb(&mut self, other: PullSummary) { + self.pages += other.pages; + self.notes_applied += other.notes_applied; + self.notes_deleted += other.notes_deleted; + self.labels_applied += other.labels_applied; + self.labels_deleted += other.labels_deleted; + self.clobbered_dirty += other.clobbered_dirty; + self.cursor = other.cursor; + } +} + +fn now() -> String { + Utc::now().to_rfc3339_opts(SecondsFormat::Millis, true) +} + +/// Apply one page and advance the cursor, atomically. +/// +/// Labels are applied before notes so a membership never references a label row that +/// doesn't exist yet. +pub fn apply_page(conn: &Connection, page: &wire::ChangesPage) -> rusqlite::Result { + let tx = conn.unchecked_transaction()?; + let mut summary = PullSummary { + pages: 1, + cursor: page.cursor, + ..Default::default() + }; + + for label in &page.labels { + if label.is_tombstone() { + tx.execute("DELETE FROM labels WHERE id = ?1", params![label.id])?; + summary.labels_deleted += 1; + } else { + upsert_label(&tx, label)?; + summary.labels_applied += 1; + } + } + + for note in &page.notes { + if note.is_tombstone() { + // A purge tombstone carries no content — its only job is to say "delete + // your copy". Children go with it via ON DELETE CASCADE. + tx.execute("DELETE FROM notes WHERE id = ?1", params![note.id])?; + summary.notes_deleted += 1; + continue; + } + if is_dirty(&tx, ¬e.id)? { + summary.clobbered_dirty += 1; + } + upsert_note(&tx, note)?; + summary.notes_applied += 1; + } + + state::set_cursor(&tx, page.cursor)?; + tx.commit()?; + Ok(summary) +} + +fn is_dirty(conn: &Connection, note_id: &str) -> rusqlite::Result { + let dirty: Option = conn + .query_row( + "SELECT dirty FROM notes WHERE id = ?1", + params![note_id], + |r| r.get(0), + ) + .optional()?; + Ok(dirty == Some(1)) +} + +fn upsert_label(conn: &Connection, label: &wire::Label) -> rusqlite::Result<()> { + // One label per name is enforced on both sides (locally a UNIQUE index on + // lower(name); on the server, per owner). A label created offline can therefore + // collide with one the server already had under a different id — "work" typed on + // this machine and "work" that already existed. + // + // The server's row wins, but its MEMBERSHIPS have to survive the swap. Just + // deleting the local duplicate would cascade its note_labels away, stripping the + // label off notes that this pull never even mentions — silent loss that no later + // page would repair. So: free the name, insert the server's row, re-point the + // memberships onto it, then drop the husk. + let duplicates: Vec = { + let mut stmt = + conn.prepare("SELECT id FROM labels WHERE lower(name) = lower(?1) AND id <> ?2")?; + let rows = stmt.query_map(params![label.name, label.id], |r| r.get::<_, String>(0))?; + rows.collect::>>()? + }; + // Renaming first is what makes the insert possible at all — the unique index + // would otherwise reject the server's row before anything could be merged. + for old in &duplicates { + conn.execute( + "UPDATE labels SET name = name || ' (superseded ' || id || ')' WHERE id = ?1", + params![old], + )?; + } + + let created = label.created_at.clone().unwrap_or_else(now); + conn.execute( + "INSERT INTO labels (id, name, color, created_at, updated_at, sync_revision, dirty) + VALUES (?1, ?2, ?3, ?4, ?4, ?5, 0) + ON CONFLICT(id) DO UPDATE SET + name = excluded.name, + color = excluded.color, + sync_revision = excluded.sync_revision, + dirty = 0", + params![ + label.id, + label.name, + label.color, + created, + label.sync_revision + ], + )?; + + for old in &duplicates { + // OR IGNORE guards a (note_id, label_id) collision. Today the unique index on + // lower(name) makes that unreachable — two same-name labels can't coexist + // locally — so this is belt-and-braces against that index changing, not a + // case we've seen. Anything it skips cascades away with the husk below, which + // is correct: those are duplicates of a membership that now exists. + conn.execute( + "UPDATE OR IGNORE note_labels SET label_id = ?1 WHERE label_id = ?2", + params![label.id, old], + )?; + conn.execute("DELETE FROM labels WHERE id = ?1", params![old])?; + } + Ok(()) +} + +fn upsert_note(conn: &Connection, note: &wire::Note) -> rusqlite::Result<()> { + let created = note.created_at.clone().unwrap_or_else(now); + let updated = note.updated_at.clone().unwrap_or_else(|| created.clone()); + // `created_at` is deliberately absent from the UPDATE clause: a note's birth time + // never changes, and the server's copy is the same value anyway. + conn.execute( + "INSERT INTO notes (id, title, body, color, kind, position, pinned, archived, + trashed, remind_at, recurrence, created_at, updated_at, + sync_revision, dirty) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, 0) + ON CONFLICT(id) DO UPDATE SET + title = excluded.title, + body = excluded.body, + color = excluded.color, + kind = excluded.kind, + position = excluded.position, + pinned = excluded.pinned, + archived = excluded.archived, + trashed = excluded.trashed, + remind_at = excluded.remind_at, + recurrence = excluded.recurrence, + updated_at = excluded.updated_at, + sync_revision = excluded.sync_revision, + dirty = 0", + params![ + note.id, + note.title, + note.body, + note.color, + note.kind, + note.position, + note.pinned, + note.archived, + note.trashed, + note.remind_at, + note.recurrence, + created, + updated, + note.sync_revision, + ], + )?; + + // Children are replaced wholesale: a delta carries the note's FULL current state, + // so "what the server sent" IS the complete set. Diffing would be more code and + // could leave behind a row the server no longer has. + replace_items(conn, note)?; + replace_attachments(conn, note)?; + replace_previews(conn, note)?; + replace_labels(conn, note)?; + Ok(()) +} + +fn replace_items(conn: &Connection, note: &wire::Note) -> rusqlite::Result<()> { + conn.execute( + "DELETE FROM checklist_items WHERE note_id = ?1", + params![note.id], + )?; + for (index, item) in note.items.iter().enumerate() { + conn.execute( + "INSERT INTO checklist_items (id, note_id, text, checked, position) + VALUES (?1, ?2, ?3, ?4, ?5)", + params![ + item.id, + note.id, + item.text, + item.checked, + position_of(item.position, index) + ], + )?; + } + Ok(()) +} + +fn replace_attachments(conn: &Connection, note: &wire::Note) -> rusqlite::Result<()> { + conn.execute( + "DELETE FROM attachments WHERE note_id = ?1", + params![note.id], + )?; + for (index, att) in note.attachments.iter().enumerate() { + // The feed carries no explicit position for attachments — they arrive in + // creation order, so the index preserves it. + conn.execute( + "INSERT INTO attachments (id, note_id, url, filename, mime, size, sha256, position) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8)", + params![ + att.id, + note.id, + att.url, + att.filename, + att.mime, + att.size, + att.sha256, + index as i64 + ], + )?; + } + Ok(()) +} + +fn replace_previews(conn: &Connection, note: &wire::Note) -> rusqlite::Result<()> { + conn.execute( + "DELETE FROM link_previews WHERE note_id = ?1", + params![note.id], + )?; + for (index, preview) in note.previews.iter().enumerate() { + conn.execute( + "INSERT INTO link_previews (id, note_id, url, title, description, image_url, + site_name, position) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8)", + params![ + preview.id, + note.id, + preview.url, + preview.title, + preview.description, + preview.image_url, + preview.site_name, + index as i64 + ], + )?; + } + Ok(()) +} + +fn replace_labels(conn: &Connection, note: &wire::Note) -> rusqlite::Result<()> { + conn.execute( + "DELETE FROM note_labels WHERE note_id = ?1", + params![note.id], + )?; + for label in ¬e.labels { + ensure_label_stub(conn, label)?; + // `via_tag` is applied verbatim rather than re-derived from the body. The + // server already reconciled tags when it saved the note, and re-deriving here + // would call the local find-or-create path, which marks new labels dirty and + // would push them straight back — sync churn out of nothing. + conn.execute( + "INSERT OR IGNORE INTO note_labels (note_id, label_id, via_tag) + VALUES (?1, ?2, ?3)", + params![note.id, label.id, label.via_tag], + )?; + } + Ok(()) +} + +/// Materialize a label referenced by a note, if we don't have it yet. +/// +/// Notes and labels page from one shared sequence, so a note can reference a label +/// whose own delta landed in an earlier page — or, right at a page boundary, hasn't +/// landed. The note carries enough of the label to create it, so a membership never +/// fails on a missing row. `OR IGNORE` because the label's real delta (later in this +/// page or a future one) is the authority on its name and color. +fn ensure_label_stub(conn: &Connection, label: &wire::NoteLabel) -> rusqlite::Result<()> { + let ts = now(); + conn.execute( + "INSERT OR IGNORE INTO labels (id, name, color, created_at, updated_at, dirty) + VALUES (?1, ?2, ?3, ?4, ?4, 0)", + params![label.id, label.name, label.color, ts], + )?; + Ok(()) +} + +/// Trust an explicit position; fall back to arrival order when the server sent 0 for +/// everything (which is what an unordered list looks like on the wire). +fn position_of(explicit: i64, index: usize) -> i64 { + if explicit > 0 { + explicit + } else { + index as i64 + } +} + +/// Loop the feed to exhaustion, starting from the persisted cursor. +/// +/// NOTE ON ORDERING: the full cycle is push-then-pull (docs/sync.md). Running this +/// against a store with unpushed edits lets the server's version land on top of them +/// — counted as `clobbered_dirty` and logged, rather than hidden. +pub async fn run(db: &Db, base_url: &str, token: &str) -> Result { + let mut total = PullSummary::default(); + + loop { + let since = { + let conn = db.0.lock().map_err(|e| e.to_string())?; + state::read(&conn).map_err(|e| e.to_string())?.last_cursor + }; + + let page = client::fetch_changes(base_url, token, since).await?; + + // Trust the data over the flag: a server that claims more pages without + // advancing the cursor would spin this loop forever. + if page.has_more && page.cursor <= since { + return Err(format!( + "The server reported more changes but its cursor didn't advance past \ + {since}. Stopping rather than looping forever." + )); + } + + let has_more = page.has_more; + let applied = { + let conn = db.0.lock().map_err(|e| e.to_string())?; + apply_page(&conn, &page).map_err(|e| e.to_string())? + }; + total.absorb(applied); + + if !has_more { + break; + } + if total.pages >= MAX_PAGES { + return Err(format!( + "Stopped after {MAX_PAGES} pages without reaching the end of the \ + server's changes. Something is wrong with the feed." + )); + } + } + + if total.clobbered_dirty > 0 { + log::warn!( + "pull overwrote {} note(s) that still had unpushed local edits", + total.clobbered_dirty + ); + } + log::info!( + "pull complete: {} page(s), {} note(s) applied, {} deleted, {} label(s) applied, cursor {}", + total.pages, + total.notes_applied, + total.notes_deleted, + total.labels_applied, + total.cursor + ); + Ok(total) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::local::schema; + + fn db() -> Connection { + let conn = Connection::open_in_memory().expect("in-memory db"); + schema::migrate(&conn).expect("migrate"); + conn + } + + fn note(id: &str, revision: i64) -> wire::Note { + wire::Note { + id: id.to_string(), + title: Some("Title".into()), + body: "Body".into(), + color: "default".into(), + kind: "text".into(), + position: 0, + pinned: false, + archived: false, + trashed: false, + remind_at: None, + recurrence: None, + created_at: Some("2026-07-26T00:00:00.000Z".into()), + updated_at: Some("2026-07-26T00:00:00.000Z".into()), + sync_revision: revision, + purged_at: None, + labels: vec![], + items: vec![], + attachments: vec![], + previews: vec![], + } + } + + fn page(notes: Vec, labels: Vec, cursor: i64) -> wire::ChangesPage { + wire::ChangesPage { + notes, + labels, + cursor, + has_more: false, + } + } + + fn count(conn: &Connection, sql: &str) -> i64 { + conn.query_row(sql, [], |r| r.get(0)).expect("count") + } + + #[test] + fn applies_a_note_and_advances_the_cursor() { + let conn = db(); + let summary = apply_page(&conn, &page(vec![note("n1", 7)], vec![], 7)).expect("apply"); + assert_eq!(summary.notes_applied, 1); + assert_eq!(count(&conn, "SELECT COUNT(*) FROM notes"), 1); + assert_eq!(state::read(&conn).expect("state").last_cursor, 7); + } + + #[test] + fn pulled_rows_are_not_dirty() { + // They came FROM the server, so pushing them back would be pure churn. + let conn = db(); + apply_page(&conn, &page(vec![note("n1", 1)], vec![], 1)).expect("apply"); + assert_eq!(count(&conn, "SELECT dirty FROM notes WHERE id = 'n1'"), 0); + } + + #[test] + fn tombstone_deletes_the_local_note() { + let conn = db(); + apply_page(&conn, &page(vec![note("n1", 1)], vec![], 1)).expect("apply"); + let mut dead = note("n1", 2); + dead.purged_at = Some("2026-07-26T01:00:00.000Z".into()); + let summary = apply_page(&conn, &page(vec![dead], vec![], 2)).expect("apply"); + assert_eq!(summary.notes_deleted, 1); + assert_eq!(count(&conn, "SELECT COUNT(*) FROM notes"), 0); + } + + #[test] + fn trashed_is_not_a_tombstone() { + // `trashed` is ordinary state that keeps syncing; only `purged_at` deletes. + let conn = db(); + let mut trashed = note("n1", 1); + trashed.trashed = true; + apply_page(&conn, &page(vec![trashed], vec![], 1)).expect("apply"); + assert_eq!(count(&conn, "SELECT COUNT(*) FROM notes"), 1); + assert_eq!(count(&conn, "SELECT trashed FROM notes WHERE id = 'n1'"), 1); + } + + #[test] + fn children_are_replaced_not_merged() { + let conn = db(); + let mut first = note("n1", 1); + first.items = vec![ + wire::Item { + id: "i1".into(), + text: "one".into(), + checked: false, + position: 0, + }, + wire::Item { + id: "i2".into(), + text: "two".into(), + checked: false, + position: 1, + }, + ]; + apply_page(&conn, &page(vec![first], vec![], 1)).expect("apply"); + assert_eq!(count(&conn, "SELECT COUNT(*) FROM checklist_items"), 2); + + // The server dropped an item; the local copy must drop it too. + let mut second = note("n1", 2); + second.items = vec![wire::Item { + id: "i1".into(), + text: "one".into(), + checked: true, + position: 0, + }]; + apply_page(&conn, &page(vec![second], vec![], 2)).expect("apply"); + assert_eq!(count(&conn, "SELECT COUNT(*) FROM checklist_items"), 1); + } + + #[test] + fn note_label_membership_materializes_a_missing_label() { + // The label's own delta may have landed in an earlier page, or not yet. + let conn = db(); + let mut n = note("n1", 1); + n.labels = vec![wire::NoteLabel { + id: "l1".into(), + name: "work".into(), + color: "blue".into(), + via_tag: true, + }]; + apply_page(&conn, &page(vec![n], vec![], 1)).expect("apply"); + assert_eq!(count(&conn, "SELECT COUNT(*) FROM labels"), 1); + assert_eq!( + count(&conn, "SELECT via_tag FROM note_labels WHERE note_id = 'n1'"), + 1, + "via_tag is applied verbatim, not re-derived" + ); + } + + #[test] + fn server_label_replaces_a_local_duplicate_by_name() { + let conn = db(); + conn.execute( + "INSERT INTO labels (id, name, color, created_at, updated_at, dirty) + VALUES ('local-id', 'Work', 'default', '2026-01-01', '2026-01-01', 1)", + [], + ) + .expect("seed local label"); + + let server = wire::Label { + id: "server-id".into(), + name: "work".into(), + color: "blue".into(), + sync_revision: 5, + purged_at: None, + created_at: Some("2026-07-26T00:00:00.000Z".into()), + }; + apply_page(&conn, &page(vec![], vec![server], 5)).expect("apply"); + + assert_eq!(count(&conn, "SELECT COUNT(*) FROM labels"), 1); + let id: String = conn + .query_row("SELECT id FROM labels", [], |r| r.get(0)) + .expect("label"); + assert_eq!(id, "server-id", "the server's row wins on pull"); + } + + #[test] + fn merging_a_duplicate_label_keeps_its_note_memberships() { + // The notes carrying the local label may not be in this page at all, so a + // plain delete would strip the label off them with nothing to repair it. + let conn = db(); + apply_page(&conn, &page(vec![note("n1", 1)], vec![], 1)).expect("seed note"); + conn.execute( + "INSERT INTO labels (id, name, color, created_at, updated_at, dirty) + VALUES ('local-id', 'Work', 'default', '2026-01-01', '2026-01-01', 1)", + [], + ) + .expect("seed local label"); + conn.execute( + "INSERT INTO note_labels (note_id, label_id, via_tag) + VALUES ('n1', 'local-id', 0)", + [], + ) + .expect("seed membership"); + + let server = wire::Label { + id: "server-id".into(), + name: "work".into(), + color: "blue".into(), + sync_revision: 5, + purged_at: None, + created_at: None, + }; + apply_page(&conn, &page(vec![], vec![server], 5)).expect("apply"); + + assert_eq!(count(&conn, "SELECT COUNT(*) FROM labels"), 1); + let label_id: String = conn + .query_row("SELECT label_id FROM note_labels WHERE note_id = 'n1'", [], |r| r.get(0)) + .expect("membership survived"); + assert_eq!(label_id, "server-id", "membership re-pointed, not dropped"); + } + + + #[test] + fn label_tombstone_deletes_and_cascades_memberships() { + let conn = db(); + let mut n = note("n1", 1); + n.labels = vec![wire::NoteLabel { + id: "l1".into(), + name: "work".into(), + color: "blue".into(), + via_tag: false, + }]; + apply_page(&conn, &page(vec![n], vec![], 1)).expect("apply"); + assert_eq!(count(&conn, "SELECT COUNT(*) FROM note_labels"), 1); + + let dead = wire::Label { + id: "l1".into(), + name: "work".into(), + color: "blue".into(), + sync_revision: 2, + purged_at: Some("2026-07-26T01:00:00.000Z".into()), + created_at: None, + }; + apply_page(&conn, &page(vec![], vec![dead], 2)).expect("apply"); + assert_eq!(count(&conn, "SELECT COUNT(*) FROM labels"), 0); + assert_eq!( + count(&conn, "SELECT COUNT(*) FROM note_labels"), + 0, + "membership should cascade with the label" + ); + } + + #[test] + fn overwriting_a_dirty_note_is_counted() { + let conn = db(); + conn.execute( + "INSERT INTO notes (id, body, created_at, updated_at, dirty) + VALUES ('n1', 'local edit', '2026-01-01', '2026-01-01', 1)", + [], + ) + .expect("seed dirty note"); + let summary = apply_page(&conn, &page(vec![note("n1", 9)], vec![], 9)).expect("apply"); + assert_eq!(summary.clobbered_dirty, 1); + } + + #[test] + fn applying_a_fresh_note_reports_no_clobber() { + let conn = db(); + let summary = apply_page(&conn, &page(vec![note("n1", 1)], vec![], 1)).expect("apply"); + assert_eq!(summary.clobbered_dirty, 0); + } + + #[test] + fn empty_page_still_advances_the_cursor() { + // The server can page past rows that were trimmed to the shared watermark. + let conn = db(); + apply_page(&conn, &page(vec![], vec![], 42)).expect("apply"); + assert_eq!(state::read(&conn).expect("state").last_cursor, 42); + } + + #[test] + fn note_upsert_preserves_the_original_created_at() { + let conn = db(); + apply_page(&conn, &page(vec![note("n1", 1)], vec![], 1)).expect("apply"); + let mut later = note("n1", 2); + later.created_at = Some("2099-01-01T00:00:00.000Z".into()); + apply_page(&conn, &page(vec![later], vec![], 2)).expect("apply"); + let created: String = conn + .query_row("SELECT created_at FROM notes WHERE id = 'n1'", [], |r| { + r.get(0) + }) + .expect("created_at"); + assert_eq!(created, "2026-07-26T00:00:00.000Z"); + } + + #[test] + fn a_page_that_fails_leaves_the_cursor_untouched() { + // Atomicity is the whole resumability story: a cursor committed ahead of its + // data would skip those rows forever. Force a failure with a duplicate + // checklist-item id inside one page. + let conn = db(); + let mut n = note("n1", 3); + n.items = vec![ + wire::Item { + id: "dup".into(), + text: "one".into(), + checked: false, + position: 0, + }, + wire::Item { + id: "dup".into(), + text: "two".into(), + checked: false, + position: 1, + }, + ]; + assert!(apply_page(&conn, &page(vec![n], vec![], 3)).is_err()); + assert_eq!(state::read(&conn).expect("state").last_cursor, 0); + assert_eq!(count(&conn, "SELECT COUNT(*) FROM notes"), 0); + } +} diff --git a/desktop/src-tauri/src/sync/wire.rs b/desktop/src-tauri/src/sync/wire.rs new file mode 100644 index 0000000..9ee8ad8 --- /dev/null +++ b/desktop/src-tauri/src/sync/wire.rs @@ -0,0 +1,161 @@ +//! The delta-feed JSON shapes, exactly as `GET /api/sync/changes` sends them. +//! +//! Mirrors the server's serializers (`notes/serialize.py` + `serialize.py`) — see +//! `docs/sync.md` for the contract. Every field is `#[serde(default)]` or `Option` +//! so a NEWER server adding fields, or an older one omitting one, degrades to a +//! partial note rather than failing the whole page. Losing one attribute is +//! recoverable; refusing a page stalls sync permanently at that cursor. + +use serde::Deserialize; + +#[derive(Debug, Clone, Deserialize, Default)] +pub struct ChangesPage { + #[serde(default)] + pub notes: Vec, + #[serde(default)] + pub labels: Vec