From 544cf7273590e029b14a59f8d48b2b69c3c1385a Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sat, 29 Aug 2026 16:59:44 -0400 Subject: [PATCH 001/214] install: the stable fallback is dead now that stable publishes its own bundles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It existed for one window: `stable` was a manifest-only pointer at whatever `v*` tag had last been cut, and `stable` is the DEFAULT channel, so without the fallback `curl … | sh` was broken for everyone between step 3 landing and the first merge to `main`. That merge happened (`b6673c6`), and `stable` now holds its own signed bundles at 1.0.3503145 — AppImage, deb and pacman, all resolving by the one lookup both channels share. Kept as a fallback it stops being a safety net and becomes a mask: the branch only runs when `stable` has no bundles, which from here on means something is broken, and chasing a `v*` release instead of saying so is the wrong answer. The header now says the transition is finished and that neither channel should be special-cased again, because the shape of that code invites re-adding it. --- desktop/packaging/install.sh | 34 +++++++--------------------------- 1 file changed, 7 insertions(+), 27 deletions(-) diff --git a/desktop/packaging/install.sh b/desktop/packaging/install.sh index 385fc8a..5ddded9 100755 --- a/desktop/packaging/install.sh +++ b/desktop/packaging/install.sh @@ -10,7 +10,10 @@ # Both are fixed-tag releases: the tag never moves and the assets are pruned to the # current build, so the tag alone names the newest one. `stable` only became one in # M314 step 3, when `main` started publishing — before that it was a manifest-only -# pointer at whatever `v*` tag somebody had last cut. +# pointer at whatever `v*` tag somebody had last cut, and this script carried a +# fallback that chased the `v*` release its manifest named. That came out once +# `main` had published to `stable` for real (`b6673c6`); the two channels are the +# same shape now and nothing here should special-case one of them again. # Pick one with `--channel dev` or `TS_CHANNEL=dev`. Through a pipe the options go # after a `--`: curl -fsSL | sh -s -- --channel dev # @@ -86,36 +89,13 @@ esac # --- resolve the release for this channel ----------------------------------- say "Finding the latest ThoughtSync build on the $channel channel…" -# ONE lookup for both channels now. Each is a release whose tag never moves and whose -# assets are pruned to the current build, so the tag alone names the newest build on -# that channel — which is exactly what an installer wants and what the in-app updater +# ONE lookup, both channels. Each is a release whose tag never moves and whose assets +# are pruned to the current build, so the tag alone names the newest build on that +# channel — which is exactly what an installer wants and what the in-app updater # already reads. json="$(curl -fsSL "$API/releases/tags/$channel" 2>/dev/null)" || die "the $channel channel has nothing published yet." -# TRANSITIONAL — delete with the rest of the old scheme (M314 step 7). -# -# `stable` existed before this as a manifest-ONLY pointer: `latest.json` naming a -# version whose bundles lived on a separate `v` release. Between this commit -# and the first merge to `main` it still looks like that, and `stable` is the DEFAULT -# channel — so without this fallback `curl … | sh` is broken for everyone in that -# window. It costs nothing once main has published: the grep finds the bundles and -# this branch never runs again. -if [ "$channel" = "stable" ] && ! printf '%s' "$json" | grep -q "releases/download/stable/[^\"]*\.\(AppImage\|deb\|pkg\.tar\)"; then - say "stable has no bundles of its own yet — falling back to the version its manifest names." - manifest="$(curl -fsSL "$INSTANCE/$REPO/releases/download/stable/latest.json" 2>/dev/null || true)" - stable_version="$(printf '%s' "$manifest" | - grep -oE '"version"[[:space:]]*:[[:space:]]*"[^"]+"' | head -1 | - sed -E 's/.*"([^"]+)"$/\1/')" - if [ -n "$stable_version" ]; then - json="$(curl -fsSL "$API/releases/tags/v$stable_version" 2>/dev/null)" || - die "the stable channel names $stable_version, but there is no v$stable_version release to install." - else - json="$(curl -fsSL "$API/releases/latest" 2>/dev/null)" || - die "no stable build published yet — try --channel dev, or merge to main." - fi -fi - # Pull asset URLs straight out of the release JSON (no jq). Anchored on the closing # quote so a `…AppImage.sig` URL can't be truncated into a match of its own. asset_url() { -- 2.54.0 From f992439588249399ee9241950d7b4981a495b188 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sat, 29 Aug 2026 23:07:29 -0400 Subject: [PATCH 002/214] version: every surface can say which build it is, and two of them were lying MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Note 3127 §5 removed version tags, so an artifact's self-report is now the only answer to "which build is this?" — and nothing exists to contradict it when it is wrong. Three surfaces gain a dim build line: the foot of the web rail, the login screen, and the foot of Sync on Android. The login screen because "I can't sign in" is a bug report like any other, and requiring an account to read a build number withholds it from exactly the people who can't get past that page. `/api/config` is already public. Two of the values it was going to show were wrong, which is the part worth knowing about. The DESKTOP reported `env!("CARGO_PKG_VERSION")` from `config_get` and from the startup log. `cargo tauri build --config '{"version": ...}'` overrides tauri.conf.json, not Cargo's own metadata — so both read the literal `0.2.0` in Cargo.toml, on every build ever shipped. They now read a display version baked in by the lane through `option_env!`, hoisted to the crate root because two readers of one fact is how this repo keeps producing 2181-2183. Not the ordering key either: `1.0.` is the opaque value Tauri's updater compares and must never be shown to a person, and `update.rs` still reads it because a comparator is exactly what it is (rule 149). The SERVER fell back to `__version__` when APP_VERSION was absent, so a server run from a checkout reported `0.2.0` — a real-looking version naming no build anybody could obtain. `__init__.py` already asserted the honest answer was "APP_VERSION being missing, which app.py already handles"; it did not, and a comment claiming a behaviour two files away is how that stayed true-sounding. Now an explicit "unknown", with the packaging version left where "unknown" is not a legal value. Android reads the INSTALLED package's versionName rather than BuildConfig, so it reports what is actually on the phone. Everything renders "unknown" rather than blank when it cannot say. A blank looks like a layout bug; a plausible default cannot be caught by anything. build.rs gets `rerun-if-env-changed` for the baked value: cargo does not track an `option_env!` variable on its own, and the desktop lane having no cache today is what makes that easy to forget the day one is added. --- .forgejo/workflows/desktop.yml | 16 +++++++ .../fabledsword/thoughtsync/ui/SyncScreen.kt | 34 ++++++++++++++ android/app/src/main/res/values/strings.xml | 5 ++ desktop/src-tauri/build.rs | 10 ++++ desktop/src-tauri/src/commands/local.rs | 4 +- desktop/src-tauri/src/lib.rs | 35 +++++++++++++- frontend/src/components/AppShell.vue | 30 +++++++++++- frontend/src/views/LoginView.vue | 12 +++++ src/thoughtsync/__init__.py | 20 +++++--- src/thoughtsync/app.py | 16 ++++++- tests/test_app.py | 47 +++++++++++++++++++ 11 files changed, 216 insertions(+), 13 deletions(-) diff --git a/.forgejo/workflows/desktop.yml b/.forgejo/workflows/desktop.yml index 39748ef..9c12c20 100644 --- a/.forgejo/workflows/desktop.yml +++ b/.forgejo/workflows/desktop.yml @@ -162,6 +162,14 @@ jobs: # separate value and arrives with the UI that shows it (#3181). version="$(sh ../../packaging/version.sh key desktop)" echo "Building desktop ordering key $version" + # The DISPLAY version, baked into the binary by `option_env!` (#3181). + # A different value for a different audience: this is the one a person + # quotes in a bug report, the key above is the one only a comparator + # sees. Exported rather than passed as a flag because the macro that + # reads it is in Rust source, not in Tauri's config. + THOUGHTSYNC_DISPLAY_VERSION="$(sh ../../packaging/version.sh display desktop)" + export THOUGHTSYNC_DISPLAY_VERSION + echo "Baking display version $THOUGHTSYNC_DISPLAY_VERSION" cargo tauri build \ --config '{"build":{"beforeBuildCommand":""}}' \ --config "{\"version\":\"$version\"}" \ @@ -347,6 +355,14 @@ jobs: # separate value and arrives with the UI that shows it (#3181). version="$(sh ../../packaging/version.sh key desktop)" echo "Building desktop ordering key $version" + # The DISPLAY version, baked into the binary by `option_env!` (#3181). + # A different value for a different audience: this is the one a person + # quotes in a bug report, the key above is the one only a comparator + # sees. Exported rather than passed as a flag because the macro that + # reads it is in Rust source, not in Tauri's config. + THOUGHTSYNC_DISPLAY_VERSION="$(sh ../../packaging/version.sh display desktop)" + export THOUGHTSYNC_DISPLAY_VERSION + echo "Baking display version $THOUGHTSYNC_DISPLAY_VERSION" updater='{}' if [ -n "${TAURI_SIGNING_PRIVATE_KEY:-}" ]; then updater='{"bundle":{"createUpdaterArtifacts":true}}' 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 461d700..be5d857 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 @@ -31,6 +31,7 @@ import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.res.pluralStringResource import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.font.FontWeight @@ -120,10 +121,43 @@ fun SyncScreen( onDismissRevokeNotice = onDismissRevokeNotice, ) } + + BuildLine() } } } +/** + * 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. + */ +@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 + } + Text( + text = version, + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.padding(bottom = 16.dp), + ) +} + // ───────────────────────────────── linked ───────────────────────────────── @Composable diff --git a/android/app/src/main/res/values/strings.xml b/android/app/src/main/res/values/strings.xml index ec1c943..2224f4a 100644 --- a/android/app/src/main/res/values/strings.xml +++ b/android/app/src/main/res/values/strings.xml @@ -200,4 +200,9 @@ Dismiss + + + unknown diff --git a/desktop/src-tauri/build.rs b/desktop/src-tauri/build.rs index d860e1e..c407dd0 100644 --- a/desktop/src-tauri/build.rs +++ b/desktop/src-tauri/build.rs @@ -1,3 +1,13 @@ fn main() { + // Cargo does NOT track an `option_env!` variable on its own — the macro is + // expanded at compile time and nothing records that the crate depends on it. + // So without this line, a cached `target/` would keep a binary reporting + // whatever version the previous build baked, and the footer would confidently + // name the wrong build. The desktop lane has no cache today, which is exactly + // why this is easy to forget the day one is added. + // + // See DISPLAY_VERSION in `src/commands/local.rs`. + println!("cargo::rerun-if-env-changed=THOUGHTSYNC_DISPLAY_VERSION"); + tauri_build::build() } diff --git a/desktop/src-tauri/src/commands/local.rs b/desktop/src-tauri/src/commands/local.rs index e2e4a29..a41c2ec 100644 --- a/desktop/src-tauri/src/commands/local.rs +++ b/desktop/src-tauri/src/commands/local.rs @@ -31,7 +31,9 @@ pub fn config_get(db: State<'_, Db>) -> PublicConfig { PublicConfig { site_name: "ThoughtSync".to_string(), allow_registration: false, - version: env!("CARGO_PKG_VERSION").to_string(), + // The build a person reads, baked at compile time — see crate::display_version + // for why this is neither CARGO_PKG_VERSION nor the updater's ordering key. + version: crate::display_version().to_string(), enable_url_unfurl: false, trash_retention_days: retention_days.max(0) as u32, } diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index 51dd52f..5aba367 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -13,6 +13,37 @@ mod commands; mod integration; mod update; +/// The build a PERSON reads, baked in by the desktop lane at compile time. +/// +/// Lives at the crate root because it has two readers — `config_get`, which puts it +/// in the UI, and `log_environment`, which puts it in the log — and this repo has +/// spent several issues on one fact held in two places (2181, 2182, 2183). +/// +/// `option_env!`, not `env!`: a local `cargo tauri build` sets nothing, and this has +/// to keep compiling. `None` becomes "unknown" at each call site rather than a +/// plausible-looking default — note 3127 §5 makes this string the only answer to +/// "which build is this?" now that there are no version tags, so there is nothing +/// left to contradict it if it lies. An honest "I cannot say" is the only safe wrong +/// answer. +/// +/// NOT `CARGO_PKG_VERSION`, which both readers used to use, and which was wrong on +/// every build ever shipped: `cargo tauri build --config '{"version": ...}'` +/// overrides `tauri.conf.json`, not Cargo's own metadata, so the literal `0.2.0` in +/// Cargo.toml is what reached the UI and the log regardless of what was built. +/// +/// NOT the ordering key either. That value — `1.0.`, which the override +/// above does set — is the opaque value Tauri's updater compares; it lands in bundle +/// filenames and `latest.json` and must never be shown to a person (#3144). Two +/// values, two audiences. `update.rs` deliberately still reads the key, through +/// `app.package_info().version`, because a comparator is exactly what it is. +const DISPLAY_VERSION: Option<&str> = option_env!("THOUGHTSYNC_DISPLAY_VERSION"); + +/// The baked build, or the honest "I cannot say". The only way in — the const is +/// private so no caller can reach past the fallback. +pub(crate) fn display_version() -> &'static str { + DISPLAY_VERSION.unwrap_or("unknown") +} + // The store and the sync engine live in the shared `thoughtsync-core` crate, which // the Android client binds through uniffi (Scribe note 2730). Aliased to their old // names so every call site below reads exactly as it did when they were modules of @@ -219,8 +250,8 @@ fn log_event(level: String, message: String) { fn log_environment(app: &tauri::App) { use tauri::Manager; log::info!( - "ThoughtSync desktop v{} starting ({} {})", - env!("CARGO_PKG_VERSION"), + "ThoughtSync desktop {} starting ({} {})", + display_version(), std::env::consts::OS, std::env::consts::ARCH, ); diff --git a/frontend/src/components/AppShell.vue b/frontend/src/components/AppShell.vue index 1d601ab..3eaadb0 100644 --- a/frontend/src/components/AppShell.vue +++ b/frontend/src/components/AppShell.vue @@ -27,6 +27,23 @@ const ui = useUiStore(); // Sync is a desktop-app concern: the web build already IS the server's UI. const desktopApp = isDesktop(); +// The build, for the dim line at the foot of the rail (#3181). +// +// NEVER BLANK. "unknown" is the honest answer when the value is missing, and an +// empty space is a bug that reads as a design choice. Note 3127 §5: with version +// tags gone this is the only answer to "which build is this?", so it has to be +// either right or visibly absent. +// +// One slot, two artifacts, and that is deliberate rather than sloppy. In the +// browser `repo` is `rest`, so this is the SERVER's version; in the desktop shell +// `repo` is `local` and `config_get` returns the desktop build's own. Each surface +// names the thing the person is actually looking at. A linked server's version is +// a different question and Sync answers it separately. +const buildVersion = computed(() => config.version || "unknown"); +const buildLabel = computed( + () => `ThoughtSync ${desktopApp ? "desktop" : "server"} build ${buildVersion.value}`, +); + async function removeView(f: SavedFilter) { if (!window.confirm(`Delete the "${f.name}" view?`)) return; try { @@ -415,7 +432,7 @@ async function signOut() { @click="drawer = false" > +

+ {{ config.version || "unknown" }} +

diff --git a/src/thoughtsync/__init__.py b/src/thoughtsync/__init__.py index 0c2c273..0148171 100644 --- a/src/thoughtsync/__init__.py +++ b/src/thoughtsync/__init__.py @@ -1,10 +1,16 @@ """ThoughtSync — self-hosted personal thought-capture web app (FabledSword family).""" -# The FALLBACK version, used only when APP_VERSION is absent from the environment — -# i.e. running from a checkout rather than from an image. A built image always has -# it, derived from the server's own shipped file set (packaging/version.sh), so this -# string never reaches a deployed instance and bumping it changes nothing a user -# sees. Kept because a package needs a version and "unknown" is not a valid one for -# packaging metadata; the honest "I cannot say" for a running server is APP_VERSION -# being missing, which app.py already handles. +# PACKAGING METADATA, and nothing else. Not the version any running server reports. +# +# A built image carries APP_VERSION in the environment, derived from the server's +# own shipped file set (packaging/version.sh); `app.py` reads that and reports an +# explicit "unknown" when it is absent, so this string never reaches a user and +# bumping it changes nothing anybody sees. +# +# It exists because a Python package needs a version and "unknown" is not a legal +# one here. It used to double as app.py's fallback, which meant a server run from a +# checkout confidently reported `0.2.0` — a real-looking version naming no build +# that exists. Note 3127 §5 is why that matters more than it reads: with version +# tags gone, a build's self-report is the only answer to "which build is this?", +# and there is nothing left to catch it lying. __version__ = "0.2.0" diff --git a/src/thoughtsync/app.py b/src/thoughtsync/app.py index 8c655d9..0809c59 100644 --- a/src/thoughtsync/app.py +++ b/src/thoughtsync/app.py @@ -11,7 +11,6 @@ from datetime import timedelta from quart import Quart, jsonify, send_from_directory from quart.sessions import SecureCookieSessionInterface -from . import __version__ from .auth import bp as auth_bp from .client_dist import advertisement as client_advertisement, bp as client_bp from .config import Config @@ -64,7 +63,20 @@ def create_app() -> Quart: # Ephemeral/env secret so the app (and DB-free unit tests) construct without a # database. before_serving swaps in the real, DB-persisted key before serving. app.secret_key = Config.secret_key_env() or secrets.token_urlsafe(48) - app.config["APP_VERSION"] = os.environ.get("APP_VERSION", __version__) + # The RUNNING build, or an explicit "unknown" — never the packaging fallback. + # + # This read `os.environ.get("APP_VERSION", __version__)`, so a server started + # from a checkout reported `0.2.0`: a real-looking version that names no build + # anybody could get. `__init__.py` already claimed the honest answer was + # "APP_VERSION being missing, which app.py already handles" — it did not, and a + # comment asserting a behaviour two files away from the code is how that stayed + # true-sounding for months. + # + # It matters more than it used to. Note 3127 §5 removed version tags, so this + # string is the only answer to "which build is this?" and nothing exists to + # contradict it when it is wrong. `__version__` stays where it belongs, as + # packaging metadata, which is the one place "unknown" is not a legal value. + app.config["APP_VERSION"] = os.environ.get("APP_VERSION") or "unknown" app.config["SESSION_COOKIE_HTTPONLY"] = True app.config["SESSION_COOKIE_SAMESITE"] = "Lax" # Auto-mark the session cookie Secure on HTTPS requests (see the interface above). diff --git a/tests/test_app.py b/tests/test_app.py index 942563f..e9f95ec 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -27,3 +27,50 @@ async def test_unknown_api_route_404s(app): client = app.test_client() resp = await client.get("/api/does-not-exist") assert resp.status_code == 404 + + +# --- the version a running server reports ------------------------------------ +# +# Note 3127 §5 removed version tags, so this string is the only answer to "which +# build is this?" and nothing exists to contradict it when it is wrong. That makes +# the FALLBACK the interesting case rather than the happy path: it used to be +# `__version__`, so a server run from a checkout reported `0.2.0` — a real-looking +# version naming no build anybody could obtain. + + +async def reported_version() -> str: + """What a freshly built app tells /api/health it is. + + Built per call rather than through the `app` fixture: the value is read from the + environment in `create_app`, so an app constructed before `monkeypatch` ran would + answer about the wrong environment. + """ + client = create_app().test_client() + return (await (await client.get("/api/health")).get_json())["version"] + + +async def test_the_version_is_whatever_the_environment_says(monkeypatch): + monkeypatch.setenv("APP_VERSION", "2026.08.29.0443") + assert await reported_version() == "2026.08.29.0443" + + +async def test_no_version_in_the_environment_reports_unknown(monkeypatch): + """The honest "I cannot say", not a plausible default. + + Also asserted against `__version__` by name rather than against the literal it + happens to hold, so bumping the packaging version cannot make this pass for the + wrong reason. + """ + from thoughtsync import __version__ + + monkeypatch.delenv("APP_VERSION", raising=False) + reported = await reported_version() + assert reported == "unknown" + assert reported != __version__ + + +async def test_an_empty_version_reports_unknown_too(monkeypatch): + """`APP_VERSION=` is what a mis-set build arg looks like, and an empty string + renders as a blank space rather than as a missing value.""" + monkeypatch.setenv("APP_VERSION", "") + assert await reported_version() == "unknown" -- 2.54.0 From ef8aa9340f9b37b58421aae8bb09951200831410 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 30 Aug 2026 12:52:40 -0400 Subject: [PATCH 003/214] clients: the server hands out five platforms, not "the Android client" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `client_dist.py` was written for one platform and everything structural in it was already right — drop-in beats baked, the pair must describe one build, absence is an ordinary answer, metadata public and bytes authenticated. This widens it to a table rather than building beside it. Its own docstring made the argument years before there was a second platform: a self-hoster should not need an account on someone else's forge to get the app for their own notes. Server side only. CI bakes nothing new until step 3 and the UI reads nothing new until step 4, so this lands green and inert. Five rows — android, linux-deb, linux-pacman, linux-appimage, windows — each naming its artifact, sidecar and mimetype. Fixed filenames, version only in the sidecar: a version-stamped name would force a glob, and a glob over a directory an operator drops files into is how you serve the older of two builds, which is the failure write-manifest.sh already carries a comment about. THE ANDROID NAMES AND ROUTE DO NOT MOVE. The lane publishes those exact filenames, clients in the field poll /api/client/android, and `android_client` stays on /api/config beside the new `clients` map. Renaming them to match the pattern would buy tidiness and strand every installed phone; retiring the key belongs to a later change made when nothing polls it, not to the change introducing its replacement. Fields were added, not moved — `ClientRelease` in core is a plain serde struct and ignores what it does not know. PRECEDENCE IS PER PLATFORM, which is the trap the table introduces. "First directory holding anything wins" would mean dropping in an APK silently retracts the four desktop downloads. Pinned by a test. The AppImage needs a third file. It is the only bundle that replaces itself in place, so the updater verifies a minisign signature before it does — and a bundle that cannot be verified cannot be offered. A missing or empty `.sig` therefore makes it absent rather than merely unsigned, and the signature travels WITH the version so an updater can never pair one build's version with another's signature. The tests parametrize over the table instead of testing Android and trusting the rest. The bugs this module can have are not platform-specific, and a suite that only exercised one platform is how the other four would ship untested. --- docs/android-distribution.md | 4 + src/thoughtsync/app.py | 7 +- src/thoughtsync/client_dist.py | 324 ++++++++++++++++++++++++------- src/thoughtsync/config.py | 13 +- tests/test_client_dist.py | 342 ++++++++++++++++++++++++++------- 5 files changed, 544 insertions(+), 146 deletions(-) diff --git a/docs/android-distribution.md b/docs/android-distribution.md index 506ea96..66c0505 100644 --- a/docs/android-distribution.md +++ b/docs/android-distribution.md @@ -36,6 +36,10 @@ pinning. If you want a specific build — testing something, or holding back — drop it in `/var/thoughtsync/client/` and it wins over the image's copy. +That directory is shared with the desktop clients the server hands out, and +**precedence is decided per platform**: dropping in an APK overrides the baked APK +and leaves every other client alone. It is one directory, not one choice. + Two files, both required: | File | What it is | diff --git a/src/thoughtsync/app.py b/src/thoughtsync/app.py index 0809c59..bd2ed75 100644 --- a/src/thoughtsync/app.py +++ b/src/thoughtsync/app.py @@ -204,9 +204,10 @@ def create_app() -> Quart: # 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()) - # Which Android client this server can hand out, if any. Absent rather than - # null when it has none, so the web UI hides the download instead of - # offering a button that 404s. + # Which CLIENTS this server can hand out, if any — the whole set under + # `clients`, plus the older `android_client` key that phones in the field + # still read. Absent rather than null when it has none, so the web UI hides + # a download instead of offering a button that 404s. data.update(client_advertisement()) return jsonify(data) diff --git a/src/thoughtsync/client_dist.py b/src/thoughtsync/client_dist.py index 7d3239d..a75ede4 100644 --- a/src/thoughtsync/client_dist.py +++ b/src/thoughtsync/client_dist.py @@ -1,4 +1,4 @@ -"""The server hands out the Android client it is in step with. +"""The server hands out the clients it is in step with. ## Why the server, and not a release page @@ -11,43 +11,69 @@ It also keeps the two in step by construction. Client and server already negotiate a sync protocol version before linking, so a server that also serves the client cannot hand out a phone it is unable to talk to. -## Where the file comes from +That argument was never Android-specific, which is why this module now serves a +TABLE of platforms rather than the one it was written for. -Two places, checked in that order: +## Where the files come from + +Two places, checked in that order, per platform: 1. `DATA_DIR/client/` — the mounted volume that already holds attachments. An operator who wants a SPECIFIC build drops it there and it wins. 2. the copy baked into the image at build time — CI fetches the newest published - Android build into every image, so `:dev`, `:latest` and `:` all - carry a client and `docker compose pull` delivers a new one with nothing - copied by hand. + build of each client into every image, so `:dev` and `:latest` both carry a + full set and `docker compose pull` delivers new ones with nothing copied by + hand. The precedence is the point: the image is the default, and a person who wants to override it should not have to fight it. The baked copy sits inside the package rather than under DATA_DIR because DATA_DIR is a volume mount, and anything the image wrote there would be hidden the moment one is attached. +**Precedence is decided per platform, not for the set.** A drop-in `.deb` does +not shadow the baked APK. The alternative — first directory holding anything +wins — would mean replacing one client silently retracts the other four. + +## What a platform needs on disk + Two files, and both must be present: -- `thoughtsync.apk` — the client -- `thoughtsync-android.json` — `{version_name, version_code, size, sha256}` +- the artifact, at a FIXED name (`thoughtsync.deb`, not + `ThoughtSync_2026.08.30.0307_amd64.deb`) +- its sidecar, `{version_name, version_code, size, sha256}` -The sidecar exists because an APK's version lives in a binary AXML manifest that -Python cannot read without the Android build tools. CI writes it beside the APK -at publish time, where the real values are already known. +**Fixed names, and the version only in the sidecar.** A version-stamped filename +would force this module to glob, and a glob over a directory an operator can drop +files into is how you serve the older of two builds — `write-manifest.sh` carries +a comment about exactly that, from the time it advertised a new version while +pointing at an old binary. + +The sidecar exists because a version is not reliably readable from the artifact: +an APK keeps it in a binary AXML manifest Python cannot parse without the Android +build tools, and a `.deb` or an AppImage would each need a different unpacker. CI +writes the sidecar where the real value is already known. + +The AppImage needs a THIRD file, `thoughtsync.AppImage.sig`. That is the minisign +signature the desktop updater verifies before replacing the running binary, and a +bundle that cannot be verified cannot be offered as an update — so a missing +signature makes the AppImage absent rather than merely unsigned. ## Absence is normal -A server with no APK advertises nothing, and the web UI hides the download -rather than offering a button that 404s. Same for a mismatched pair: if the -sidecar's recorded size does not match the file on disk, the two did not arrive -together and the server says it has nothing rather than serving one build while -describing another. +A server with no client for a platform advertises none, and the web UI hides that +download rather than offering a button that 404s. Same for a mismatched pair: if +the sidecar's recorded size does not match the file on disk, the two did not +arrive together, and the server says it has nothing rather than serving one build +while describing another. + +This is the ONLY state available before a platform's first build has ever +published, so it is an ordinary answer and never an error. """ from __future__ import annotations import json +from dataclasses import dataclass from pathlib import Path from quart import Blueprint, jsonify, send_from_directory @@ -55,10 +81,90 @@ from quart import Blueprint, jsonify, send_from_directory from .auth import login_required from .config import Config -APK_NAME = "thoughtsync.apk" -MANIFEST_NAME = "thoughtsync-android.json" -DOWNLOAD_PATH = "/api/client/android/download" -APK_MIMETYPE = "application/vnd.android.package-archive" + +@dataclass(frozen=True) +class Platform: + """One installable client, and where its files sit.""" + + id: str + # What a person calls it. Named for the DISTRO rather than the package format + # ("Debian / Ubuntu", not ".deb") — someone knows which system they run and + # does not necessarily know which packaging it uses. + label: str + artifact: str + sidecar: str + mimetype: str + # An updater-verifiable bundle: `.sig` must be present too, and its + # contents are published with the metadata. Only the AppImage, because it is + # the only bundle that can replace itself in place — a package-manager install + # cannot, by design (see the desktop's update.rs). + signed: bool = False + + @property + def signature(self) -> str: + return f"{self.artifact}.sig" + + @property + def download_path(self) -> str: + return f"/api/client/{self.id}/download" + + +# ONE definition of what this server can hand out. Every route, the /api/config +# advertisement and the tests all read this table; adding a platform is adding a +# row. +PLATFORMS: tuple[Platform, ...] = ( + Platform( + id="android", + label="Android", + # UNCHANGED, and it must stay unchanged: the Android lane publishes these + # exact names, CI bakes them in under them, and clients in the field poll + # `/api/client/android`. Renaming them to match the pattern below would + # buy tidiness and strand every installed phone. + artifact="thoughtsync.apk", + sidecar="thoughtsync-android.json", + mimetype="application/vnd.android.package-archive", + ), + Platform( + id="linux-deb", + label="Debian / Ubuntu", + artifact="thoughtsync.deb", + sidecar="thoughtsync-linux-deb.json", + mimetype="application/vnd.debian.binary-package", + ), + Platform( + id="linux-pacman", + label="Arch / CachyOS", + artifact="thoughtsync.pkg.tar.zst", + sidecar="thoughtsync-linux-pacman.json", + mimetype="application/zstd", + ), + Platform( + id="linux-appimage", + label="Other Linux (AppImage)", + artifact="thoughtsync.AppImage", + sidecar="thoughtsync-linux-appimage.json", + # No registered type for an AppImage, and guessing one buys nothing: it is + # served as an attachment either way, and octet-stream is the answer that + # cannot be wrong. + mimetype="application/octet-stream", + signed=True, + ), + Platform( + id="windows", + label="Windows", + artifact="thoughtsync-setup.exe", + sidecar="thoughtsync-windows.json", + mimetype="application/vnd.microsoft.portable-executable", + ), +) + +BY_ID: dict[str, Platform] = {p.id: p for p in PLATFORMS} + +# The Android names, still importable under their old spellings because docs and +# the CI lane refer to them. Derived from the table rather than restated, so the +# two cannot drift. +APK_NAME = BY_ID["android"].artifact +MANIFEST_NAME = BY_ID["android"].sidecar # The copy CI bakes into the image. Inside the package, NOT under DATA_DIR: that # is a volume mount, and a file the image wrote there would vanish behind it. @@ -67,17 +173,17 @@ BAKED_ROOT = Path(__file__).resolve().parent / "client" bp = Blueprint("client_dist", __name__) -def _read(root: Path) -> dict | None: - """The build in one directory, or None. +def _read(root: Path, platform: Platform) -> dict | None: + """One platform's build in one directory, or None. - Never raises. A missing directory, an unreadable sidecar, malformed JSON and a - sidecar that describes a different file are all the same answer to the only - question being asked — "is there a client here I can honestly offer?" — and - that answer is no. + Never raises. A missing directory, an unreadable sidecar, malformed JSON, a + sidecar that describes a different file and — for a signed bundle — a missing + signature are all the same answer to the only question being asked, "is there + a client here I can honestly offer?", and that answer is no. """ try: - size = (root / APK_NAME).stat().st_size - meta = json.loads((root / MANIFEST_NAME).read_text(encoding="utf-8")) + size = (root / platform.artifact).stat().st_size + meta = json.loads((root / platform.sidecar).read_text(encoding="utf-8")) version = str(meta["version_name"]) code = int(meta["version_code"]) recorded = int(meta["size"]) @@ -87,83 +193,163 @@ def _read(root: Path) -> dict | None: # The pair has to describe one build. A sidecar left behind by a previous # release would otherwise advertise a version this server cannot serve, and the - # phone would download something other than what it was promised. + # client would download something other than what it was promised. if recorded != size: return None - return { + release = { + "platform": platform.id, + "label": platform.label, "version": version, - # What Android actually compares. `version` is for people; a name is a - # string and sorts like one, which is not how "is this newer" works. + # What a comparator reads. `version` is for people; a name is a string and + # sorts like one, which is not how "is this newer" works. "version_code": code, "size": size, # Computed by CI over the same bytes it uploaded, so a client can tell a - # truncated download from a complete one BEFORE handing it to the - # installer. Not a trust anchor — the signature is that. + # truncated download from a complete one BEFORE handing it to an installer. + # Not a trust anchor — the signature is that. "sha256": digest, - "url": DOWNLOAD_PATH, + # A PATH, never an absolute URL: the client joins it to the base it is + # already linked to, so a compromised or misconfigured server cannot + # redirect the download somewhere else. `core/src/sync/client.rs` relies on + # this and says so. + "url": platform.download_path, } + if platform.signed: + # The signature travels WITH the metadata rather than behind its own route. + # It is ~100 bytes, it is public wherever these bundles are published, and + # the updater needs the version and the signature in the same breath — one + # request that cannot return a signature belonging to a different build. + try: + sig = (root / platform.signature).read_text(encoding="utf-8").strip() + except OSError: + return None + if not sig: + return None + release["signature"] = sig -def _resolve() -> tuple[Path, dict] | None: - """Which directory this server serves from, and what is in it. + return release + + +def _resolve(platform: Platform) -> tuple[Path, dict] | None: + """Which directory this server serves a platform from, and what is in it. The operator's drop-in beats the baked copy — someone who deliberately put a - build on the volume wants that build, not whatever the image happened to ship - with. A directory holding a broken or half-copied pair does NOT shadow the - image: it simply is not a client, so the search moves on. + build on the volume wants that build, older or not. A directory holding a + broken or half-copied pair does NOT shadow the image: it simply is not a + client, so the search moves on. """ for root in (Path(Config.client_root()), BAKED_ROOT): - release = _read(root) + release = _read(root, platform) if release is not None: return root, release return None -def android_release() -> dict | None: - """What Android build this server holds, or None if it holds none.""" - resolved = _resolve() +def release(platform_id: str) -> dict | None: + """What build of one client this server holds, or None if it holds none.""" + platform = BY_ID.get(platform_id) + if platform is None: + return None + resolved = _resolve(platform) return resolved[1] if resolved else None -def advertisement() -> dict: - """The `/api/config` fragment describing this server's Android client. +def releases() -> dict[str, dict]: + """Every client this server can hand out, keyed by platform id. - An empty dict when there is none, so the key is ABSENT rather than null — a - client testing for the key gets one unambiguous answer instead of having to - distinguish "no client" from "old server that never had this field". + Platforms it holds nothing for are ABSENT rather than present-and-null, so a + caller can test for the key instead of distinguishing "no build" from "a + server that never had this platform". """ - release = android_release() - return {"android_client": release} if release else {} + found = {p.id: _resolve(p) for p in PLATFORMS} + return {pid: r[1] for pid, r in found.items() if r is not None} -@bp.get("/api/client/android") -async def android_metadata(): - """Version and digest without the 55 MiB. What an updater polls.""" - release = android_release() - if release is None: - return jsonify({"error": "this server has no Android client"}), 404 - return jsonify(release) +def android_release() -> dict | None: + """What Android build this server holds, or None. + + Kept as its own name because the back-compatible `/api/config` key below is + about Android specifically, and because saying so reads better than + `release("android")` at the two call sites that mean the phone. + """ + return release("android") -@bp.get(DOWNLOAD_PATH) +def advertisement() -> dict: + """The `/api/config` fragment describing this server's clients. + + Two keys, deliberately, and the older one is not deprecated here: + + `clients` is the whole table, which is what the web UI renders the downloads + section from. + + `android_client` is what phones in the field already read. It costs one + duplicated dict to not strand every installed Android client, and retiring it + is a later decision made when nothing polls it — not a tidy-up done in the + change that introduces its replacement. + + Both are ABSENT rather than null when empty, so a client testing for a key gets + one unambiguous answer instead of having to distinguish "no client" from "an + old server that never had this field". + """ + data: dict = {} + found = releases() + if found: + data["clients"] = found + if "android" in found: + data["android_client"] = found["android"] + return data + + +@bp.get("/api/client") +async def client_index(): + """Everything this server holds, in one request. + + The downloads UI needs all five to decide what to lead with, and five requests + to answer one question is five chances to render half a page. + """ + return jsonify({"clients": releases()}) + + +@bp.get("/api/client/") +async def client_metadata(platform_id: str): + """Version and digest without the payload. What an updater polls. + + Public, because a client has to be able to ask "is there something newer?" + cheaply — before it has a token, in the case of a first pairing. + + An unknown platform and a platform with no build both 404. They are the same + answer to the caller ("not here"), and distinguishing them would only tell an + unauthenticated stranger which platforms this build of the server knows about. + """ + found = release(platform_id) + if found is None: + return jsonify({"error": f"this server has no {platform_id} client"}), 404 + return jsonify(found) + + +@bp.get("/api/client//download") @login_required -async def android_download(): - """The APK itself. +async def client_download(platform_id: str): + """The bytes themselves. Authenticated — by session cookie from a browser, or by device bearer token from a client updating itself; `login_required` accepts either. The metadata - above is public because a client has to be able to ask "is there something - newer?" cheaply, but the bytes are not for anyone who can reach the port. + above is public; the bytes are not for anyone who can reach the port. """ - resolved = _resolve() + platform = BY_ID.get(platform_id) + if platform is None: + return jsonify({"error": f"unknown client platform '{platform_id}'"}), 404 + resolved = _resolve(platform) if resolved is None: - return jsonify({"error": "this server has no Android client"}), 404 - # From the SAME directory the advertisement came from, or a drop-in appearing - # between the two calls would serve bytes the metadata does not describe. + return jsonify({"error": f"this server has no {platform_id} client"}), 404 + # From the SAME directory the metadata came from, or a drop-in appearing between + # the two calls would serve bytes the metadata does not describe. root, _ = resolved - response = await send_from_directory(root, APK_NAME, mimetype=APK_MIMETYPE) + response = await send_from_directory(root, platform.artifact, mimetype=platform.mimetype) # Without this some browsers try to render it, and Android's download handler # wants a filename to hand to the package installer. - response.headers["Content-Disposition"] = f'attachment; filename="{APK_NAME}"' + response.headers["Content-Disposition"] = f'attachment; filename="{platform.artifact}"' return response diff --git a/src/thoughtsync/config.py b/src/thoughtsync/config.py index 17f787f..690e177 100644 --- a/src/thoughtsync/config.py +++ b/src/thoughtsync/config.py @@ -35,12 +35,15 @@ class Config: @classmethod def client_root(cls) -> Path: - """Where the Android APK this server hands out lives. + """Where an operator DROPS IN clients for this server to hand out. - Under DATA_DIR rather than baked into the image: the APK is ~55 MiB and an - install that never touches Android should not carry it. Being on the same - mounted volume as uploads also means an operator drops a build there once - and container recreation does not lose it. See client_dist.py. + One directory for every platform; `client_dist.py` picks files out of it by + name. Under DATA_DIR because it is a mounted volume: a build placed here + survives container recreation, and it beats the copy baked into the image, + which is the whole point of the directory existing. + + Empty is the ordinary case — the image ships its own set and most operators + never touch this. """ return Path(cls.DATA_DIR) / "client" diff --git a/tests/test_client_dist.py b/tests/test_client_dist.py index 70dac52..5ef3eb2 100644 --- a/tests/test_client_dist.py +++ b/tests/test_client_dist.py @@ -5,7 +5,16 @@ import pytest from thoughtsync import client_dist from thoughtsync.app import create_app -from thoughtsync.client_dist import APK_NAME, MANIFEST_NAME, advertisement, android_release +from thoughtsync.client_dist import ( + APK_NAME, + BY_ID, + MANIFEST_NAME, + PLATFORMS, + advertisement, + android_release, + release, + releases, +) from thoughtsync.config import Config # DB-free, like the rest of this suite — the test lane runs no Postgres. That is @@ -13,11 +22,17 @@ from thoughtsync.config import Config # `/api/config`: the route is a one-line merge of this dict into a payload whose # other half needs a database, and testing it here tests the part that can be wrong. # -# The two routes below ARE exercised, because neither opens a session: the metadata -# route only stats files, and the download's 401 is returned before any token -# lookup. +# The routes below ARE exercised, because none opens a session: the metadata route +# only stats files, and the download's 401 is returned before any token lookup. -PAYLOAD = b"not really an apk, but the server only ever stats it" +PAYLOAD = b"not really a client, but the server only ever stats it" + +# Every test that is about the MECHANISM rather than about one platform runs +# against all of them. The bugs this module can have — a sidecar describing a +# different build, a half-finished copy shadowing a good one — are not +# platform-specific, and a suite that only ever exercised Android is how the other +# four would ship untested. +ALL_IDS = [p.id for p in PLATFORMS] @pytest.fixture(autouse=True) @@ -26,7 +41,7 @@ def _empty_baked_client(tmp_path, monkeypatch): In a source checkout `src/thoughtsync/client/` does not exist, so these tests would pass anyway — but only by accident of where they are run. A built image - has a real APK there, and a test that silently depends on which tree it is in + has real clients there, and a test that silently depends on which tree it is in is one that will eventually lie. """ monkeypatch.setattr(client_dist, "BAKED_ROOT", tmp_path / "baked") @@ -38,109 +53,225 @@ def app(): return create_app() -def place_client(payload: bytes = PAYLOAD, root: Path | None = None, **overrides) -> dict: - """Put a client + sidecar where the server looks. Overrides corrupt the pair.""" - root = root if root is not None else Config.client_root() +def place( + platform_id: str = "android", + payload: bytes = PAYLOAD, + root: Path | None = None, + signature: str | None = "a signature", + **overrides, +) -> dict: + """Put one platform's client + sidecar where the server looks. + + Overrides corrupt the pair. `signature=None` withholds the `.sig` a signed + bundle needs, which is its own failure mode rather than a variant of the others. + """ + platform = BY_ID[platform_id] + root = root if root is not None else Path(Config.client_root()) root.mkdir(parents=True, exist_ok=True) - (root / APK_NAME).write_bytes(payload) + (root / platform.artifact).write_bytes(payload) meta = { - "version_name": "0.1.216", - "version_code": 216, + "version_name": "2026.08.30.0307", + "version_code": 3503708, "size": len(payload), "sha256": "ab" * 32, } meta.update(overrides) - (root / MANIFEST_NAME).write_text(json.dumps(meta), encoding="utf-8") + (root / platform.sidecar).write_text(json.dumps(meta), encoding="utf-8") + if platform.signed and signature is not None: + (root / platform.signature).write_text(signature, encoding="utf-8") return meta -def test_absent_client_is_advertised_as_nothing_at_all(): - """The KEY is missing, not null. +# --- the table --------------------------------------------------------------- - A client testing for it then gets one unambiguous answer rather than having to - tell "this server has no APK" apart from "this server predates the feature". + +def test_every_platform_has_its_own_filenames(): + """Two platforms sharing an artifact or a sidecar name would overwrite each + other in the one directory they all live in — silently, and the survivor would + be whichever was copied last.""" + artifacts = [p.artifact for p in PLATFORMS] + sidecars = [p.sidecar for p in PLATFORMS] + assert len(set(artifacts)) == len(artifacts) + assert len(set(sidecars)) == len(sidecars) + assert not set(artifacts) & set(sidecars) + + +def test_the_android_names_are_the_ones_already_published(): + """Pinned because renaming them is a tidy-up that strands every installed phone. + + The Android lane publishes these exact names and CI bakes them in under them. """ - assert android_release() is None + assert APK_NAME == "thoughtsync.apk" + assert MANIFEST_NAME == "thoughtsync-android.json" + + +# --- absence is an ordinary answer ------------------------------------------- + + +@pytest.mark.parametrize("platform_id", ALL_IDS) +def test_an_absent_client_is_no_client(platform_id): + assert release(platform_id) is None + + +def test_a_server_holding_nothing_advertises_no_keys_at_all(): + """The KEYS are missing, not null. + + A client testing for one then gets an unambiguous answer rather than having to + tell "this server has no client" apart from "this server predates the feature". + """ + assert releases() == {} assert advertisement() == {} -def test_a_present_client_is_advertised_with_what_android_compares(): - place_client() - advertised = advertisement()["android_client"] - assert advertised["version"] == "0.1.216" - # The integer is what decides "is this newer", not the name — a name is a - # string and sorts like one. - assert advertised["version_code"] == 216 - assert advertised["size"] == len(PAYLOAD) - assert advertised["url"].endswith("/download") - - -def test_a_sidecar_describing_a_different_build_counts_as_no_client(): - """The likeliest real corruption: a new APK copied over an old sidecar. +@pytest.mark.parametrize("platform_id", ALL_IDS) +def test_a_sidecar_describing_a_different_build_counts_as_no_client(platform_id): + """The likeliest real corruption: a new artifact copied over an old sidecar. Serving one build while advertising another is worse than serving none — the - phone would compare versions against a promise the bytes do not keep. + client would compare versions against a promise the bytes do not keep. """ - place_client(size=999_999) - assert android_release() is None - assert advertisement() == {} + place(platform_id, size=999_999) + assert release(platform_id) is None -def test_an_unreadable_sidecar_counts_as_no_client(): - place_client() - (Config.client_root() / MANIFEST_NAME).write_text("{ this is not json", encoding="utf-8") - assert android_release() is None +@pytest.mark.parametrize("platform_id", ALL_IDS) +def test_an_unreadable_sidecar_counts_as_no_client(platform_id): + place(platform_id) + sidecar = Path(Config.client_root()) / BY_ID[platform_id].sidecar + sidecar.write_text("{ this is not json", encoding="utf-8") + assert release(platform_id) is None -def test_a_sidecar_missing_a_field_counts_as_no_client(): - root = Config.client_root() +@pytest.mark.parametrize("platform_id", ALL_IDS) +def test_a_sidecar_missing_a_field_counts_as_no_client(platform_id): + platform = BY_ID[platform_id] + root = Path(Config.client_root()) root.mkdir(parents=True, exist_ok=True) - (root / APK_NAME).write_bytes(PAYLOAD) - (root / MANIFEST_NAME).write_text(json.dumps({"version_name": "0.1.216"}), encoding="utf-8") - assert android_release() is None + (root / platform.artifact).write_bytes(PAYLOAD) + (root / platform.sidecar).write_text(json.dumps({"version_name": "x"}), encoding="utf-8") + assert release(platform_id) is None -def test_a_sidecar_with_no_apk_beside_it_counts_as_no_client(): - root = Config.client_root() +@pytest.mark.parametrize("platform_id", ALL_IDS) +def test_a_sidecar_with_no_artifact_beside_it_counts_as_no_client(platform_id): + platform = BY_ID[platform_id] + root = Path(Config.client_root()) root.mkdir(parents=True, exist_ok=True) - (root / MANIFEST_NAME).write_text(json.dumps({"version_name": "x", "version_code": 1, "size": 1, "sha256": ""})) - assert android_release() is None + (root / platform.sidecar).write_text( + json.dumps({"version_name": "x", "version_code": 1, "size": 1, "sha256": ""}), + encoding="utf-8", + ) + assert release(platform_id) is None -async def test_metadata_endpoint_is_public_so_an_updater_can_ask_cheaply(app): - place_client() - resp = await app.test_client().get("/api/client/android") - assert resp.status_code == 200 - assert (await resp.get_json())["version_code"] == 216 +def test_an_unknown_platform_is_not_a_client(): + assert release("blackberry") is None -async def test_metadata_404s_rather_than_describing_a_client_that_is_not_there(app): - resp = await app.test_client().get("/api/client/android") - assert resp.status_code == 404 +# --- what a present client reports ------------------------------------------- -async def test_the_bytes_need_authentication_even_though_the_version_does_not(app): - """Anyone who can reach the port may ask what version exists; only an account - or a linked device may pull the 55 MiB.""" - place_client() - resp = await app.test_client().get("/api/client/android/download") - assert resp.status_code == 401 +@pytest.mark.parametrize("platform_id", ALL_IDS) +def test_a_present_client_reports_what_a_comparator_reads(platform_id): + place(platform_id) + found = release(platform_id) + assert found["version"] == "2026.08.30.0307" + # The integer is what decides "is this newer", not the name — a name is a string + # and sorts like one. + assert found["version_code"] == 3503708 + assert found["size"] == len(PAYLOAD) + assert found["platform"] == platform_id + # A PATH, not an absolute URL: the client joins it to the base it is already + # linked to, so a server cannot redirect the download elsewhere. + assert found["url"] == f"/api/client/{platform_id}/download" + assert not found["url"].startswith("http") + + +def test_the_android_payload_still_carries_every_field_it_used_to(): + """Phones in the field parse this. Fields may be ADDED — `ClientRelease` in + core/src/sync/client.rs is a plain serde struct and ignores what it does not + know — but none of these may move or change meaning.""" + place("android") + found = android_release() + for key in ("version", "version_code", "size", "sha256", "url"): + assert key in found, key + assert found["url"] == "/api/client/android/download" + + +# --- the signed bundle ------------------------------------------------------- + + +def test_the_appimage_publishes_its_signature_with_its_version(): + """One request returns both, so an updater cannot pair a version with a + signature belonging to a different build.""" + place("linux-appimage", signature="minisign output here") + assert release("linux-appimage")["signature"] == "minisign output here" + + +def test_an_appimage_without_a_signature_is_absent_rather_than_unsigned(): + """It is the only bundle that replaces itself in place, and an update the app + cannot verify is one it will refuse. Offering it unverifiable would turn a + missing file into a failed install on the user's machine.""" + place("linux-appimage", signature=None) + assert release("linux-appimage") is None + + +def test_an_empty_signature_file_is_not_a_signature(): + """A truncated copy leaves a zero-byte file, which reads as present.""" + place("linux-appimage", signature=" \n") + assert release("linux-appimage") is None + + +def test_only_the_appimage_carries_a_signature(): + """A package-manager install cannot replace itself in place, so nothing verifies + one and claiming a signature would imply an update path that does not exist.""" + for platform_id in ALL_IDS: + place(platform_id) + found = releases() + assert "signature" in found["linux-appimage"] + for platform_id in ALL_IDS: + if platform_id != "linux-appimage": + assert "signature" not in found[platform_id], platform_id + + +# --- the set, and precedence within it --------------------------------------- + + +def test_a_platform_the_server_lacks_is_simply_not_in_the_set(): + """Not null, not an error — a server holding some clients and not others is the + ordinary state, and the UI hides what is absent.""" + place("android") + place("windows") + found = releases() + assert set(found) == {"android", "windows"} def test_the_baked_in_copy_is_used_when_nothing_was_dropped_in(): """The ordinary case for a self-hoster who just pulled the image.""" - place_client(root=client_dist.BAKED_ROOT, version_name="0.1.300", version_code=300) + place("android", root=client_dist.BAKED_ROOT, version_code=300) assert android_release()["version_code"] == 300 def test_a_dropped_in_build_beats_the_one_the_image_shipped(): """Someone who deliberately put a build on the volume wants that build.""" - place_client(root=client_dist.BAKED_ROOT, version_name="0.1.300", version_code=300) - place_client(version_name="0.1.99", version_code=99) - advertised = android_release() + place("android", root=client_dist.BAKED_ROOT, version_code=300) + place("android", version_code=99) # Lower version and all — precedence is about intent, not about newness. An # operator pinning an older client is doing it on purpose. - assert advertised["version_code"] == 99 + assert android_release()["version_code"] == 99 + + +def test_precedence_is_decided_per_platform_not_for_the_whole_set(): + """THE trap this table introduces. Dropping in one client must not retract the + other four — "first directory holding anything wins" would mean overriding the + APK silently takes the desktop downloads offline.""" + for platform_id in ALL_IDS: + place(platform_id, root=client_dist.BAKED_ROOT, version_code=300) + place("android", version_code=99) + found = releases() + assert found["android"]["version_code"] == 99 + assert found["linux-deb"]["version_code"] == 300 + assert set(found) == set(ALL_IDS) def test_a_broken_drop_in_does_not_shadow_the_baked_copy(): @@ -149,6 +280,79 @@ def test_a_broken_drop_in_does_not_shadow_the_baked_copy(): This is the failure the copy-order advice in docs/android-distribution.md is about, and the server should ride it out rather than go dark. """ - place_client(root=client_dist.BAKED_ROOT, version_name="0.1.300", version_code=300) - place_client(size=999_999) # sidecar describing a different build + place("android", root=client_dist.BAKED_ROOT, version_code=300) + place("android", size=999_999) # sidecar describing a different build assert android_release()["version_code"] == 300 + + +# --- the advertisement ------------------------------------------------------- + + +def test_the_advertisement_carries_the_whole_table(): + place("linux-deb") + assert set(advertisement()["clients"]) == {"linux-deb"} + + +def test_the_advertisement_still_carries_the_key_phones_already_read(): + """Not deprecated in the change that introduces its replacement. An installed + Android client reads `android_client`, and one duplicated dict is what it costs + to not strand it.""" + place("android") + data = advertisement() + assert data["android_client"] == data["clients"]["android"] + + +def test_no_android_client_means_no_android_key_even_when_others_are_present(): + place("windows") + data = advertisement() + assert "android_client" not in data + assert set(data["clients"]) == {"windows"} + + +# --- routes ------------------------------------------------------------------ + + +@pytest.mark.parametrize("platform_id", ALL_IDS) +async def test_metadata_endpoint_is_public_so_an_updater_can_ask_cheaply(app, platform_id): + place(platform_id) + resp = await app.test_client().get(f"/api/client/{platform_id}") + assert resp.status_code == 200 + assert (await resp.get_json())["version_code"] == 3503708 + + +@pytest.mark.parametrize("platform_id", ALL_IDS) +async def test_metadata_404s_rather_than_describing_a_client_that_is_not_there(app, platform_id): + resp = await app.test_client().get(f"/api/client/{platform_id}") + assert resp.status_code == 404 + + +async def test_an_unknown_platform_404s_like_an_absent_one(app): + """Same answer to the caller either way, and telling them apart would only tell + an unauthenticated stranger which platforms this build of the server knows.""" + resp = await app.test_client().get("/api/client/blackberry") + assert resp.status_code == 404 + + +async def test_the_index_returns_everything_in_one_request(app): + place("android") + place("linux-appimage") + resp = await app.test_client().get("/api/client") + assert resp.status_code == 200 + assert set((await resp.get_json())["clients"]) == {"android", "linux-appimage"} + + +async def test_the_index_is_an_empty_set_rather_than_a_404(app): + """A server with no clients has an answer; it is just an empty one. 404 here + would make the UI treat "nothing to offer" as a broken endpoint.""" + resp = await app.test_client().get("/api/client") + assert resp.status_code == 200 + assert (await resp.get_json())["clients"] == {} + + +@pytest.mark.parametrize("platform_id", ALL_IDS) +async def test_the_bytes_need_authentication_even_though_the_version_does_not(app, platform_id): + """Anyone who can reach the port may ask what version exists; only an account or + a linked device may pull the payload.""" + place(platform_id) + resp = await app.test_client().get(f"/api/client/{platform_id}/download") + assert resp.status_code == 401 -- 2.54.0 From ff6e99eb6269b28d5e374bf09704f77961851578 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 30 Aug 2026 13:11:04 -0400 Subject: [PATCH 004/214] image: bake every client in, not just the phone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ~104 MB on top of ~85 MB, almost all of it the AppImage. That is what the product being complete costs (rule 23): a self-hoster gets a working app for their machine from the server holding their notes, with no account on a forge that is private. The AppImage is not optional within that — it is the only bundle that can replace itself in place, so a server without one cannot serve in-app updates to anybody. `packaging/fetch-clients.sh` replaces the inline fetch and writes the fixed names and sidecars `client_dist.py` reads. It never fails: a platform with nothing published means the server advertises nothing for it and the UI hides that download, and eight fetches must not become eight ways to redden a green lane. THE VERSION IS FETCHED, NOT DERIVED, and this is the part that would have been wrong the easy way. The obvious shortcut is `version.sh display desktop` in the image job — it has the checkout. But this commit may not be the commit the channel is serving: a push touching only `src/` does not rebuild the desktop, so the channel still holds an older build and a locally-derived version would describe those bytes with this commit's number. `client_dist.py`'s size check could not catch it, because size IS measured from the real file — it would sail through and lie about the version alone. So `write-manifest.sh` now publishes `thoughtsync-desktop.json` beside `latest.json`, from the same two values in the same breath, and only size/sha256 are measured at bake time. Which needed the prune's keep-list, or the sidecar would have been uploaded and deleted again in the same run — a fixed name is self-limiting, which is exactly why that list exists. `version_code` is NOT uniformly an integer, and coercing it was a leftover from the days when Android was the only platform. Android's must stay a JSON number: `ClientRelease` in core declares it `i64` and a string fails to deserialize on every phone in the field. The desktop's is Tauri's semver key `1.0.` — the value its updater actually compares — and `int()` would have rejected every desktop sidecar CI writes. The table now says which is which, and tests pin both directions. Also retires the comment above the fetch step, which claimed the APK came from "always the rolling dev release" and mentioned `:` images. M314 step 3 made the channel conditional in the code directly below it, and step 6 removed version-shaped image tags entirely. Verified against the live dev channel before pushing: the Android half resolves and exits 0, the desktop half degrades with a warning because the sidecar does not exist yet, and all five constructed bundle filenames return 200. --- .forgejo/workflows/ci.yml | 60 +++++------ .forgejo/workflows/desktop.yml | 9 +- Dockerfile | 20 ++-- desktop/packaging/write-manifest.sh | 59 +++++++--- packaging/fetch-clients.sh | 161 ++++++++++++++++++++++++++++ src/thoughtsync/client_dist.py | 25 ++++- tests/test_client_dist.py | 42 +++++++- 7 files changed, 311 insertions(+), 65 deletions(-) create mode 100755 packaging/fetch-clients.sh diff --git a/.forgejo/workflows/ci.yml b/.forgejo/workflows/ci.yml index a842cb7..f77daea 100644 --- a/.forgejo/workflows/ci.yml +++ b/.forgejo/workflows/ci.yml @@ -323,53 +323,41 @@ jobs: docker system prune -af || true docker builder prune --keep-storage 5g -f || true - # Bake the Android client in, on EVERY image build, so :dev, :latest and - # : all carry one and a `docker compose pull` delivers a new client - # along with the new server. + # Bake EVERY client in, on every image build, so a self-hoster gets a working + # app for their machine from the server holding their notes — without an + # account on this forge, which is private (issue 2091) and is why serving them + # from a release page was never an option for anybody but the operator. # - # Always the rolling `dev` release — the newest build there is. A versioned - # image therefore carries the newest client rather than one pinned to that - # version; the two negotiate a sync protocol version before linking, so - # "newest" is safe in a way "matching" would not buy anything over. + # ~104 MB on top of the ~85 MB image, almost all of it the AppImage. That is + # the price of the product being complete (rule 23), and the AppImage is not + # optional within it: it is the ONLY bundle that can replace itself in place, + # so a server without one cannot serve in-app updates to anyone. # - # Fetched by the JOB, not by the Dockerfile: the release is private, and a + # Fetched by the JOB, not by the Dockerfile: the releases are private, and a # token used inside a build lands in the context or a layer. # - # NEVER fails the build. An image with no Android client advertises none and - # hides the download — a supported state, and the only one available before - # the first Android build has ever published. - - name: Fetch the Android client to bake in + # NEVER fails the build — see the script. A platform with nothing published + # means the server advertises nothing for it and the UI hides that download, + # which is a supported state and the only one available before that platform's + # first build has ever published. + - name: Fetch the clients to bake in env: GITHUB_TOKEN: ${{ github.token }} + GITHUB_SERVER_URL: ${{ github.server_url }} + GITHUB_REPOSITORY: ${{ github.repository }} run: | - mkdir -p client - # THE CHANNEL IS A PROPERTY OF THE IMAGE. A :dev image serves the dev - # client; :latest serves the stable one. This read `download/dev` - # unconditionally until M314 step 3, on every branch — so every stable - # server shipped a dev-channel APK to anyone who downloaded the client - # from it. Not a versioning gap; a plain defect, fixed here because this - # is the step that gave `stable` an APK to point at. + # THE CHANNEL IS A PROPERTY OF THE IMAGE. A :dev image serves dev clients; + # :latest serves stable ones. This read `download/dev` unconditionally + # until M314 step 3, on every branch — so every stable server shipped a + # dev-channel APK to anyone who downloaded the client from it. Not a + # versioning gap; a plain defect, and the reason the channel is chosen here + # rather than inside the script: the caller is what knows which image it is + # building. case "${{ github.ref_name }}" in main) channel=stable ;; *) channel=dev ;; esac - echo "Baking in the $channel client." - base="${{ github.server_url }}/${{ github.repository }}/releases/download/$channel" - ok=1 - for f in thoughtsync.apk thoughtsync-android.json; do - curl -fsSL -H "Authorization: token $GITHUB_TOKEN" -o "client/$f" "$base/$f" || ok=0 - done - if [ "$ok" = 1 ]; then - echo "Baking in:" - cat client/thoughtsync-android.json - ls -l client/thoughtsync.apk - else - # Both or neither. Half a pair is worse than none: the server would - # read a sidecar describing an APK that isn't there, or an APK it - # cannot state a version for. - echo "::warning::No Android client on the dev release — this image ships without one." - rm -f client/thoughtsync.apk client/thoughtsync-android.json - fi + sh packaging/fetch-clients.sh "$channel" client - name: Set up Docker Buildx uses: docker/setup-buildx-action@v4 diff --git a/.forgejo/workflows/desktop.yml b/.forgejo/workflows/desktop.yml index 9c12c20..0b51bde 100644 --- a/.forgejo/workflows/desktop.yml +++ b/.forgejo/workflows/desktop.yml @@ -463,6 +463,12 @@ jobs: # match the binary it points at is an updater that never settles. It must # be `key`: this value is matched against bundle filenames. version="$(sh packaging/version.sh key desktop)" + # The version a PERSON reads, published beside the manifest as + # `thoughtsync-desktop.json`. The image build reads it to describe the + # bundles it bakes in (packaging/fetch-clients.sh) without re-deriving + # anything from its own checkout — which would be a different commit + # whenever the desktop did not rebuild. + display="$(sh packaging/version.sh display desktop)" # Both channels are rolling: the manifest lands on the same release that # holds the bundles, and the previous build's bundles are dropped once it # points at this one. Nothing can reach them, and they are ~100 MB a push. @@ -476,4 +482,5 @@ jobs: export RELEASE_NOTES="Development build from ${GITHUB_SHA}" ;; esac export PRUNE_OLD_ASSETS=true - APP_VERSION="$version" bash desktop/packaging/write-manifest.sh + APP_VERSION="$version" DISPLAY_VERSION="$display" \ + bash desktop/packaging/write-manifest.sh diff --git a/Dockerfile b/Dockerfile index 2ffe9ce..d8106e3 100644 --- a/Dockerfile +++ b/Dockerfile @@ -24,17 +24,23 @@ COPY --from=build-frontend /build/dist/ src/thoughtsync/static/ COPY alembic.ini . COPY alembic/ alembic/ -# The Android client this server hands out. CI fetches the newest published build -# into ./client immediately before this runs (ci.yml), so every image tag — :dev, -# :latest and : alike — ships a client, and a `docker compose pull` -# delivers a new one with no file copying by hand. +# The clients this server hands out — the APK and all four desktop bundles. CI +# fetches the newest published build of each into ./client immediately before this +# runs (packaging/fetch-clients.sh), so both image tags ship a full set and a +# `docker compose pull` delivers new ones with no file copying by hand. # -# Fetched by the JOB rather than here on purpose: the release is private, and a +# ~104 MB of this image is that set, almost all of it the AppImage. +# +# Fetched by the JOB rather than here on purpose: the releases are private, and a # token used inside a build ends up in the build context or a layer. # +# LAST of the COPYs, deliberately: this directory changes on every build, so +# putting it above the `pip install` layer would invalidate that layer every time. +# # The directory is tracked (client/.keep) so this COPY cannot fail on a tree where -# that step never ran. An image with no APK is a supported state — the server -# advertises nothing and the web UI hides the download (client_dist.py). +# that step never ran. An image with no clients — or with some and not others — is +# a supported state: the server advertises what it has and the web UI hides the +# rest (client_dist.py). COPY client/ src/thoughtsync/client/ ENV PYTHONPATH=/app/src diff --git a/desktop/packaging/write-manifest.sh b/desktop/packaging/write-manifest.sh index 60351de..3d0ed46 100644 --- a/desktop/packaging/write-manifest.sh +++ b/desktop/packaging/write-manifest.sh @@ -24,6 +24,11 @@ set -euo pipefail : "${GITHUB_REPOSITORY:?GITHUB_REPOSITORY is required (owner/repo)}" : "${RELEASE_TAG:?RELEASE_TAG is required (the release holding the bundles)}" : "${APP_VERSION:?APP_VERSION is required (the version the bundles carry)}" +# The version a PERSON reads, published beside the manifest so the image build can +# describe the bundles it bakes in without re-deriving anything. Required rather +# than defaulted: a missing value here would silently publish a sidecar naming the +# wrong build, and there is nothing downstream that could catch it. +: "${DISPLAY_VERSION:?DISPLAY_VERSION is required (the human-readable version)}" # The manifest is published to the release that HOLDS the bundles. There is no # second place any more. @@ -116,27 +121,47 @@ pub_date="$(date -u '+%Y-%m-%dT%H:%M:%SZ')" echo "==> Manifest:" cat "$work/latest.json" -# The manifest goes on the same release the bundles were just read from — which is -# also the one `publish-release.sh` created or refreshed moments earlier, so it is +# Both files go on the same release the bundles were just read from — which is also +# the one `publish-release.sh` created or refreshed moments earlier, so it is # guaranteed to exist by the time this runs. -target_id="$release_id" -target_assets="$assets" # Replace rather than duplicate: Forgejo rejects a second asset with the same name, -# and this file is rewritten on every publish by design. -old_id="$(printf '%s' "$target_assets" \ - | grep -oE "\"id\"[[:space:]]*:[[:space:]]*[0-9]+[^}]*\"name\"[[:space:]]*:[[:space:]]*\"latest\.json\"" \ - | head -1 | grep -oE '[0-9]+' | head -1 || true)" -if [ -n "${old_id:-}" ]; then - echo "==> Removing the previous latest.json (id $old_id)" - curl -fsS -X DELETE "${AUTH[@]}" "$API/releases/$target_id/assets/$old_id" >/dev/null -fi +# and these files are rewritten on every publish by design. +replace_asset() { + local path="$1" name="$2" escaped old_id + escaped="${name//./\\.}" + old_id="$(printf '%s' "$assets" \ + | grep -oE "\"id\"[[:space:]]*:[[:space:]]*[0-9]+[^}]*\"name\"[[:space:]]*:[[:space:]]*\"$escaped\"" \ + | head -1 | grep -oE '[0-9]+' | head -1 || true)" + if [ -n "${old_id:-}" ]; then + echo "==> Removing the previous $name (id $old_id)" + curl -fsS -X DELETE "${AUTH[@]}" "$API/releases/$release_id/assets/$old_id" >/dev/null + fi + echo "==> Uploading $name to $RELEASE_TAG" + curl -fsS -X POST "${AUTH[@]}" "$API/releases/$release_id/assets?name=$name" \ + -F "attachment=@$path" >/dev/null +} -echo "==> Uploading latest.json to $RELEASE_TAG" -curl -fsS -X POST "${AUTH[@]}" "$API/releases/$target_id/assets?name=latest.json" \ - -F "attachment=@$work/latest.json" >/dev/null +replace_asset "$work/latest.json" "latest.json" -echo "==> Done. $RELEASE_TAG now advertises $APP_VERSION for ${#entries[@]} platform(s)." +# The version pair, for whoever needs to describe these bundles without rebuilding +# them — today the image build, which bakes the desktop clients in and writes each +# one a sidecar (`packaging/fetch-clients.sh`). +# +# It is published HERE, beside the manifest, because this is the step that speaks +# for what the channel serves: both files are written in the same breath from the +# same two values, so they cannot disagree about which build is current. A consumer +# deriving the version from its own checkout instead would describe these bytes +# with whatever commit it happened to be on. +# +# No `size` or `sha256` — those are per-artifact and there are four. Whoever +# downloads a bundle measures the bytes it actually got, which is the only way to +# tell a truncated download from a whole one. +printf '{\n "version_name": "%s",\n "version_code": "%s"\n}\n' \ + "$DISPLAY_VERSION" "$APP_VERSION" > "$work/thoughtsync-desktop.json" +replace_asset "$work/thoughtsync-desktop.json" "thoughtsync-desktop.json" + +echo "==> Done. $RELEASE_TAG now advertises $DISPLAY_VERSION ($APP_VERSION) for ${#entries[@]} platform(s)." # --- prune superseded builds from a rolling channel --------------------------- # @@ -169,7 +194,7 @@ if [ "${PRUNE_OLD_ASSETS:-false}" = "true" ]; then # every desktop push regardless. That is exactly what happened on run # 4098, which swept the APK run 4092 had just published. case "$asset_name" in - latest.json|thoughtsync.apk|thoughtsync-android.json) continue ;; + latest.json|thoughtsync-desktop.json|thoughtsync.apk|thoughtsync-android.json) continue ;; *"$APP_VERSION"*) continue ;; esac echo " removing $asset_name" diff --git a/packaging/fetch-clients.sh b/packaging/fetch-clients.sh new file mode 100755 index 0000000..480cdb9 --- /dev/null +++ b/packaging/fetch-clients.sh @@ -0,0 +1,161 @@ +#!/usr/bin/env sh +# +# Collect every client this image should hand out, into one directory. +# +# fetch-clients.sh +# +# The server serves clients from `DATA_DIR/client/` or from the copy baked into the +# image (`client_dist.py`). This is what fills the second one. It runs in CI, right +# before `docker build`, and writes the FIXED filenames that module looks for. +# +# THE CHANNEL IS A PROPERTY OF THE IMAGE. A `:dev` image serves dev clients; +# `:latest` serves stable ones. Passed in rather than derived here, because the +# caller is the thing that knows which image it is building. +# +# NEVER FAILS. A platform with nothing published means the server advertises +# nothing for it and the UI hides that download — a supported state, and the only +# one available before a platform's first build has ever published. Turning eight +# fetches into eight ways to redden an otherwise fine lane would be strictly worse +# than shipping an image that offers four clients instead of five. +# +# WHY THE VERSION IS FETCHED AND NOT DERIVED. The obvious shortcut is to run +# `version.sh display desktop` here — this job has the checkout, after all. It is +# wrong: this commit may not be the commit the channel is serving. A push touching +# only `src/` does not rebuild the desktop, so the channel still holds an older +# build, and a locally-derived version would describe those bytes with this +# commit's number. The size check in `client_dist.py` would not catch it, because +# the size IS measured from the real file — it would sail through and lie about the +# version only. So the version comes from the channel, beside the bytes it +# describes, and only `size`/`sha256` are measured here. +set -eu + +channel="${1:?usage: fetch-clients.sh }" +dest="${2:?usage: fetch-clients.sh }" + +case "$channel" in dev|stable) : ;; *) + echo "fetch-clients.sh: unknown channel '$channel'" >&2; exit 2 ;; +esac + +SERVER="${GITHUB_SERVER_URL:-https://git.fabledsword.com}" +REPO="${GITHUB_REPOSITORY:-bvandeusen/thoughtsync}" +BASE="$SERVER/$REPO/releases/download/$channel" + +mkdir -p "$dest" + +# Authenticated when we have a token — these releases are private (issue 2091), so +# on this instance we always do. Anonymous still works against a public fork. +fetch() { + if [ -n "${GITHUB_TOKEN:-}" ]; then + curl -fsSL -H "Authorization: token $GITHUB_TOKEN" -o "$2" "$1" + else + curl -fsSL -o "$2" "$1" + fi +} + +# One field out of a small flat JSON object. `grep`/`sed` rather than a parser +# because this runs in the CI image's busybox sh and adding a jq dependency to buy +# one string is not a trade worth making. The sidecars are written by us and are +# one level deep. +field() { + grep -oE "\"$2\"[[:space:]]*:[[:space:]]*\"?[^,\"}]+\"?" "$1" 2>/dev/null \ + | head -1 | sed -E 's/.*:[[:space:]]*"?([^"]*)"?[[:space:]]*$/\1/' +} + +bytes() { wc -c < "$1" | tr -d ' '; } +digest() { sha256sum "$1" | cut -d' ' -f1; } + +# The sidecar shape `client_dist.py` reads. `size` and `sha256` are measured from +# the file that actually landed, so a truncated download cannot be described as a +# whole one. +sidecar() { + _file="$1"; _out="$2"; _name="$3"; _code="$4" + printf '{\n "version_name": "%s",\n "version_code": %s,\n "size": %s,\n "sha256": "%s"\n}\n' \ + "$_name" "$_code" "$(bytes "$_file")" "$(digest "$_file")" > "$_out" +} + +echo "==> Collecting the $channel clients" + +# --- Android ----------------------------------------------------------------- +# +# Its sidecar is published whole by the Android lane — an APK keeps its version in +# a binary AXML manifest, so the values are recorded where they were already known. +# Copied verbatim rather than rebuilt here. +if fetch "$BASE/thoughtsync.apk" "$dest/thoughtsync.apk" && + fetch "$BASE/thoughtsync-android.json" "$dest/thoughtsync-android.json"; then + echo " android $(field "$dest/thoughtsync-android.json" version_name)" +else + # Both or neither. Half a pair is worse than none: the server would read a + # sidecar describing an APK that is not there, or an APK it cannot state a + # version for. + echo "::warning::No Android client on the $channel channel — this image ships without one." + rm -f "$dest/thoughtsync.apk" "$dest/thoughtsync-android.json" +fi + +# --- desktop ----------------------------------------------------------------- +# +# One sidecar on the channel carries the version PAIR for all four bundles, because +# they are one build: `version_name` is what a person reads, `version_code` is the +# ordering key, and the key is also what the bundle filenames are stamped with. +# Written by `write-manifest.sh`, which is the step that speaks for what the channel +# serves. +bake_desktop() { + desk="$dest/.desktop-release.json" + if ! fetch "$BASE/thoughtsync-desktop.json" "$desk"; then + echo "::warning::No desktop release on the $channel channel — this image ships without desktop clients." + rm -f "$desk" + return 0 + fi + + name="$(field "$desk" version_name)" + key="$(field "$desk" version_code)" + rm -f "$desk" + + if [ -z "$name" ] || [ -z "$key" ]; then + echo "::warning::The $channel desktop sidecar named no version — skipping desktop clients." + return 0 + fi + echo " desktop $name (key $key)" + + # Bundle filenames are stamped with the ORDERING KEY — what Tauri puts in them, + # and what `write-manifest.sh` already selects on. Constructed rather than + # discovered from the release's asset list: one shape, no JSON walk, and a name + # that does not resolve is caught by the fetch failing rather than by matching + # the wrong file. + # + # `||` + for row in \ + "linux-deb|ThoughtSync_${key}_amd64.deb|thoughtsync.deb" \ + "linux-pacman|thoughtsync-${key}-1-x86_64.pkg.tar.zst|thoughtsync.pkg.tar.zst" \ + "linux-appimage|ThoughtSync_${key}_amd64.AppImage|thoughtsync.AppImage" \ + "windows|ThoughtSync_${key}_x64-setup.exe|thoughtsync-setup.exe" + do + id="${row%%|*}"; rest="${row#*|}" + remote="${rest%%|*}"; local_name="${rest#*|}" + + if ! fetch "$BASE/$remote" "$dest/$local_name"; then + echo "::warning::$channel has no $remote — this image ships without the $id client." + rm -f "$dest/$local_name" + continue + fi + + # The AppImage is the only bundle that replaces itself in place, so the updater + # verifies a signature before it does. Without one it is not servable as an + # update, and `client_dist.py` treats it as absent rather than offering it + # unverifiable — so drop the bundle too rather than baking 95 MB nothing can use. + if [ "$id" = "linux-appimage" ]; then + if ! fetch "$BASE/$remote.sig" "$dest/$local_name.sig"; then + echo "::warning::$remote has no signature on $channel — dropping the AppImage." + rm -f "$dest/$local_name" "$dest/$local_name.sig" + continue + fi + fi + + sidecar "$dest/$local_name" "$dest/thoughtsync-$id.json" "$name" "$key" + echo " $id $(bytes "$dest/$local_name") bytes" + done +} + +bake_desktop + +echo "==> Baked in:" +ls -l "$dest" diff --git a/src/thoughtsync/client_dist.py b/src/thoughtsync/client_dist.py index a75ede4..a85c6c0 100644 --- a/src/thoughtsync/client_dist.py +++ b/src/thoughtsync/client_dist.py @@ -99,6 +99,18 @@ class Platform: # the only bundle that can replace itself in place — a package-manager install # cannot, by design (see the desktop's update.rs). signed: bool = False + # Whether this platform's ordering key is an INTEGER. + # + # `version_code` is "whatever this platform's comparator reads", and that is not + # one type. Android's is an int because Android's own install gate compares one, + # and it must stay a JSON number — `ClientRelease` in core/src/sync/client.rs + # declares it `i64` and a string would fail to deserialize on every phone in the + # field. The desktop's is Tauri's semver key, `1.0.`, which is the value + # its updater compares and is not an integer at all. + # + # Coercing everything to int was inherited from the days when Android was the + # only platform, and would have rejected every desktop sidecar written. + code_is_int: bool = True @property def signature(self) -> str: @@ -130,6 +142,7 @@ PLATFORMS: tuple[Platform, ...] = ( artifact="thoughtsync.deb", sidecar="thoughtsync-linux-deb.json", mimetype="application/vnd.debian.binary-package", + code_is_int=False, ), Platform( id="linux-pacman", @@ -137,6 +150,7 @@ PLATFORMS: tuple[Platform, ...] = ( artifact="thoughtsync.pkg.tar.zst", sidecar="thoughtsync-linux-pacman.json", mimetype="application/zstd", + code_is_int=False, ), Platform( id="linux-appimage", @@ -148,6 +162,7 @@ PLATFORMS: tuple[Platform, ...] = ( # cannot be wrong. mimetype="application/octet-stream", signed=True, + code_is_int=False, ), Platform( id="windows", @@ -155,6 +170,7 @@ PLATFORMS: tuple[Platform, ...] = ( artifact="thoughtsync-setup.exe", sidecar="thoughtsync-windows.json", mimetype="application/vnd.microsoft.portable-executable", + code_is_int=False, ), ) @@ -185,12 +201,19 @@ def _read(root: Path, platform: Platform) -> dict | None: size = (root / platform.artifact).stat().st_size meta = json.loads((root / platform.sidecar).read_text(encoding="utf-8")) version = str(meta["version_name"]) - code = int(meta["version_code"]) + # See `code_is_int`. Android's must parse as an integer; the desktop's is + # Tauri's semver key and is carried through as written. + code = int(meta["version_code"]) if platform.code_is_int else str(meta["version_code"]) recorded = int(meta["size"]) digest = str(meta["sha256"]) except (OSError, ValueError, TypeError, KeyError): return None + if not str(code).strip() or not version.strip(): + # A sidecar can be well-formed and still say nothing. An empty version is + # not a version, and it would render as a blank on the download card. + return None + # The pair has to describe one build. A sidecar left behind by a previous # release would otherwise advertise a version this server cannot serve, and the # client would download something other than what it was promised. diff --git a/tests/test_client_dist.py b/tests/test_client_dist.py index 5ef3eb2..7a257e9 100644 --- a/tests/test_client_dist.py +++ b/tests/test_client_dist.py @@ -34,6 +34,18 @@ PAYLOAD = b"not really a client, but the server only ever stats it" # four would ship untested. ALL_IDS = [p.id for p in PLATFORMS] +# `version_code` is "whatever this platform's comparator reads", and that is not one +# type. Android's install gate compares an integer; the desktop's updater compares +# Tauri's semver key. The tests carry both shapes for the same reason the module +# does — a suite that only ever wrote integers would pass while every desktop +# sidecar CI writes was being rejected. +ANDROID_CODE = 3503708 +DESKTOP_CODE = "1.0.3503707" + + +def code_for(platform_id: str): + return ANDROID_CODE if BY_ID[platform_id].code_is_int else DESKTOP_CODE + @pytest.fixture(autouse=True) def _empty_baked_client(tmp_path, monkeypatch): @@ -71,7 +83,7 @@ def place( (root / platform.artifact).write_bytes(payload) meta = { "version_name": "2026.08.30.0307", - "version_code": 3503708, + "version_code": code_for(platform_id), "size": len(payload), "sha256": "ab" * 32, } @@ -164,6 +176,30 @@ def test_a_sidecar_with_no_artifact_beside_it_counts_as_no_client(platform_id): assert release(platform_id) is None +def test_androids_code_must_be_an_integer(): + """`ClientRelease` in core/src/sync/client.rs declares it `i64`. A string here + would fail to deserialize on every phone in the field, so a sidecar carrying one + is not a client this server can honestly offer.""" + place("android", version_code="1.0.3503707") + assert release("android") is None + + +def test_the_desktop_keeps_tauris_semver_key_verbatim(): + """It is not an integer and must not be coerced into one: this is the value the + desktop updater compares, and `1.0.3503707` truncated to `1` orders against + nothing.""" + place("linux-deb") + assert release("linux-deb")["version_code"] == "1.0.3503707" + + +@pytest.mark.parametrize("platform_id", ALL_IDS) +def test_an_empty_version_name_counts_as_no_client(platform_id): + """A sidecar can be well-formed and still say nothing. A blank would render as + an empty space on the download card, which reads as a layout bug.""" + place(platform_id, version_name="") + assert release(platform_id) is None + + def test_an_unknown_platform_is_not_a_client(): assert release("blackberry") is None @@ -178,7 +214,7 @@ def test_a_present_client_reports_what_a_comparator_reads(platform_id): assert found["version"] == "2026.08.30.0307" # The integer is what decides "is this newer", not the name — a name is a string # and sorts like one. - assert found["version_code"] == 3503708 + assert found["version_code"] == code_for(platform_id) assert found["size"] == len(PAYLOAD) assert found["platform"] == platform_id # A PATH, not an absolute URL: the client joins it to the base it is already @@ -317,7 +353,7 @@ async def test_metadata_endpoint_is_public_so_an_updater_can_ask_cheaply(app, pl place(platform_id) resp = await app.test_client().get(f"/api/client/{platform_id}") assert resp.status_code == 200 - assert (await resp.get_json())["version_code"] == 3503708 + assert (await resp.get_json())["version_code"] == code_for(platform_id) @pytest.mark.parametrize("platform_id", ALL_IDS) -- 2.54.0 From d2f9d316cffd1c896d6554500c8e46b46056d663 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 30 Aug 2026 13:19:18 -0400 Subject: [PATCH 005/214] tests: 300 comes back as "300" from a platform whose key is not an integer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The precedence test wrote `version_code=300` for all five platforms and compared the desktop's against the int it wrote. It comes back as `"300"`, because the module preserves each platform's own comparator type instead of flattening both to int — which is the behaviour the change it was testing had just introduced. A `coded()` helper now says which shape to expect and why, and the assertion runs over every non-Android platform rather than spot-checking `linux-deb`. The test caught a real inconsistency in itself precisely because it compared against a concrete value rather than round-tripping what it wrote. --- tests/test_client_dist.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/test_client_dist.py b/tests/test_client_dist.py index 7a257e9..812e04a 100644 --- a/tests/test_client_dist.py +++ b/tests/test_client_dist.py @@ -47,6 +47,16 @@ def code_for(platform_id: str): return ANDROID_CODE if BY_ID[platform_id].code_is_int else DESKTOP_CODE +def coded(platform_id: str, value: int): + """`value` in the shape that platform's sidecar carries. + + A test writing `version_code=300` gets `300` back from Android and `"300"` from + a desktop platform, because the module preserves each platform's own comparator + type rather than flattening both to int. + """ + return value if BY_ID[platform_id].code_is_int else str(value) + + @pytest.fixture(autouse=True) def _empty_baked_client(tmp_path, monkeypatch): """Point the baked-in copy at an empty directory. @@ -306,7 +316,9 @@ def test_precedence_is_decided_per_platform_not_for_the_whole_set(): place("android", version_code=99) found = releases() assert found["android"]["version_code"] == 99 - assert found["linux-deb"]["version_code"] == 300 + for platform_id in ALL_IDS: + if platform_id != "android": + assert found[platform_id]["version_code"] == coded(platform_id, 300), platform_id assert set(found) == set(ALL_IDS) -- 2.54.0 From 8a75e5f340c88c05753b79a7ac0a721475bafbb4 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 30 Aug 2026 13:21:03 -0400 Subject: [PATCH 006/214] clients: an unquoted 1.0.3504551 is not JSON, and every sidecar was one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `fetch-clients.sh` wrote `"version_code": %s` unquoted, which was right when the only ordering key in sight was Android's integer. The desktop's is Tauri's `1.0.`, and unquoted that is not valid JSON at all — so `json.loads` raised on all four generated sidecars and the server advertised nothing. A silent zero, not an error: `_read` treats a malformed sidecar as "no client here", which is right for a corrupt drop-in and indistinguishable from this. Caught by running the real fetch against the live dev channel and feeding the result to the real resolver, rather than by reading the printf. Also makes `_resolve` wrap BOTH candidate roots in Path(). Only the first was, and the asymmetry fails the same quiet way: a str `/` str raises TypeError, `_read` catches it, and a perfectly good directory reads as empty. The whole pipeline now resolves end to end against the live channel — five of five platforms, every sidecar valid JSON, one human-readable version across all of them with each artifact keeping its own comparator type: android 2026.08.30.1711 code=3504552 57.6 MB linux-appimage 2026.08.30.1711 code='1.0.3504551' 95.3 MB signed linux-deb 2026.08.30.1711 code='1.0.3504551' 3.3 MB linux-pacman 2026.08.30.1711 code='1.0.3504551' 2.7 MB windows 2026.08.30.1711 code='1.0.3504551' 2.6 MB --- packaging/fetch-clients.sh | 7 ++++++- src/thoughtsync/client_dist.py | 5 ++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/packaging/fetch-clients.sh b/packaging/fetch-clients.sh index 480cdb9..4b96266 100755 --- a/packaging/fetch-clients.sh +++ b/packaging/fetch-clients.sh @@ -69,7 +69,12 @@ digest() { sha256sum "$1" | cut -d' ' -f1; } # whole one. sidecar() { _file="$1"; _out="$2"; _name="$3"; _code="$4" - printf '{\n "version_name": "%s",\n "version_code": %s,\n "size": %s,\n "sha256": "%s"\n}\n' \ + # `version_code` is QUOTED here, and that is not a slip. This function only ever + # writes DESKTOP sidecars, whose ordering key is Tauri's `1.0.` — which + # unquoted is not valid JSON at all, so every sidecar this wrote would fail to + # parse and the server would advertise nothing. Android's sidecar is a different + # file, copied verbatim from its lane, and keeps its integer. + printf '{\n "version_name": "%s",\n "version_code": "%s",\n "size": %s,\n "sha256": "%s"\n}\n' \ "$_name" "$_code" "$(bytes "$_file")" "$(digest "$_file")" > "$_out" } diff --git a/src/thoughtsync/client_dist.py b/src/thoughtsync/client_dist.py index a85c6c0..6cba4be 100644 --- a/src/thoughtsync/client_dist.py +++ b/src/thoughtsync/client_dist.py @@ -263,7 +263,10 @@ def _resolve(platform: Platform) -> tuple[Path, dict] | None: broken or half-copied pair does NOT shadow the image: it simply is not a client, so the search moves on. """ - for root in (Path(Config.client_root()), BAKED_ROOT): + # Both wrapped in Path(), not just the first. The asymmetry was arbitrary and it + # fails quietly: `_read` catches the TypeError a str `/` str raises and reports + # "no client here", so a directory that is perfectly fine reads as empty. + for root in (Path(Config.client_root()), Path(BAKED_ROOT)): release = _read(root, platform) if release is not None: return root, release -- 2.54.0 From fd1e4ae4876ec817d6182e780854a6ffacf4d7ac Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 31 Aug 2026 07:58:17 -0400 Subject: [PATCH 007/214] downloads: five clients, and the page leads with the one that fits you MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Account page offered the APK and nothing else, because the APK was all the server held. Step 3 baked in four more, so the single card had to become a section — and five artifacts is exactly where a downloads page turns into a table of filenames and stops being a product. So it LEADS with what fits the machine asking, from the user agent, and keeps the rest quiet but visible. A wrong guess costs nothing: nothing is behind a disclosure and every other client is one click away. Linux gets all three at once, because the UA says "Linux" and nothing about dpkg or pacman — there is no better answer available. They are named for the distro rather than the package format, since a person knows which system they run and not necessarily which packaging it uses. The AppImage carries one clause of its own: it is 95 MB against 3, and it is also the only bundle that updates itself in place. Both facts belong to the same decision. macOS and iOS lead with nothing and say so. There is no build for either, and "There's no macOS build yet" is the difference between deliberate and broken. The version renders `unknown` rather than blank, and the download stays offered — not knowing which build it is, is not a reason to withhold it. Two things this did NOT do, both deliberate: The task asked for a Tauri case — do not offer the desktop app to someone already running it. That case cannot be reached: `/account` redirects to the board in the desktop app (requiresServer, router/index.ts), because device tokens are a server-side concept. A branch for it would be dead code. `.btn-link` mirrors BaseButton's declarations rather than replacing them. BaseButton is a