diff --git a/hermes_cli/update_cmd.py b/hermes_cli/update_cmd.py index 7618c36be8..505cafaace 100644 --- a/hermes_cli/update_cmd.py +++ b/hermes_cli/update_cmd.py @@ -832,6 +832,12 @@ def _rollback_if_pulled_syntax_error(git_cmd, pre_pull_sha, *, rollback_branch=N target = rollback_branch if rollback_branch not in (None, "HEAD") else pre_pull_sha[:10] print(f"→ Rolling back to {target}...") rollback_result = _git_run(git_cmd, rollback_args) + if rollback_result.returncode != 0 and rollback_branch not in (None, "HEAD"): + # The parked branch can be unavailable (e.g. checked out in another worktree): restore + # its commit detached so the install still boots the code it ran before. + print(f" ✗ Could not check out {rollback_branch}; restoring its commit detached.") + rollback_args = ["checkout", "--detach", pre_pull_sha] + rollback_result = _git_run(git_cmd, rollback_args) if rollback_result.returncode == 0: print(" ✓ Rollback complete — your install is unchanged.") print(" Try ``hermes update`` again later once a fix lands.") @@ -855,9 +861,9 @@ def _update_movement_baseline(git_cmd, pre_pull_sha, pre_sync_sha, rollback_bran # the stale/diverged branch tip, not the original detached checkout. contains_target = _git_run( git_cmd, ["merge-base", "--is-ancestor", target_sha, pre_pull_sha]) - # rc 128 (a SHA git cannot resolve) falls through to the pre-switch baseline: the worst - # case is a loud "did not move" refusal, never a silent success. - if contains_target.returncode == 1: + # Anything but "contained" (rc 1, or rc 128 for a SHA git cannot resolve) keeps the strict + # baseline: the worst case is a loud "did not move" refusal, never a silent success. + if contains_target.returncode != 0: return pre_pull_sha return pre_sync_sha or pre_pull_sha @@ -868,7 +874,7 @@ def _pull_updates( in_place_update=False, _windows_gateway_resume=None): """Fast-forward onto ``origin/`` and settle the autostash. Divergence by shape: custom branch -> merge, same branch -> rescue ref then reset; a - post-pull syntax error in a critical file rolls back. Exits on failure; returns pre-pull SHA.""" + post-pull syntax error in a critical file rolls back. Exits on failure; returns the SHA HEAD had to move off.""" update_succeeded = False # Rescue refs must retain the immediate pre-pull tip, even when syntax # rollback needs to cross an earlier upstream sync. @@ -934,7 +940,7 @@ def _pull_updates( _m()._restore_stashed_changes( git_cmd, _m().PROJECT_ROOT, auto_stash_ref, prompt_user=prompt_for_restore, input_fn=gw_input_fn) - return pre_pull_sha + return movement_baseline @dataclass @@ -1104,7 +1110,7 @@ def _prepare_checkout_for_update( parked_branch_switched=parked_branch_switched, prompt_for_restore=prompt_for_restore, switch_block_reason=switch_block_reason, upstream_checked=upstream_checked, pre_sync_sha=moved_from_sha, rollback_branch=rollback_branch, - switched_without_new_commits=switched_without_new_commits and commit_count == -1) + switched_without_new_commits=switched_without_new_commits) @dataclass @@ -1343,13 +1349,10 @@ def _finish_already_up_to_date( def _apply_pulled_update( - git_cmd, branch, pre_pull_sha, _plan, *, _windows_gateway_resume, completion_request: dict) -> None: - """Post-pull phase: verify HEAD, sync Python/Node/web/Desktop, maintenance, fleet restart.""" - movement_baseline = _plan.pre_sync_sha or pre_pull_sha - if _plan.rollback_branch is not None: - target_sha = (_git_run(git_cmd, ["rev-parse", f"origin/{branch}^{{commit}}"]).stdout or "").strip() - movement_baseline = _update_movement_baseline( - git_cmd, pre_pull_sha, _plan.pre_sync_sha, _plan.rollback_branch, target_sha) + git_cmd, branch, movement_baseline, _plan, *, _windows_gateway_resume, completion_request: dict) -> None: + """Post-pull phase: verify HEAD, sync Python/Node/web/Desktop, maintenance, fleet restart. + + ``movement_baseline`` is what ``_pull_updates`` returned: the SHA HEAD had to move off.""" post_pull_sha = _verify_head_after_pull( git_cmd, branch, movement_baseline, in_place_update=_plan.in_place_update, _windows_gateway_resume=_windows_gateway_resume) @@ -1526,7 +1529,7 @@ def _cmd_update_impl(args, gateway_mode: bool): print("→ Updates available (commit count unknown on this shallow checkout)") print("→ Pulling updates...") - pre_pull_sha = _pull_updates( + movement_baseline = _pull_updates( git_cmd, branch, _plan.auto_stash_ref, prompt_for_restore=_plan.prompt_for_restore, gw_input_fn=gw_input_fn, discard_local_changes=opts.discard_local_changes, keep_stash=opts.keep_stash, target_ref=target_ref, pre_sync_sha=_plan.pre_sync_sha, @@ -1534,7 +1537,7 @@ def _cmd_update_impl(args, gateway_mode: bool): sync_upstream=is_fork and branch == "main" and not release_sha, assume_yes=assume_yes, in_place_update=_plan.in_place_update, _windows_gateway_resume=_windows_gateway_resume) _apply_pulled_update( - git_cmd, branch, pre_pull_sha, _plan, + git_cmd, branch, movement_baseline, _plan, _windows_gateway_resume=_windows_gateway_resume, completion_request=completion_request) except subprocess.CalledProcessError as e: try: diff --git a/tests/hermes_cli/test_update_existing_branch_baseline.py b/tests/hermes_cli/test_update_existing_branch_baseline.py index 22bf58f35b..6a245560dc 100644 --- a/tests/hermes_cli/test_update_existing_branch_baseline.py +++ b/tests/hermes_cli/test_update_existing_branch_baseline.py @@ -1,25 +1,25 @@ """Real Git regression coverage for updates that land via an existing branch.""" import subprocess +from pathlib import Path import pytest from hermes_cli import update_cmd -from tests.hermes_cli.test_update_target_identity import update_tree # noqa: F401 +from tests.hermes_cli.test_update_target_identity import git, update_tree # noqa: F401 -def git(root, *args): - result = subprocess.run(["git", *args], cwd=root, capture_output=True, text=True) - assert result.returncode == 0, result.stderr - return result.stdout.strip() +def init_repo(root, monkeypatch): + root.mkdir() + git(root, "init", "-q", "-b", "main") + git(root, "config", "user.name", "Fixture") + git(root, "config", "user.email", "fixture@example.com") + monkeypatch.setattr(update_cmd._m(), "PROJECT_ROOT", root) @pytest.fixture def checkout(tmp_path, monkeypatch): root = tmp_path / "checkout" - root.mkdir() - git(root, "init", "-q", "-b", "main") - git(root, "config", "user.name", "Fixture") - git(root, "config", "user.email", "fixture@example.com") + init_repo(root, monkeypatch) (root / "cli.py").write_text("value = 1\n", encoding="utf8") git(root, "add", ".") git(root, "commit", "-qm", "old") @@ -29,7 +29,6 @@ def checkout(tmp_path, monkeypatch): tip = git(root, "rev-parse", "HEAD") git(root, "update-ref", "refs/remotes/origin/main", tip) git(root, "checkout", "-q", "--detach", old) - monkeypatch.setattr(update_cmd._m(), "PROJECT_ROOT", root) return root, old, tip @@ -41,18 +40,21 @@ def prepare(root, *, is_fork=False): ) +def pull(plan, **kwargs): + return update_cmd._pull_updates( + ["git"], "main", plan.auto_stash_ref, prompt_for_restore=False, + gw_input_fn=None, discard_local_changes=False, keep_stash=False, + pre_sync_sha=plan.pre_sync_sha, rollback_branch=plan.rollback_branch, **kwargs, + ) + + def test_existing_main_counts_from_running_detached_code(checkout): root, old, tip = checkout plan = prepare(root) assert git(root, "rev-parse", "HEAD") == tip assert plan.commit_count == 1 assert plan.pre_sync_sha == old - update_cmd._pull_updates( - ["git"], "main", plan.auto_stash_ref, prompt_for_restore=False, - gw_input_fn=None, discard_local_changes=False, keep_stash=False, - pre_sync_sha=plan.pre_sync_sha, - rollback_branch=plan.rollback_branch, - ) + pull(plan) assert git(root, "rev-parse", "HEAD") == tip @@ -80,11 +82,7 @@ def test_syntax_failure_returns_to_original_checkout_without_rewriting_main(chec git(root, "checkout", "-q", "feature" if parked else old) plan = prepare(root) with pytest.raises(SystemExit) as exc: - update_cmd._pull_updates( - ["git"], "main", plan.auto_stash_ref, prompt_for_restore=False, - gw_input_fn=None, discard_local_changes=False, keep_stash=False, - pre_sync_sha=plan.pre_sync_sha, rollback_branch=plan.rollback_branch, - ) + pull(plan) assert exc.value.code == 1 assert git(root, "rev-parse", "HEAD") == old assert git(root, "rev-parse", "--abbrev-ref", "HEAD") == ("feature" if parked else "HEAD") @@ -92,6 +90,26 @@ def test_syntax_failure_returns_to_original_checkout_without_rewriting_main(chec assert (root / "cli.py").read_text(encoding="utf8") == "value = 1\n" + +def test_rollback_restores_the_commit_when_the_parked_branch_is_taken(checkout, tmp_path): + """Another worktree holding the parked branch must not leave the install on broken code.""" + root, old, tip = checkout + git(root, "checkout", "-qb", "feature") + git(root, "commit", "--allow-empty", "-qm", "local") + feature_tip = git(root, "rev-parse", "HEAD") + git(root, "checkout", "-q", "main") + (root / "cli.py").write_text("def broken(\n", encoding="utf8") + git(root, "commit", "-qam", "bad upstream") + git(root, "update-ref", "refs/remotes/origin/main", git(root, "rev-parse", "HEAD")) + git(root, "checkout", "-q", "feature") + plan = prepare(root) + git(root, "worktree", "add", "-q", str(tmp_path / "other"), "feature") + with pytest.raises(SystemExit): + pull(plan) + assert git(root, "rev-parse", "HEAD") == feature_tip + assert git(root, "rev-parse", "--abbrev-ref", "HEAD") == "HEAD" + assert (root / "cli.py").read_text(encoding="utf8") == "value = 1\n" + def test_locally_ahead_switch_still_needs_completion(checkout): root, old, tip = checkout git(root, "checkout", "-q", "--detach", tip) @@ -123,11 +141,7 @@ def test_merge_that_reports_success_without_moving_stale_main_is_refused(checkou monkeypatch.setattr(update_cmd, "_git_run", merge_is_a_silent_noop) with pytest.raises(SystemExit): - update_cmd._pull_updates( - ["git"], "main", plan.auto_stash_ref, prompt_for_restore=False, - gw_input_fn=None, discard_local_changes=False, keep_stash=False, - pre_sync_sha=plan.pre_sync_sha, rollback_branch=plan.rollback_branch, - ) + pull(plan) assert git(root, "rev-parse", "HEAD") == old @@ -137,11 +151,7 @@ def test_stale_local_main_can_catch_up_to_original_detached_tip(checkout): git(root, "checkout", "-q", "--detach", tip) plan = prepare(root) assert plan.commit_count != 0 - update_cmd._pull_updates( - ["git"], "main", plan.auto_stash_ref, prompt_for_restore=False, - gw_input_fn=None, discard_local_changes=False, keep_stash=False, - pre_sync_sha=plan.pre_sync_sha, rollback_branch=plan.rollback_branch, - ) + pull(plan) assert git(root, "rev-parse", "main") == tip @@ -207,12 +217,7 @@ def test_early_fork_sync_without_push_still_completes(checkout, monkeypatch): assert plan.upstream_checked assert git(root, "rev-parse", "HEAD") == upstream_tip assert git(root, "rev-parse", "origin/main") == tip - before_pull = update_cmd._pull_updates( - ["git"], "main", plan.auto_stash_ref, prompt_for_restore=False, - gw_input_fn=None, discard_local_changes=False, keep_stash=False, - pre_sync_sha=plan.pre_sync_sha, rollback_branch=plan.rollback_branch, - sync_upstream=True, - ) + before_pull = pull(plan, sync_upstream=True) completed = [] monkeypatch.setattr(update_cmd, "_complete_source_update", completed.append) request = {} @@ -230,7 +235,6 @@ def test_command_hands_off_after_existing_main_switch(update_tree, monkeypatch, run = subprocess.run def local_git_only(command, *args, **kwargs): - from pathlib import Path assert Path(command[0]).name.lower() in {"git", "git.exe"}, command assert Path(kwargs["cwd"]).resolve() in {t.clone, t.origin}, command return run(command, *args, **kwargs) @@ -251,11 +255,7 @@ def test_fork_sync_round_trip_is_not_misclassified_as_noop(tmp_path, monkeypatch """origin/main moves first, then the upstream sync returns HEAD to the SHA that was running before the update: still a successful branch repair, not a no-op.""" root = tmp_path / "fork" - root.mkdir() - git(root, "init", "-q", "-b", "main") - git(root, "config", "user.name", "Fixture") - git(root, "config", "user.email", "fixture@example.com") - monkeypatch.setattr(update_cmd._m(), "PROJECT_ROOT", root) + init_repo(root, monkeypatch) def commit(value): (root / "state.txt").write_text(value + "\n", encoding="utf-8") @@ -279,11 +279,7 @@ def test_fork_sync_round_trip_is_not_misclassified_as_noop(tmp_path, monkeypatch return True monkeypatch.setattr(update_cmd._m(), "_sync_with_upstream_if_needed", sync_upstream) - update_cmd._pull_updates( - ["git"], "main", plan.auto_stash_ref, prompt_for_restore=False, - gw_input_fn=None, discard_local_changes=False, keep_stash=False, - pre_sync_sha=plan.pre_sync_sha, rollback_branch=plan.rollback_branch, sync_upstream=True, - ) + pull(plan, sync_upstream=True) assert git(root, "rev-parse", "HEAD") == upstream_tip assert git(root, "rev-parse", "main") == upstream_tip diff --git a/website/docs/getting-started/updating.md b/website/docs/getting-started/updating.md index bef23da263..bebadc87ec 100644 --- a/website/docs/getting-started/updating.md +++ b/website/docs/getting-started/updating.md @@ -135,7 +135,7 @@ For an admitted source checkout, `hermes update` runs these phases: 1. **Pre-update snapshot** — Hermes saves selected state files for every profile in that profile's `state-snapshots/` directory. These include pairing data, cron jobs, `config.yaml`, `.env`, and `auth.json`. Automatic quick snapshots skip individual files larger than 1 GiB. `updates.pre_update_backup` selects `quick`, `full`, or `off`. Full archives use the [backup exclusions](../reference/faq.md#hermes-backup-vs-hermes-profile-export). Recovery uses [Snapshots and rollback](../user-guide/checkpoints-and-rollback.md). Quick snapshots recover state files, not application code. The snapshot is best-effort: if it fails, the update prints a `⚠ Pre-update snapshot FAILED` warning and continues, and the receipt records `pre_update_backup` as a failed step (a deliberate `off`/`--no-backup` lands in the receipt's skips with its reason instead). 2. **Code update** — applies the configured source branch or stable release tag and updates submodules. -3. **Post-pull syntax validation + auto-rollback** — after the pull, Hermes compiles the nine critical files every `hermes` invocation imports at startup. If any fails to parse (e.g. an orphan merge-conflict marker, an accidentally truncated file), Hermes runs `git reset --hard ` to roll the install back so your shell stays bootable. Re-run `hermes update` once the upstream fix lands. +3. **Post-pull syntax validation + auto-rollback** — after the pull, Hermes compiles the nine critical files every `hermes` invocation imports at startup. If any fails to parse (e.g. an orphan merge-conflict marker, an accidentally truncated file), Hermes rolls the install back so your shell stays bootable: `git reset --hard ` on the update branch, or, when the update started from a detached checkout or a parked feature branch, `git checkout --detach ` / `git checkout ` back to where it started (a branch it cannot check out again is restored detached at the same commit). Re-run `hermes update` once the upstream fix lands. 4. **Dependency preparation** — PM provisions required tools and prepares a complete Python environment from the new lock, existing extras, and enabled plugin requirements. It validates that environment before publishing its selection. A plugin never fails the update. A plugin that no longer fits the new core is added to `plugins.disabled` in every profile that enables it (a memory provider has `memory.provider` cleared). That covers a `requires-python` that excludes Hermes's Python, a `manifest_version` newer than this Hermes supports, and dependencies that the resolver proves can't resolve alongside core and earlier plugins in config order, or that fail their own build. A download or network failure gets one retry and is disabled if it fails again. The update prints `⚠ Disabled plugin '' in : `, records it in the receipt's warnings, and continues. Re-enable it with `hermes plugins enable ` once the plugin ships a compatible release, or once the network is back. A `requires_hermes` range the running version misses never disables a plugin, because a source checkout without release tags can read as an older release. That plugin sits out instead (`⚠ Left plugin '' … out of this update`), stays enabled, and rejoins once Hermes reports a version it accepts. A secondary profile whose config cannot be read sits out the same way until the config is fixed. Only a core that cannot build on its own fails this step. 5. **Config migration** — detects new config options added since your version and prompts you to set them 6. **Desktop rebuild (stage-and-swap)** — if the Hermes Desktop app was built from this checkout, it is rebuilt so the GUI matches the new code. The rebuild packs into a temporary staging directory next to `apps/desktop/release/`, verifies the staged app, and only then renames it over the previous build (on Windows a real-time scanner briefly holding `release/win-unpacked` is ridden out with a few short retries). A rebuild that fails at any point — corrupt Electron download, missing dependency, disk full — leaves the previous app untouched and launchable; the update fails at that step, and `hermes desktop --build-only --force-build` or the next `hermes update` retries the rebuild. On macOS the rebuilt bundle is then copied (with `ditto`, signature intact) over a stale `/Applications/Hermes.app` or `~/Applications/Hermes.app`, so the copy Finder and the Dock launch matches the backend; an installed copy that is currently running is left alone and the update tells you to quit it and run `hermes update` again. On Windows, when the update finishes from inside the Desktop app it would rebuild (the app completing an interrupted update at launch), the rebuild is skipped with a notice instead: Windows locks a running app's files, and stopping the app would end the update with it. The rest of the update completes; quit Hermes Desktop and run `hermes desktop` from a terminal, or use **Update now** in **Settings → About**, to rebuild and reopen it.