fix(updater): rebuild desktop on Windows hand-off repair path
The HERMES_UPDATE_REEXEC child and the current-checkout Node repair path printed success without calling _rebuild_desktop_after_update. A failed rebuild now withholds the success banner the same way the commits-pulled path does. Fixes #97343
This commit is contained in:
@@ -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()
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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("✓")
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user