diff --git a/hermes_cli/update_cmd.py b/hermes_cli/update_cmd.py index 2c42fbd989..85dc437f25 100644 --- a/hermes_cli/update_cmd.py +++ b/hermes_cli/update_cmd.py @@ -4126,6 +4126,7 @@ def _repair_node_deps_on_current_checkout( gateway_mode: bool = False, pre_update_snapshot_id: str | None = None, completion_message: str = "✓ Already up to date!", + had_desktop_app_before_update: bool = False, ) -> bool: """Repair Node deps on the ``commit_count == 0`` path (#77211). @@ -4156,6 +4157,23 @@ def _repair_node_deps_on_current_checkout( gateway_mode=gateway_mode, pre_update_snapshot_id=pre_update_snapshot_id, ) + # A current checkout can still owe a Desktop rebuild (#97343): the + # packaged app is built from source the pull already landed — or, on the + # Windows hand-off, by a child that never reaches the commits-pulled + # rebuild. Skipping it leaves a stale desktop app behind a + # successful-looking update. Self-gates on the build stamp, so this is a + # no-op when nothing changed. + if not _rebuild_desktop_after_update( + _m().PROJECT_ROOT / "apps" / "desktop", + had_desktop_app_before_update=had_desktop_app_before_update, + ): + # _rebuild_desktop_after_update already printed the retry hint; withhold + # success rather than claiming the update finished (#88251). + print_completion( + "⚠ Update partially complete — the desktop app was not rebuilt " + "and is still on the previous build." + ) + return False return bool(print_completion(completion_message)) @@ -8647,9 +8665,24 @@ def _cmd_update_impl(args, gateway_mode: bool): gateway_mode=gateway_mode, pre_update_snapshot_id=pre_update_snapshot_id, ) - current_checkout_complete = _print_verified_update_completion( - "✓ Update complete!" - ) + # The Windows hand-off child lands here after doing the + # sync its parent could not, and the commits-pulled + # rebuild below is never reached — rebuild the Desktop + # app here or it silently stays on the old build + # (#97343). + if _rebuild_desktop_after_update( + desktop_dir, + had_desktop_app_before_update=had_desktop_app_before_update, + ): + current_checkout_complete = _print_verified_update_completion( + "✓ Update complete!" + ) + else: + current_checkout_complete = False + _print_update_completion( + "⚠ Update partially complete — the desktop app was " + "not rebuilt and is still on the previous build." + ) else: current_checkout_complete = False print(f"⚠ Venv still unhealthy after repair: {detail_after}") @@ -8665,6 +8698,7 @@ def _cmd_update_impl(args, gateway_mode: bool): if upstream_checked else "✓ Up to date with your fork (official repo not checked)." ), + had_desktop_app_before_update=had_desktop_app_before_update, ) if runtime_repaired is not None and not _m()._is_windows(): print() diff --git a/tests/hermes_cli/test_update_config_migration_on_current_checkout.py b/tests/hermes_cli/test_update_config_migration_on_current_checkout.py index 43e5eb8eb5..f35d359d06 100644 --- a/tests/hermes_cli/test_update_config_migration_on_current_checkout.py +++ b/tests/hermes_cli/test_update_config_migration_on_current_checkout.py @@ -29,6 +29,7 @@ def test_repair_node_deps_runs_config_migration_on_version_bump(capsys): "_run_migrate_config_fresh", return_value={"env_added": [], "config_added": ["migrated to v38"], "warnings": []}, ) as mock_migrate, + patch.object(update_cmd, "_rebuild_desktop_after_update", return_value=True), ): update_cmd._repair_node_deps_on_current_checkout(completion) @@ -52,6 +53,7 @@ def test_repair_node_deps_up_to_date_config(capsys): patch("hermes_cli.config.get_missing_env_vars", return_value=[]), patch("hermes_cli.config.get_missing_config_fields", return_value=[]), patch.object(update_cmd, "_run_migrate_config_fresh") as mock_migrate, + patch.object(update_cmd, "_rebuild_desktop_after_update", return_value=True), ): update_cmd._repair_node_deps_on_current_checkout(completion) diff --git a/tests/hermes_cli/test_update_current_node_repair.py b/tests/hermes_cli/test_update_current_node_repair.py index 3af7ce6279..2f31cb835e 100644 --- a/tests/hermes_cli/test_update_current_node_repair.py +++ b/tests/hermes_cli/test_update_current_node_repair.py @@ -36,7 +36,9 @@ def test_current_checkout_healthy_node_deps_reports_up_to_date(): completion = MagicMock() with patch.object( update_cmd, "_update_node_dependencies", return_value=[] - ), patch.object(update_cmd, "_m") as m: + ), patch.object(update_cmd, "_m") as m, patch.object( + update_cmd, "_rebuild_desktop_after_update", return_value=True + ): update_cmd._repair_node_deps_on_current_checkout(completion) # The refresh pairs with the web build like every other call site. diff --git a/tests/hermes_cli/test_update_handoff_desktop_rebuild.py b/tests/hermes_cli/test_update_handoff_desktop_rebuild.py new file mode 100644 index 0000000000..357d12bdd1 --- /dev/null +++ b/tests/hermes_cli/test_update_handoff_desktop_rebuild.py @@ -0,0 +1,54 @@ +"""The current-checkout repair path must rebuild the Desktop app (#97343). + +A Windows git install runs `hermes update` from `hermes.exe`, which reexecs a +venv-Python child to finish the dependency sync. That child completes through +``_repair_node_deps_on_current_checkout`` / the hand-off repair branch, never +through the commits-pulled path that owns the Desktop rebuild — so a +successful-looking update left the packaged desktop app on the previous build. +""" + +from __future__ import annotations + +from unittest.mock import MagicMock, patch + +from hermes_cli import update_cmd + + +def test_current_checkout_repair_rebuilds_desktop_under_project_root(): + """The repair passes PROJECT_ROOT/apps/desktop and the pre-update flag.""" + completion = MagicMock(return_value=True) + with ( + patch.object(update_cmd, "_update_node_dependencies", return_value=[]), + patch.object(update_cmd, "_m") as m, + patch.object(update_cmd, "_check_and_apply_config_migration"), + patch.object( + update_cmd, "_rebuild_desktop_after_update", return_value=True + ) as rebuild, + ): + m.return_value.PROJECT_ROOT = update_cmd.Path("/fake/hermes") + complete = update_cmd._repair_node_deps_on_current_checkout( + completion, had_desktop_app_before_update=True + ) + + assert complete is True + rebuild.assert_called_once() + assert rebuild.call_args[0][0] == update_cmd.Path("/fake/hermes/apps/desktop") + assert rebuild.call_args[1]["had_desktop_app_before_update"] is True + completion.assert_called_once_with("✓ Already up to date!") + + +def test_failed_desktop_rebuild_withholds_success_completion(): + """A failed rebuild must not report success and must return False.""" + completion = MagicMock(return_value=True) + with ( + patch.object(update_cmd, "_update_node_dependencies", return_value=[]), + patch.object(update_cmd, "_m") as m, + patch.object(update_cmd, "_check_and_apply_config_migration"), + patch.object(update_cmd, "_rebuild_desktop_after_update", return_value=False), + ): + m.return_value.PROJECT_ROOT = update_cmd.Path("/fake/hermes") + complete = update_cmd._repair_node_deps_on_current_checkout(completion) + + assert complete is False + for call in completion.call_args_list: + assert not call[0][0].startswith("✓") diff --git a/tests/hermes_cli/test_update_sqlite_remediation.py b/tests/hermes_cli/test_update_sqlite_remediation.py index 13d6056704..e2a280759a 100644 --- a/tests/hermes_cli/test_update_sqlite_remediation.py +++ b/tests/hermes_cli/test_update_sqlite_remediation.py @@ -70,6 +70,11 @@ def test_current_checkout_completion_is_verified_before_success(capsys, monkeypa def test_current_checkout_repair_returns_verified_completion_result(monkeypatch): monkeypatch.setattr(update_cmd, "_update_node_dependencies", lambda: []) monkeypatch.setattr(update_cmd._m(), "_build_web_ui", lambda _path: None) + monkeypatch.setattr( + update_cmd, + "_rebuild_desktop_after_update", + lambda _dir, **_kwargs: True, + ) complete = update_cmd._repair_node_deps_on_current_checkout( lambda _message: False