From c40916699b135c37f6b7bfb9faba0349b0abbf6a Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 31 Aug 2026 08:40:01 -0400 Subject: [PATCH] sync: the client header said "desktop" from every phone, and named the wrong version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `client_headers()` built `thoughtsync-desktop/{CARGO_PKG_VERSION}`, and both halves were wrong. This crate is compiled into the Android app as well as the desktop one, so every phone in the field announced itself as a desktop. And CARGO_PKG_VERSION here is the CORE crate's version — a number no build stamps and no user has ever seen — where the thing a reader of that header wants is the app's own build (note 3127 §5: with no version tags, the artifact's self-report is the only answer to "which build is this?"). The core cannot know either value, so the host says them. `set_client_agent` is a OnceLock the desktop fills in `run()` and Android fills in `ThoughtSyncApplication.onCreate`, before anything can sync. A host that never introduces itself sends `thoughtsync-unidentified/unknown` rather than a plausible default: nothing reads this header today, which is exactly why a wrong value could sit in it for months — the first person to look at a server log is the first who could catch it, and only if what they see is obviously a host that never said who it was. Android's version comes from the INSTALLED package, through a new `Context.installedVersionName()` that the foot of the Sync screen now shares. One answer to "which build is on this phone", so the line a person quotes in a bug report and the line in the server's log cannot disagree. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01K3MMqUtzX1TJgA1oypvm1c --- .../fabledsword/thoughtsync/BuildIdentity.kt | 21 ++++++++ .../thoughtsync/ThoughtSyncApplication.kt | 9 ++++ .../fabledsword/thoughtsync/ui/SyncScreen.kt | 16 +++--- android/ffi/src/lib.rs | 14 ++++++ core/src/sync/compat.rs | 50 ++++++++++++++++++- desktop/src-tauri/src/lib.rs | 6 +++ 6 files changed, 104 insertions(+), 12 deletions(-) create mode 100644 android/app/src/main/java/com/fabledsword/thoughtsync/BuildIdentity.kt diff --git a/android/app/src/main/java/com/fabledsword/thoughtsync/BuildIdentity.kt b/android/app/src/main/java/com/fabledsword/thoughtsync/BuildIdentity.kt new file mode 100644 index 0000000..1ad8ae0 --- /dev/null +++ b/android/app/src/main/java/com/fabledsword/thoughtsync/BuildIdentity.kt @@ -0,0 +1,21 @@ +package com.fabledsword.thoughtsync + +import android.content.Context + +/** + * The `versionName` of the INSTALLED package, or null when it cannot be read. + * + * From the package manager rather than from `BuildConfig`: this reports what is + * actually on the phone, which is the question both callers are asking — a bug + * report reading the foot of Sync, and a server log reading the client header. It + * also needs no `buildFeatures.buildConfig`, which this module does not enable. + * + * Returns null rather than a fallback string, because the two callers want + * different ones: the UI wants a localized "unknown" from string resources, the + * client header wants the literal the core recognizes. Note 3127 §5 governs both — + * with no version tags, the artifact's self-report is the only answer to "which + * build is this?", so a missing name must read as missing and never as a plausible + * default that nothing can contradict. + */ +fun Context.installedVersionName(): String? = + runCatching { packageManager.getPackageInfo(packageName, 0).versionName }.getOrNull() diff --git a/android/app/src/main/java/com/fabledsword/thoughtsync/ThoughtSyncApplication.kt b/android/app/src/main/java/com/fabledsword/thoughtsync/ThoughtSyncApplication.kt index 1303297..ad7f2bc 100644 --- a/android/app/src/main/java/com/fabledsword/thoughtsync/ThoughtSyncApplication.kt +++ b/android/app/src/main/java/com/fabledsword/thoughtsync/ThoughtSyncApplication.kt @@ -3,6 +3,7 @@ package com.fabledsword.thoughtsync import android.app.Application import android.util.Log import com.fabledsword.thoughtsync.core.ThoughtSync +import com.fabledsword.thoughtsync.core.setClientAgent /** * Opens the shared Rust core once, for the process lifetime. @@ -30,6 +31,14 @@ class ThoughtSyncApplication : Application() { override fun onCreate() { super.onCreate() + + // Introduce this app to any server it links to, before anything can sync. + // The core cannot name us — the same crate is compiled into the desktop app, + // and it used to announce every phone as `thoughtsync-desktop` carrying the + // core crate's own version. "unknown" rather than a guess when the package + // manager will not say (note 3127 §5). + setClientAgent("thoughtsync-android", installedVersionName() ?: "unknown") + try { val handle = ThoughtSync(filesDir.absolutePath) core = handle diff --git a/android/app/src/main/java/com/fabledsword/thoughtsync/ui/SyncScreen.kt b/android/app/src/main/java/com/fabledsword/thoughtsync/ui/SyncScreen.kt index be5d857..52daadc 100644 --- a/android/app/src/main/java/com/fabledsword/thoughtsync/ui/SyncScreen.kt +++ b/android/app/src/main/java/com/fabledsword/thoughtsync/ui/SyncScreen.kt @@ -38,6 +38,7 @@ import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.unit.dp import com.fabledsword.thoughtsync.R import com.fabledsword.thoughtsync.UpdateOutcome +import com.fabledsword.thoughtsync.installedVersionName /** * Opt-in server pairing. @@ -131,25 +132,20 @@ fun SyncScreen( * The build, dim, at the foot of Sync — the same thing the web UI puts at the * bottom of its rail (#3181). * - * Read from the INSTALLED package rather than from `BuildConfig`: this reports what - * is actually on the phone, which is the question a bug report is asking. It also - * needs no `buildFeatures.buildConfig`, which this module does not enable. - * * Note 3127 §5 is why it is here at all. With version tags gone, an artifact's own * self-report is the only answer to "which build is this?" — so it renders * "unknown" rather than nothing when the name is absent, because a blank line looks * like a layout bug and a plausible default cannot be caught by anything. + * + * The read itself is `installedVersionName()`, shared with the client header the + * app sends its server: one answer to "which build is on this phone", so the line + * a person quotes in a bug report and the line in the server's log cannot disagree. */ @Composable private fun BuildLine() { val context = LocalContext.current val unknown = stringResource(R.string.build_unknown) - val version = - remember(context) { - runCatching { - context.packageManager.getPackageInfo(context.packageName, 0).versionName - }.getOrNull() ?: unknown - } + val version = remember(context) { context.installedVersionName() ?: unknown } Text( text = version, style = MaterialTheme.typography.bodySmall, diff --git a/android/ffi/src/lib.rs b/android/ffi/src/lib.rs index 5d8e766..76c21b9 100644 --- a/android/ffi/src/lib.rs +++ b/android/ffi/src/lib.rs @@ -520,6 +520,20 @@ pub fn checklist_render(text: String, checked: bool) -> String { local::derive::render_item(&text, checked) } +/// Tell the core which app it is running inside, and which build of it. +/// +/// Android has to say so because the core cannot: the same crate is compiled into +/// the desktop app, and it used to announce every phone in the field as +/// `thoughtsync-desktop` carrying the CORE crate's version — a number no build +/// stamps and nobody has seen. The honest value is the installed package's own +/// `versionName`, which is what Kotlin passes here. +/// +/// Called once from `ThoughtSyncApplication.onCreate`, before anything can sync. +#[uniffi::export] +pub fn set_client_agent(name: String, version: String) { + compat::set_client_agent(&name, &version); +} + /// Every checklist item in a body, with the line each one sits on — so a renderer /// walking the body line by line knows which lines are boxes and what is in them. #[uniffi::export] diff --git a/core/src/sync/compat.rs b/core/src/sync/compat.rs index d2cf77a..19c604b 100644 --- a/core/src/sync/compat.rs +++ b/core/src/sync/compat.rs @@ -17,6 +17,7 @@ //! `docs/sync.md` for the policy that governs when those numbers move. use serde::{Deserialize, Serialize}; +use std::sync::OnceLock; /// The sync wire protocol this client speaks. /// @@ -172,10 +173,41 @@ pub fn evaluate(info: &ServerInfo) -> Compatibility { } } +/// Who this client says it is, set once by the host application at startup. +/// +/// THE CORE CANNOT KNOW THIS, and the value it used to invent was wrong twice. It +/// was `thoughtsync-desktop/{CARGO_PKG_VERSION}`, and this crate is compiled into +/// the desktop app AND the Android app — so every phone in the field announced +/// itself as a desktop. The version was worse: `CARGO_PKG_VERSION` here is the +/// version of the CORE crate, a number no build stamps and no user has ever seen, +/// while the thing a reader of that header wants is the app's own build (note 3127 +/// §5 — with no version tags, the artifact's self-report is the only answer to +/// "which build is this?"). +/// +/// So the host names itself. `OnceLock` because identity is fixed for the life of +/// the process and a second caller should be ignored rather than race the first. +static CLIENT_AGENT: OnceLock = OnceLock::new(); + +/// Name this client for the servers it talks to — `("thoughtsync-android", "2026.08.31.1204")`. +/// +/// Call once at startup, before any sync. Calling twice is not an error and the +/// first name wins; not calling it at all is visible in the header rather than +/// silently plausible. +pub fn set_client_agent(name: &str, version: &str) { + let _ = CLIENT_AGENT.set(format!("{name}/{version}")); +} + /// 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")); + // `unidentified/unknown`, never a plausible default. Nothing reads this header + // today, which is exactly why a wrong value could sit in it for months: the + // first person to look at a server log is the first person who could catch it, + // and only if what they see is obviously a host that never introduced itself. + let agent = CLIENT_AGENT + .get() + .cloned() + .unwrap_or_else(|| "thoughtsync-unidentified/unknown".to_string()); [ ("X-ThoughtSync-Client", agent), ( @@ -374,10 +406,24 @@ mod tests { #[test] fn client_headers_identify_app_and_protocol() { + // Sets the process-wide agent, which is why this test also owns the + // assertion about it: a second test calling `set_client_agent` would race + // this one for the OnceLock, and whichever lost would see the other's name. + // One test, both branches, in order. + assert!( + client_headers()[0].1.starts_with("thoughtsync-unidentified/"), + "a host that never introduced itself must say so" + ); + + set_client_agent("thoughtsync-test", "2026.08.31.1204"); let headers = client_headers(); assert_eq!(headers[0].0, "X-ThoughtSync-Client"); - assert!(headers[0].1.starts_with("thoughtsync-desktop/")); + assert_eq!(headers[0].1, "thoughtsync-test/2026.08.31.1204"); assert_eq!(headers[1].1, CLIENT_PROTOCOL_VERSION.to_string()); + + // First name wins — a second host cannot rename a running process. + set_client_agent("thoughtsync-impostor", "0"); + assert_eq!(client_headers()[0].1, "thoughtsync-test/2026.08.31.1204"); } #[test] diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index 5aba367..c187efc 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -53,6 +53,12 @@ use thoughtsync_core::{local, sync}; pub fn run() { use tauri_plugin_log::{Target, TargetKind}; + // Introduce ourselves to any server this app links to, BEFORE anything can sync. + // The core cannot work this out — it is compiled into the Android app too — so + // the header says "desktop" only because the desktop says so here, and carries + // the build a person can read rather than the core crate's own version. + sync::compat::set_client_agent("thoughtsync-desktop", display_version()); + #[cfg(target_os = "linux")] harden_linux_webkit_rendering();