From 7aabfe4e2a63db8d0507debf1075e463dfd9525e Mon Sep 17 00:00:00 2001 From: maziggy Date: Wed, 29 Apr 2026 12:17:25 +0200 Subject: [PATCH] fix(updates): install the discovered release tag, not hardcoded origin/main MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The in-app updater ran `git fetch origin main && git reset --hard origin/main` regardless of which version the GitHub releases API reported as latest. So whenever the latest release lived on a branch other than main — e.g. during a beta cycle when 0.2.4b1 sits on its own branch and main still points at the previous stable — clicking Apply Update appeared to succeed but the user actually stayed pinned to old main HEAD. Fix: extract `_discover_target_release(db)` mirroring the same release-API + include_beta_updates selection the GUI's update-check already uses, pass the resolved tag (e.g. `v0.2.4b1`) into `_perform_update(target_ref)`, and run `git fetch --prune --tags origin && git reset --hard `. The fetch now pulls --tags so a tag ref is locally resolvable; the reset takes the caller's ref instead of a hardcoded branch. apply_update now returns a clear error if no release resolves, instead of silently kicking off an update that can't land. --- backend/app/api/routes/updates.py | 92 ++++++++++++- backend/tests/integration/test_updates_api.py | 130 +++++++++++++++++- 2 files changed, 211 insertions(+), 11 deletions(-) diff --git a/backend/app/api/routes/updates.py b/backend/app/api/routes/updates.py index 938189c2f..f940e27d4 100644 --- a/backend/app/api/routes/updates.py +++ b/backend/app/api/routes/updates.py @@ -383,8 +383,61 @@ async def check_for_updates( } -async def _perform_update(): - """Perform the actual update using git fetch and reset.""" +async def _discover_target_release(db: AsyncSession) -> str | None: + """Look up the tag we should install from GitHub releases. + + Same selection logic the GUI's update-check uses: respect + `include_beta_updates`, skip prereleases when the user opted out, take + the first matching release. Returns the raw tag name (e.g. `v0.2.4b1`) + so the git ref is unambiguous, or None if there's no release to install. + + The previous in-app updater path was hardcoded to `git fetch origin main + && git reset --hard origin/main`, which silently no-ops whenever main + isn't where the latest release lives — e.g. during a beta release cycle + where the next stable hasn't been merged to main yet. Anchoring to the + release tag instead lets the GUI install whatever GitHub says is latest. + """ + result = await db.execute(select(Settings).where(Settings.key == "include_beta_updates")) + beta_setting = result.scalar_one_or_none() + include_beta = beta_setting and beta_setting.value.lower() == "true" + + try: + async with httpx.AsyncClient() as client: + response = await client.get( + f"https://api.github.com/repos/{GITHUB_REPO}/releases?per_page=20", + headers={"Accept": "application/vnd.github.v3+json"}, + timeout=10.0, + ) + response.raise_for_status() + releases = response.json() + except (httpx.HTTPError, ValueError) as exc: + logger.error("Could not fetch GitHub releases for update target: %s", exc) + return None + + for release in releases: + tag = release.get("tag_name", "") + if not tag: + continue + if include_beta: + return tag + # Skip prereleases (parsed from version, not GitHub flag — GitHub's + # is_prerelease flag isn't always set on dailies). + parsed = parse_version(tag) + if parsed[4] == 0: + return tag + return None + + +async def _perform_update(target_ref: str): + """Perform the actual update using git fetch and reset. + + `target_ref` is whatever git ref the caller wants to land on — typically + a release tag like `v0.2.4b1` resolved by `_discover_target_release`, + but accepts any ref `git reset --hard` understands (`origin/main`, a + branch, a sha). Tag-based refs are the production path because they pin + the install to a specific release artifact instead of whatever happens + to be on a moving branch. + """ global _update_status try: @@ -447,13 +500,18 @@ async def _perform_update(): "error": None, } - # Fetch from origin + # Fetch branches AND tags from origin so any ref the caller passes + # (release tag like `v0.2.4b1`, a branch like `main`, or a sha) is + # locally resolvable for the reset below. `--tags` is required — + # plain `git fetch origin` doesn't bring tags by default, so a + # release tag would not be resolvable. process = await asyncio.create_subprocess_exec( git_path, *git_config, "fetch", + "--prune", + "--tags", "origin", - "main", cwd=str(base_dir), stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, @@ -478,13 +536,18 @@ async def _perform_update(): "error": None, } - # Hard reset to origin/main (clean update, no merge conflicts) + # Hard reset to the target ref (clean update, no merge conflicts). + # `target_ref` is typically a release tag like `v0.2.4b1` resolved + # from the GitHub releases API by `_discover_target_release`. The + # local branch name doesn't change — only HEAD moves. Falling back + # to `origin/main` here was the source of the "in-app updater can't + # reach beta releases" bug. process = await asyncio.create_subprocess_exec( git_path, *git_config, "reset", "--hard", - "origin/main", + target_ref, cwd=str(base_dir), stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, @@ -593,6 +656,7 @@ async def _perform_update(): @router.post("/apply") async def apply_update( background_tasks: BackgroundTasks, + db: AsyncSession = Depends(get_db), _: User | None = RequirePermissionIfAuthEnabled(Permission.SETTINGS_UPDATE), ): """Apply available update (git pull + rebuild).""" @@ -617,8 +681,22 @@ async def apply_update( ), } + # Discover which release tag to install. Resolved here (where we have + # a DB session) and passed into the background task; the BG task can't + # reuse this request's session since FastAPI closes it on response. + target_ref = await _discover_target_release(db) + if target_ref is None: + return { + "success": False, + "message": ( + "Could not determine a release to install. Either GitHub is " + "unreachable or no release matches your update channel " + "(check the include_beta_updates setting)." + ), + } + # Start update in background - background_tasks.add_task(_perform_update) + background_tasks.add_task(_perform_update, target_ref) _update_status = { "status": "downloading", diff --git a/backend/tests/integration/test_updates_api.py b/backend/tests/integration/test_updates_api.py index 1e78c17b8..17d490794 100644 --- a/backend/tests/integration/test_updates_api.py +++ b/backend/tests/integration/test_updates_api.py @@ -23,9 +23,16 @@ class TestUpdatesAPI: @pytest.mark.asyncio async def test_apply_update_non_docker(self, async_client: AsyncClient): - """Test non-Docker path - mock _perform_update to prevent side effects.""" + """Test non-Docker path - mock _perform_update + _discover_target_release + to prevent side effects (network call to GitHub releases API + actual + git/pip subprocesses).""" with ( patch("backend.app.api.routes.updates._is_docker_environment", return_value=False), + patch( + "backend.app.api.routes.updates._discover_target_release", + new_callable=AsyncMock, + return_value="v9.9.9", + ), patch("backend.app.api.routes.updates._perform_update", new_callable=AsyncMock), ): response = await async_client.post("/api/v1/updates/apply") @@ -118,7 +125,7 @@ class TestUpdatesAPI: side_effect=fake_create_subprocess_exec, ), ): - await updates_module._perform_update() + await updates_module._perform_update("v0.2.4b1") # The updater MUST NOT have run `git remote set-url origin ` # because origin already pointed at the right repo over SSH. @@ -167,7 +174,7 @@ class TestUpdatesAPI: side_effect=fake_create_subprocess_exec, ), ): - await updates_module._perform_update() + await updates_module._perform_update("v0.2.4b1") set_url_calls = [c for c in calls if "set-url" in c["args"] and "origin" in c["args"]] assert set_url_calls, "Updater must rewrite origin when it points at a fork." @@ -176,6 +183,121 @@ class TestUpdatesAPI: f"Expected origin to be reset to canonical HTTPS URL; got: {rewritten_to}" ) + @pytest.mark.asyncio + async def test_perform_update_resets_to_target_ref_not_hardcoded_main(self, tmp_path): + """Regression for the hardcoded-`origin/main` limitation: the in-app + updater must reset to the caller-supplied target ref (typically a + release tag like `v0.2.4b1` discovered from the GitHub releases API) + so beta releases that don't live on main can actually be installed. + Pre-fix, `_perform_update` issued `git reset --hard origin/main` + verbatim and silently no-op'd whenever the latest release wasn't on + main — leaving a 0.2.3.x user clicking *Apply Update* stranded on + 0.2.3.x. Also asserts the fetch step uses `--tags` so a tag ref is + actually resolvable post-fetch.""" + from backend.app.api.routes import updates as updates_module + + app_dir = tmp_path / "app" + data_dir = tmp_path / "app" / "data" + app_dir.mkdir() + data_dir.mkdir() + (app_dir / "requirements.txt").write_text("fastapi\n") + + calls: list[dict] = [] + + async def fake_create_subprocess_exec(*args, **kwargs): + calls.append({"args": args, "cwd": kwargs.get("cwd")}) + proc = MagicMock() + if "get-url" in args and "origin" in args: + proc.communicate = AsyncMock(return_value=(b"git@github.com:maziggy/bambuddy.git\n", b"")) + else: + proc.communicate = AsyncMock(return_value=(b"", b"")) + proc.returncode = 0 + return proc + + with ( + patch.object(updates_module.settings, "base_dir", data_dir), + patch.object(updates_module.settings, "app_dir", app_dir), + patch.object(updates_module, "_find_executable", return_value="/usr/bin/git"), + patch.object( + updates_module.asyncio, + "create_subprocess_exec", + side_effect=fake_create_subprocess_exec, + ), + ): + await updates_module._perform_update("v0.2.4b1") + + # Reset target must be the caller-supplied ref, not "origin/main". + reset_calls = [c for c in calls if "reset" in c["args"] and "--hard" in c["args"]] + assert reset_calls, "git reset must be invoked" + reset_target = reset_calls[0]["args"][-1] + assert reset_target == "v0.2.4b1", ( + f"Expected reset target to be the caller-supplied ref 'v0.2.4b1'; " + f"got {reset_target!r}. Regression to a hardcoded 'origin/main' " + "would re-introduce the in-app-updater-can't-install-betas bug." + ) + + # Fetch must include --tags so v0.2.4b1 (a tag) is locally resolvable. + fetch_calls = [c for c in calls if "fetch" in c["args"]] + assert fetch_calls + assert "--tags" in fetch_calls[0]["args"], ( + "Fetch must use --tags so release-tag refs (the production path " + "for tag-based updates) are resolvable for the subsequent reset. " + f"Captured fetch call: {fetch_calls[0]['args']}" + ) + + @pytest.mark.asyncio + async def test_apply_update_passes_discovered_release_to_perform_update(self, async_client: AsyncClient): + """End-to-end glue: the route handler calls `_discover_target_release` + to pick the tag (respecting include_beta_updates), then schedules + `_perform_update` with that tag — not with no arg, not with main.""" + from backend.app.api.routes import updates as updates_module + + captured_ref: list[str] = [] + + async def fake_perform_update(target_ref): + captured_ref.append(target_ref) + + async def fake_discover(_db): + return "v0.2.4b1" + + with ( + patch.object(updates_module, "_is_docker_environment", return_value=False), + patch.object(updates_module, "_perform_update", side_effect=fake_perform_update), + patch.object(updates_module, "_discover_target_release", side_effect=fake_discover), + ): + response = await async_client.post("/api/v1/updates/apply") + + assert response.json()["success"] is True + assert captured_ref == ["v0.2.4b1"], ( + f"apply_update must pass the discovered tag to _perform_update; captured invocations: {captured_ref}" + ) + + @pytest.mark.asyncio + async def test_apply_update_returns_clear_error_when_no_release_resolves(self, async_client: AsyncClient): + """If GitHub is unreachable or no release matches the user's channel, + the route returns a useful error instead of silently kicking off an + update that can't possibly land. Avoids the previous failure mode + where in-app update appeared to succeed but did nothing.""" + from backend.app.api.routes import updates as updates_module + + async def fake_discover(_db): + return None + + # The route guards against a concurrent update via the module-global + # `_update_status` — reset it so a previous test that left the status + # mid-flight doesn't short-circuit this one. + updates_module._update_status = {"status": "idle", "progress": 0, "message": "", "error": None} + + with ( + patch.object(updates_module, "_is_docker_environment", return_value=False), + patch.object(updates_module, "_discover_target_release", side_effect=fake_discover), + ): + response = await async_client.post("/api/v1/updates/apply") + + body = response.json() + assert body["success"] is False + assert "release" in body["message"].lower() + @pytest.mark.asyncio async def test_perform_update_runs_pip_in_app_dir_not_data_dir(self, tmp_path): """Native install: `requirements.txt` lives at INSTALL_PATH (the source- @@ -220,7 +342,7 @@ class TestUpdatesAPI: side_effect=fake_create_subprocess_exec, ), ): - await updates_module._perform_update() + await updates_module._perform_update("v0.2.4b1") # Find the pip invocation (sys.executable + "-m" + "pip" + "install"). pip_calls = [c for c in calls if "pip" in c["args"] and "install" in c["args"]]