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"