diff --git a/hermes_cli/update_cmd.py b/hermes_cli/update_cmd.py index 0d71c63804..49c46a5fae 100644 --- a/hermes_cli/update_cmd.py +++ b/hermes_cli/update_cmd.py @@ -287,7 +287,6 @@ def _migrate_sibling_profile_configs() -> list[tuple[str, int, int]]: logger.debug("Sibling profile enumeration failed: %s", exc) return migrated - def _check_and_apply_config_migration( *, assume_yes: bool = False, @@ -316,10 +315,18 @@ def _check_and_apply_config_migration( 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() + # Defensive (#91360): this helper runs on repair/retry completion paths + # too — a config-check failure must not break an otherwise-successful + # update. Log, point at the manual command, and return. + try: + missing_env = get_missing_env_vars(required_only=True) + missing_config = get_missing_config_fields() + current_ver, latest_ver = _run_config_check_fresh() + except Exception as exc: + logger.debug("Config check during update failed: %s", exc) + print(" ⚠️ Could not check config version.") + print(" Run 'hermes config migrate' to check manually.") + return has_new_options = bool(missing_env or missing_config) version_bump_only = ( diff --git a/scripts/desktop-update/posix.sh b/scripts/desktop-update/posix.sh index e4e7889fbc..ad13b85984 100755 --- a/scripts/desktop-update/posix.sh +++ b/scripts/desktop-update/posix.sh @@ -594,6 +594,19 @@ log "hermes update exit code: $CODE" if [ "$CODE" -ne 0 ] && [ "$CODE" -ne 2 ]; then # Retry once: update-boundary class (fresh code on disk, stale in memory). # Exit 2 ("close all Hermes windows") is not retryable. + # + # A parked-branch SKIP (checkout on a feature branch with unmerged + # commits) is also deterministic — the retry would hit the exact same + # branch state and skip again, so it only wastes time. Detect the skip + # by its banner, skip the retry, and surface an honest message with a + # dedicated exit code (8) so callers can distinguish "skipped" from a + # real failure. + if printf '%s' "$OUT" | grep -q "CODE UPDATE SKIPPED"; then + log "hermes update skipped (checkout parked on a non-target branch); not retrying" + FINAL_CODE=8 + FINAL_MSG="Update skipped: the git checkout is on a branch that isn't fully merged into $BRANCH. Switch to the target branch and update again (see the terminal output for the exact commands)." + exit 8 + fi log "retrying once (freshly pulled fix loads on the second run)" publish_stage "Retrying update" OUT="$("$HERMES_BIN" update --yes --gateway $KEEP_STASH --branch "$BRANCH" 2>&1)"; CODE=$? diff --git a/tests/hermes_cli/test_update_config_migration_on_current.py b/tests/hermes_cli/test_update_config_migration_on_current.py new file mode 100644 index 0000000000..907818a3dd --- /dev/null +++ b/tests/hermes_cli/test_update_config_migration_on_current.py @@ -0,0 +1,122 @@ +"""Tests for config migration on the \"Already up to date\" repair path. + +Covers ``_check_and_apply_config_migration`` on the ``commit_count == 0`` +retry paths (#91360): a previous update attempt can pull new code onto disk +and then fail before reaching the config-migration block (e.g. PyPI timeout +during dependency sync); the retry then enters the ``commit_count == 0`` +branch and returns early, skipping config migration entirely. The fresh +code (which may require a newer ``_config_version``) keeps running against +the old config and the next Hermes launch refuses to start. + +Matrix: config behind / current / ahead of the code's version, plus the +#86656 contract (quiet-migration warnings must be re-surfaced) and the +check-failure contract (a config-check failure must not break the repair +path). +""" + +from __future__ import annotations + +import contextlib +import io +from unittest.mock import patch + +import hermes_cli.update_cmd as update_cmd + + +def _run(current: int, latest: int): + """Run _check_and_apply_config_migration with mocked config checks. + + Returns (stdout, migrate_calls). + """ + migrate_calls = [] + + def _fake_migrate(interactive=False, quiet=False): + migrate_calls.append((interactive, quiet)) + return {"env_added": [], "config_added": [], "warnings": []} + + with patch.object(update_cmd, "_reload_config_modules"), 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_config_check_fresh", return_value=(current, latest) + ), patch.object( + update_cmd, "_run_migrate_config_fresh", side_effect=_fake_migrate + ), patch.object( + update_cmd, "_migrate_sibling_profile_configs", return_value=[] + ): + buf = io.StringIO() + with contextlib.redirect_stdout(buf): + update_cmd._check_and_apply_config_migration() + return buf.getvalue(), migrate_calls + + +def test_migrates_when_config_behind(): + """Version bump on the repair path must be applied silently.""" + out, calls = _run(current=37, latest=38) + assert "v37 → v38" in out + assert "Config format updated" in out + assert calls == [(False, True)] # non-interactive, quiet + + +def test_noop_when_config_current(): + """No migration when the config version is already current.""" + out, calls = _run(current=38, latest=38) + assert "Configuration is up to date" in out + assert calls == [] + + +def test_noop_when_config_ahead(): + """No migration when local config is newer than the code's default.""" + out, calls = _run(current=39, latest=38) + assert "Configuration is up to date" in out + assert calls == [] + + +def test_surfaces_migration_warnings(): + """Warnings from a quiet migration must be re-surfaced (#86656).""" + + def _fake_migrate(interactive=False, quiet=False): + return { + "env_added": [], + "config_added": [], + "warnings": ["personality reset: kawaii → default"], + } + + with patch.object(update_cmd, "_reload_config_modules"), 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_config_check_fresh", return_value=(37, 38) + ), patch.object( + update_cmd, "_run_migrate_config_fresh", side_effect=_fake_migrate + ), patch.object( + update_cmd, "_migrate_sibling_profile_configs", return_value=[] + ): + buf = io.StringIO() + with contextlib.redirect_stdout(buf): + update_cmd._check_and_apply_config_migration() + out = buf.getvalue() + + assert "personality reset" in out + + +def test_check_failure_does_not_break_repair_path(): + """A config-check failure must not break the repair path.""" + with patch.object(update_cmd, "_reload_config_modules"), 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_config_check_fresh", side_effect=RuntimeError("boom") + ), patch.object( + update_cmd, "_run_migrate_config_fresh", return_value={} + ) as mig: + buf = io.StringIO() + with contextlib.redirect_stdout(buf): + # Should not raise and should not attempt migration. + update_cmd._check_and_apply_config_migration() + out = buf.getvalue() + assert "Could not check config version" in out + mig.assert_not_called()