fix(update): check and apply config migrations on current checkout / retry paths (#91360)

When an update was interrupted or failed mid-install (e.g. dependency install
timeout) after pulling new code, the subsequent update run takes the
'commit_count == 0' path and early-returned without checking or migrating
the configuration. Fresh code requiring a newer config version would fail to
boot on the next run.

Extract _check_and_apply_config_migration and invoke it across all update
completion paths (normal update, current checkout / node repair, and python
dependency repair).
This commit is contained in:
loulanyue
2026-08-21 15:17:05 +08:00
committed by Teknium
parent 65335549a6
commit bf5ff51078
2 changed files with 372 additions and 219 deletions
+250 -219
View File
@@ -288,6 +288,228 @@ def _migrate_sibling_profile_configs() -> list[tuple[str, int, int]]:
return migrated
def _check_and_apply_config_migration(
*,
assume_yes: bool = False,
gateway_mode: bool = False,
pre_update_snapshot_id: str | None = None,
) -> None:
"""Check and apply configuration migrations on an update completion path (#91360).
CRITICAL: ``check_config_version`` and ``migrate_config`` must use
freshly-reloaded modules, not the ``sys.modules`` cache (see
``_reload_config_modules``). This must run on EVERY update completion
path — the normal post-pull path, the venv-repair retry and the
Node-deps repair on the ``commit_count == 0`` "Already up to date"
branch — so an interrupted update that previously pulled new code does
not strand the user on an older config version.
"""
print()
print("→ Checking configuration for new options...")
# Reload config modules BEFORE any config reads so get_missing_*,
# check_config_version, and migrate_config all use the updated code.
_reload_config_modules()
from hermes_cli.config import (
get_missing_env_vars,
get_missing_config_fields,
)
missing_env = get_missing_env_vars(required_only=True)
missing_config = get_missing_config_fields()
current_ver, latest_ver = _run_config_check_fresh()
has_new_options = bool(missing_env or missing_config)
version_bump_only = (
not has_new_options and current_ver < latest_ver
)
needs_migration = has_new_options or current_ver < latest_ver
if version_bump_only:
# Nothing for the user to fill in — only the config format version
# changed (new defaults already merge in transparently). Asking
# "configure new options now?" here is misleading: saying yes just
# bumps the version and looks like a no-op (issue: ScottFive /
# Tt2021). Apply it silently and say what actually happened.
print()
print(
f" ℹ Updating config format (v{current_ver} → v{latest_ver})…"
)
try:
_mig_results = _run_migrate_config_fresh(
interactive=False, quiet=True
)
print(" ✓ Config format updated (no new settings to configure)")
# quiet=True also mutes migration steps that RESET or REMOVE an
# existing setting (e.g. the v33→v34 personality reset from
# #81946, which records its note only in the results dict).
# Re-surface those notes so an unattended update never silently
# changes user configuration (#86656). In this branch
# missing_config is empty, so config_added can only contain
# migration-step mutations, not missing-key listings.
for _note in _mig_results.get("config_added") or []:
print(f" ℹ {_note}")
for _warn in _mig_results.get("warnings") or []:
print(f" ⚠️ {_warn}")
except Exception as _mig_err:
print(f" ⚠️ Config format update failed: {_mig_err}")
print(" Run 'hermes config migrate' to retry.")
elif needs_migration:
print()
# Show WHAT changed, not just a count, so the user can make an
# informed yes/no decision (previously the prompt named nothing).
def _print_items(items, label, key, fallback_key=None):
if not items:
return
print(f" {label}:")
shown = items[:8]
for it in shown:
if isinstance(it, dict):
name = it.get(key) or (fallback_key and it.get(fallback_key)) or "?"
desc = (it.get("description") or "").strip()
else:
# Defensive: some callers/mocks pass bare name strings.
name = str(it)
desc = ""
if desc:
print(f" • {name} — {desc}")
else:
print(f" • {name}")
extra = len(items) - len(shown)
if extra > 0:
print(f" … and {extra} more")
if missing_env:
print(
f" ⚠️ {len(missing_env)} new required setting(s) need configuration"
)
_print_items(missing_env, "New settings", "name")
if missing_config:
print(f" ℹ️ {len(missing_config)} new config option(s) available")
_print_items(missing_config, "New options", "key")
print()
if assume_yes:
print(
" ℹ --yes: auto-applying config migration (skipping API-key prompts)."
)
response = "y"
elif gateway_mode:
response = (
_gateway_prompt(
"Would you like to configure new options now? [Y/n]", "n"
)
.strip()
.lower()
)
elif not (sys.stdin.isatty() and sys.stdout.isatty()):
print(" ℹ Non-interactive session — applying safe config migrations.")
response = "auto"
else:
try:
response = (
input("Would you like to configure them now? [Y/n]: ")
.strip()
.lower()
)
except EOFError:
response = "n"
except UnicodeDecodeError:
# input() can raise this when the terminal encoding can't
# decode the byte sequence (e.g. a non-UTF-8 locale, or an
# embedded terminal). Without this, the exception escapes
# here and crashes the update at this prompt.
print(
" ⚠ Could not read input (encoding issue). Skipping. "
"Run 'hermes config migrate' manually to configure."
)
response = "n"
if response in {"", "y", "yes", "auto"}:
print()
# Gateway mode, --yes, and non-interactive update contexts
# (dashboard / web server actions) cannot prompt for API keys.
# Still run the non-interactive migration pass before restarting
# so new default config fields and version bumps are written
# before the freshly updated gateway validates config at startup.
interactive_migration = not (
gateway_mode or assume_yes or response == "auto"
)
results = _run_migrate_config_fresh(interactive=interactive_migration, quiet=False)
if results["env_added"] or results["config_added"]:
print()
print("✓ Configuration updated!")
if (gateway_mode or assume_yes or response == "auto") and missing_env:
print(" ℹ API keys require manual entry: hermes config migrate")
else:
print()
print("Skipped. Run 'hermes config migrate' later to configure.")
else:
print(" ✓ Configuration is up to date")
# Fleet-wide config migration (#91277 Phase 2; #20438 earliest report,
# #54926, #79048): the shared checkout serves EVERY profile, but the
# migration above only touched the active profile's config.yaml.
# Sibling profiles kept their old _config_version and silently
# drifted (field repro: sibling gateway restarted onto new code but
# stayed at config v33 vs v37). Run the same NON-INTERACTIVE safe
# migration for every sibling profile home, scoped via the
# context-local HERMES_HOME override (never os.environ — other
# threads must not see it).
try:
_migrated_siblings = _migrate_sibling_profile_configs()
for _name, _from_ver, _to_ver in _migrated_siblings:
print(
f" ✓ Profile '{_name}': config format updated "
f"(v{_from_ver} → v{_to_ver})"
)
except Exception as exc:
logger.debug("Sibling config migration failed: %s", exc)
# Safety net: config-version migrations have been observed to leave
# cron/jobs.json valid-but-empty, silently dropping every scheduled
# job (issue #34600). The desktop scheduler can also overwrite with
# its own small set, causing partial loss (issue #52144). If the
# live file now has fewer jobs than the pre-update snapshot, restore
# it and warn loudly.
try:
from hermes_cli.backup import restore_cron_jobs_if_emptied
cron_restore = restore_cron_jobs_if_emptied(pre_update_snapshot_id)
if cron_restore:
print()
print(
" ⚠️ cron/jobs.json lost jobs during this update — "
f"restored {cron_restore['job_count']} job(s) from "
f"pre-update snapshot {cron_restore['snapshot_id']}."
)
except Exception as exc:
# Never let the cron safety net break an otherwise-good update.
logger.debug("Cron jobs auto-restore check failed: %s", exc)
# #66140: run the same cron-jobs safety net for every sibling
# profile against ITS OWN pre-update snapshot (same-generation by
# construction — both taken by this run).
try:
from hermes_cli.backup import restore_cron_jobs_all_profiles
for _restored in restore_cron_jobs_all_profiles(
_LAST_SIBLING_SNAPSHOTS
):
print()
print(
f" ⚠️ Profile '{_restored['profile']}': cron/jobs.json "
f"lost jobs during this update — restored "
f"{_restored['job_count']} job(s) from pre-update "
f"snapshot {_restored['snapshot_id']}."
)
except Exception as exc:
logger.debug("Sibling cron auto-restore check failed: %s", exc)
# Critical files that Hermes must be able to import immediately after an
# update/install. Most are imported on every CLI startup; ``web_server.py``
# is the desktop/dashboard backend path that a fresh Windows install launches
@@ -3133,7 +3355,13 @@ def _record_npm_lockfile_hash(hermes_root: Path) -> None:
except OSError:
logger.debug("Could not write npm lockfile hash cache")
def _repair_node_deps_on_current_checkout(print_completion) -> None:
def _repair_node_deps_on_current_checkout(
print_completion,
*,
assume_yes: bool = False,
gateway_mode: bool = False,
pre_update_snapshot_id: str | None = None,
) -> None:
"""Repair Node deps on the ``commit_count == 0`` path (#77211).
A current checkout does not imply healthy Node deps: a previous npm
@@ -3158,6 +3386,11 @@ def _repair_node_deps_on_current_checkout(print_completion) -> None:
# _update_node_dependencies call site; it staleness-checks internally,
# so this is a no-op when nothing changed.
_m()._build_web_ui(_m().PROJECT_ROOT / "web")
_check_and_apply_config_migration(
assume_yes=assume_yes,
gateway_mode=gateway_mode,
pre_update_snapshot_id=pre_update_snapshot_id,
)
print_completion("✓ Already up to date!")
@@ -7397,12 +7630,22 @@ def _cmd_update_impl(args, gateway_mode: bool):
healthy_after, detail_after = _venv_core_imports_healthy()
if healthy_after:
print("✓ Dependencies repaired!")
_check_and_apply_config_migration(
assume_yes=assume_yes,
gateway_mode=gateway_mode,
pre_update_snapshot_id=pre_update_snapshot_id,
)
_print_update_completion("✓ Update complete!")
else:
print(f"⚠ Venv still unhealthy after repair: {detail_after}")
print(" Close all Hermes windows/gateways and re-run: hermes update")
else:
_repair_node_deps_on_current_checkout(_print_update_completion)
_repair_node_deps_on_current_checkout(
_print_update_completion,
assume_yes=assume_yes,
gateway_mode=gateway_mode,
pre_update_snapshot_id=pre_update_snapshot_id,
)
if runtime_repaired is not None and not _m()._is_windows():
print()
print(
@@ -8053,225 +8296,13 @@ def _cmd_update_impl(args, gateway_mode: bool):
except Exception:
pass # honcho plugin not installed or not configured
# Check for config migrations.
#
# CRITICAL: check_config_version and migrate_config must use
# freshly-reloaded modules, not the sys.modules cache. The
# ``hermes update`` process is the PRE-pull Python process — its
# ``sys.modules`` cache holds the OLD ``hermes_cli.config`` and
# ``hermes_cli.config_migrations`` from before ``git pull`` updated
# the source files. A function-level ``from hermes_cli.config import
# check_config_version`` returns the cached module, so
# ``DEFAULT_CONFIG["_config_version"]`` is the OLD value and
# ``check_config_version()`` reports ``(33, 33)`` — "up to date" —
# even though the freshly-pulled code has v34 with a migration to
# run. The personality reset migration (#81946) was silently skipped
# this way, leaving ``display.personality: kawaii`` active after
# updates that should have reset it.
print()
print("→ Checking configuration for new options...")
# Reload config modules BEFORE any config reads so get_missing_*,
# check_config_version, and migrate_config all use the updated code.
_reload_config_modules()
from hermes_cli.config import (
get_missing_env_vars,
get_missing_config_fields,
# Check for config migrations (#91360).
_check_and_apply_config_migration(
assume_yes=assume_yes,
gateway_mode=gateway_mode,
pre_update_snapshot_id=pre_update_snapshot_id,
)
missing_env = get_missing_env_vars(required_only=True)
missing_config = get_missing_config_fields()
current_ver, latest_ver = _run_config_check_fresh()
has_new_options = bool(missing_env or missing_config)
version_bump_only = (
not has_new_options and current_ver < latest_ver
)
needs_migration = has_new_options or current_ver < latest_ver
if version_bump_only:
# Nothing for the user to fill in — only the config format version
# changed (new defaults already merge in transparently). Asking
# "configure new options now?" here is misleading: saying yes just
# bumps the version and looks like a no-op (issue: ScottFive /
# Tt2021). Apply it silently and say what actually happened.
print()
print(
f" ℹ Updating config format (v{current_ver} → v{latest_ver})…"
)
try:
_mig_results = _run_migrate_config_fresh(
interactive=False, quiet=True
)
print(" ✓ Config format updated (no new settings to configure)")
# quiet=True also mutes migration steps that RESET or REMOVE an
# existing setting (e.g. the v33→v34 personality reset from
# #81946, which records its note only in the results dict).
# Re-surface those notes so an unattended update never silently
# changes user configuration (#86656). In this branch
# missing_config is empty, so config_added can only contain
# migration-step mutations, not missing-key listings.
for _note in _mig_results.get("config_added") or []:
print(f" ℹ {_note}")
for _warn in _mig_results.get("warnings") or []:
print(f" ⚠️ {_warn}")
except Exception as _mig_err:
print(f" ⚠️ Config format update failed: {_mig_err}")
print(" Run 'hermes config migrate' to retry.")
elif needs_migration:
print()
# Show WHAT changed, not just a count, so the user can make an
# informed yes/no decision (previously the prompt named nothing).
def _print_items(items, label, key, fallback_key=None):
if not items:
return
print(f" {label}:")
shown = items[:8]
for it in shown:
if isinstance(it, dict):
name = it.get(key) or (fallback_key and it.get(fallback_key)) or "?"
desc = (it.get("description") or "").strip()
else:
# Defensive: some callers/mocks pass bare name strings.
name = str(it)
desc = ""
if desc:
print(f" • {name} — {desc}")
else:
print(f" • {name}")
extra = len(items) - len(shown)
if extra > 0:
print(f" … and {extra} more")
if missing_env:
print(
f" ⚠️ {len(missing_env)} new required setting(s) need configuration"
)
_print_items(missing_env, "New settings", "name")
if missing_config:
print(f" ℹ️ {len(missing_config)} new config option(s) available")
_print_items(missing_config, "New options", "key")
print()
if assume_yes:
print(
" ℹ --yes: auto-applying config migration (skipping API-key prompts)."
)
response = "y"
elif gateway_mode:
response = (
_gateway_prompt(
"Would you like to configure new options now? [Y/n]", "n"
)
.strip()
.lower()
)
elif not (sys.stdin.isatty() and sys.stdout.isatty()):
print(" ℹ Non-interactive session — applying safe config migrations.")
response = "auto"
else:
try:
response = (
input("Would you like to configure them now? [Y/n]: ")
.strip()
.lower()
)
except EOFError:
response = "n"
except UnicodeDecodeError:
# input() can raise this when the terminal encoding can't
# decode the byte sequence (e.g. a non-UTF-8 locale, or an
# embedded terminal). Without this, the exception escapes
# here and crashes the update at this prompt.
print(
" ⚠ Could not read input (encoding issue). Skipping. "
"Run 'hermes config migrate' manually to configure."
)
response = "n"
if response in {"", "y", "yes", "auto"}:
print()
# Gateway mode, --yes, and non-interactive update contexts
# (dashboard / web server actions) cannot prompt for API keys.
# Still run the non-interactive migration pass before restarting
# so new default config fields and version bumps are written
# before the freshly updated gateway validates config at startup.
interactive_migration = not (
gateway_mode or assume_yes or response == "auto"
)
results = _run_migrate_config_fresh(interactive=interactive_migration, quiet=False)
if results["env_added"] or results["config_added"]:
print()
print("✓ Configuration updated!")
if (gateway_mode or assume_yes or response == "auto") and missing_env:
print(" ℹ API keys require manual entry: hermes config migrate")
else:
print()
print("Skipped. Run 'hermes config migrate' later to configure.")
else:
print(" ✓ Configuration is up to date")
# Fleet-wide config migration (#91277 Phase 2; #20438 earliest report,
# #54926, #79048): the shared checkout serves EVERY profile, but the
# migration above only touched the active profile's config.yaml.
# Sibling profiles kept their old _config_version and silently
# drifted (field repro: sibling gateway restarted onto new code but
# stayed at config v33 vs v37). Run the same NON-INTERACTIVE safe
# migration for every sibling profile home, scoped via the
# context-local HERMES_HOME override (never os.environ — other
# threads must not see it).
try:
_migrated_siblings = _migrate_sibling_profile_configs()
for _name, _from_ver, _to_ver in _migrated_siblings:
print(
f" ✓ Profile '{_name}': config format updated "
f"(v{_from_ver} → v{_to_ver})"
)
except Exception as exc:
logger.debug("Sibling config migration failed: %s", exc)
# Safety net: config-version migrations have been observed to leave
# cron/jobs.json valid-but-empty, silently dropping every scheduled
# job (issue #34600). The desktop scheduler can also overwrite with
# its own small set, causing partial loss (issue #52144). If the
# live file now has fewer jobs than the pre-update snapshot, restore
# it and warn loudly.
try:
from hermes_cli.backup import restore_cron_jobs_if_emptied
cron_restore = restore_cron_jobs_if_emptied(pre_update_snapshot_id)
if cron_restore:
print()
print(
" ⚠️ cron/jobs.json lost jobs during this update — "
f"restored {cron_restore['job_count']} job(s) from "
f"pre-update snapshot {cron_restore['snapshot_id']}."
)
except Exception as exc:
# Never let the cron safety net break an otherwise-good update.
logger.debug("Cron jobs auto-restore check failed: %s", exc)
# #66140: run the same cron-jobs safety net for every sibling
# profile against ITS OWN pre-update snapshot (same-generation by
# construction — both taken by this run).
try:
from hermes_cli.backup import restore_cron_jobs_all_profiles
for _restored in restore_cron_jobs_all_profiles(
_LAST_SIBLING_SNAPSHOTS
):
print()
print(
f" ⚠️ Profile '{_restored['profile']}': cron/jobs.json "
f"lost jobs during this update — restored "
f"{_restored['job_count']} job(s) from pre-update "
f"snapshot {_restored['snapshot_id']}."
)
except Exception as exc:
logger.debug("Sibling cron auto-restore check failed: %s", exc)
_print_update_summary(
node_failures=node_failures,
desktop_build_ok=desktop_build_ok,
@@ -0,0 +1,122 @@
"""Tests for config migration on the commit_count == 0 / retry path (#91360).
When an update is interrupted (or dependency install fails) after code has already
been pulled, the subsequent update run takes the 'commit_count == 0' path.
It must run config version checking and migrations before completing so the
install is not left in a non-bootable state with new code on old config version.
"""
from __future__ import annotations
from unittest.mock import MagicMock, patch
from hermes_cli import update_cmd
def test_repair_node_deps_runs_config_migration_on_version_bump(capsys):
"""When on-disk config version is behind, _repair_node_deps_on_current_checkout
must run _check_and_apply_config_migration and migrate the config."""
completion = MagicMock()
with (
patch.object(update_cmd, "_update_node_dependencies", return_value=[]),
patch.object(update_cmd, "_m") as m,
patch.object(update_cmd, "_reload_config_modules"),
patch.object(update_cmd, "_run_config_check_fresh", return_value=(37, 38)),
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",
return_value={"env_added": [], "config_added": ["migrated to v38"], "warnings": []},
) as mock_migrate,
):
update_cmd._repair_node_deps_on_current_checkout(completion)
m.return_value._build_web_ui.assert_called_once()
mock_migrate.assert_called_once_with(interactive=False, quiet=True)
completion.assert_called_once_with("✓ Already up to date!")
out = capsys.readouterr().out
assert "Checking configuration for new options..." in out
assert "Updating config format (v37 → v38)…" in out
assert "Config format updated" in out
def test_repair_node_deps_up_to_date_config(capsys):
"""When config is already up to date, it reports up to date without error."""
completion = MagicMock()
with (
patch.object(update_cmd, "_update_node_dependencies", return_value=[]),
patch.object(update_cmd, "_m") as m,
patch.object(update_cmd, "_reload_config_modules"),
patch.object(update_cmd, "_run_config_check_fresh", return_value=(38, 38)),
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,
):
update_cmd._repair_node_deps_on_current_checkout(completion)
m.return_value._build_web_ui.assert_called_once()
mock_migrate.assert_not_called()
completion.assert_called_once_with("✓ Already up to date!")
out = capsys.readouterr().out
assert "Checking configuration for new options..." in out
assert "Configuration is up to date" in out
def test_check_and_apply_config_migration_interactive_prompt():
"""When new config options exist in an interactive session, it prompts the user."""
with (
patch.object(update_cmd, "_reload_config_modules"),
patch.object(update_cmd, "_run_config_check_fresh", return_value=(37, 38)),
patch("hermes_cli.config.get_missing_env_vars", return_value=[{"name": "NEW_KEY", "description": "desc"}]),
patch("hermes_cli.config.get_missing_config_fields", return_value=[]),
patch("sys.stdin.isatty", return_value=True),
patch("sys.stdout.isatty", return_value=True),
patch("builtins.input", return_value="y"),
patch.object(
update_cmd,
"_run_migrate_config_fresh",
return_value={"env_added": ["NEW_KEY"], "config_added": [], "warnings": []},
) as mock_migrate,
):
update_cmd._check_and_apply_config_migration(assume_yes=False, gateway_mode=False)
mock_migrate.assert_called_once_with(interactive=True, quiet=False)
def test_check_and_apply_config_migration_assume_yes():
"""When assume_yes=True, it applies migrations non-interactively without prompting."""
with (
patch.object(update_cmd, "_reload_config_modules"),
patch.object(update_cmd, "_run_config_check_fresh", return_value=(37, 38)),
patch("hermes_cli.config.get_missing_env_vars", return_value=[{"name": "NEW_KEY"}]),
patch("hermes_cli.config.get_missing_config_fields", return_value=[]),
patch.object(
update_cmd,
"_run_migrate_config_fresh",
return_value={"env_added": [], "config_added": ["opt"], "warnings": []},
) as mock_migrate,
):
update_cmd._check_and_apply_config_migration(assume_yes=True, gateway_mode=False)
mock_migrate.assert_called_once_with(interactive=False, quiet=False)
def test_check_and_apply_config_migration_non_interactive():
"""In a non-interactive session (e.g. CI/scripts), it applies safe migrations automatically."""
with (
patch.object(update_cmd, "_reload_config_modules"),
patch.object(update_cmd, "_run_config_check_fresh", return_value=(37, 38)),
patch("hermes_cli.config.get_missing_env_vars", return_value=[]),
patch("hermes_cli.config.get_missing_config_fields", return_value=[{"key": "new_setting"}]),
patch("sys.stdin.isatty", return_value=False),
patch("sys.stdout.isatty", return_value=False),
patch.object(
update_cmd,
"_run_migrate_config_fresh",
return_value={"env_added": [], "config_added": ["new_setting"], "warnings": []},
) as mock_migrate,
):
update_cmd._check_and_apply_config_migration(assume_yes=False, gateway_mode=False)
mock_migrate.assert_called_once_with(interactive=False, quiet=False)