From fbbe877c460bbdcb1e26a998ca2c557b8e61d213 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sat, 25 Jul 2026 22:40:25 -0400 Subject: [PATCH] =?UTF-8?q?M10.6:=20client=E2=86=94server=20sync=20protoco?= =?UTF-8?q?l=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)