From e366df6889e8554870ab1ededd83e70eb5ab0235 Mon Sep 17 00:00:00 2001 From: Franci Penov Date: Tue, 28 Jul 2026 15:27:21 -0700 Subject: [PATCH] fix(cli): treat a fork's upstream sync as an update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On a fork, `hermes update` compares HEAD against origin/main, and only then syncs the fork from upstream — inside the `commit_count == 0` branch, which returns immediately afterwards. So an update that pulls hundreds of commits from upstream prints "Already up to date!" and skips everything the post-update path does, including the dependency sync and the gateway restart. Observed on a fork-based deployment: 1654 commits pulled, "Already up to date!", and the launchd gateway left running. It then held pre-update modules in memory while lazily importing post-update ones, and failed later with an AttributeError for a method that plainly exists on disk — a mixed runtime that looks nothing like an update problem. Correlating every run in update.log, a restart happened on exactly the runs that pulled upstream *without* also claiming to be up to date, and never once they started co-occurring. Decide before the branch: capture HEAD, sync, and if HEAD moved, set commit_count from the range so the normal post-update path runs. The pull that follows is a no-op (the sync updates origin too); reaching the restart is the point. commit_count is floored at 1 — HEAD moving *is* the update, so a failed or zero count query must not send us back down the early return. steps still being skipped afterwards. Refs #73108 --- hermes_cli/update_cmd.py | 27 ++++++++++++++++++---- tests/hermes_cli/test_cmd_update.py | 36 +++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 4 deletions(-) diff --git a/hermes_cli/update_cmd.py b/hermes_cli/update_cmd.py index eafe4ef4aa..8997f03508 100644 --- a/hermes_cli/update_cmd.py +++ b/hermes_cli/update_cmd.py @@ -6181,13 +6181,32 @@ def _cmd_update_impl(args, gateway_mode: bool): # not behind, fall through to the up-to-date path. commit_count = counted if counted is not None else -1 + # A fork can match origin while still trailing upstream. The sync can + # therefore advance HEAD even though the origin comparison found no + # commits. Detect that BEFORE taking the no-update return so dependency + # refreshes, gateway restarts, AND the fleet version matrix still run + # for the pulled code (#73108 — previously the sync lived inside the + # commit_count == 0 branch, which returns immediately after: an update + # that pulled hundreds of upstream commits printed "Already up to + # date!" and verified nothing). + if commit_count == 0 and is_fork and branch == "main": + pre_sync_sha = _capture_head_sha(git_cmd, _m().PROJECT_ROOT) + _m()._sync_with_upstream_if_needed(git_cmd, _m().PROJECT_ROOT) + post_sync_sha = _capture_head_sha(git_cmd, _m().PROJECT_ROOT) + if pre_sync_sha and post_sync_sha and pre_sync_sha != post_sync_sha: + synced_count = _count_commits_between( + git_cmd, + _m().PROJECT_ROOT, + pre_sync_sha, + post_sync_sha, + ) + # HEAD moving is itself proof of an update. Keep the update + # path active even if the informational count cannot be read. + commit_count = max(1, synced_count) + if commit_count == 0: _invalidate_update_cache() - # Even if origin is up to date, the fork may be behind upstream - if is_fork and branch == "main": - _m()._sync_with_upstream_if_needed(git_cmd, _m().PROJECT_ROOT) - # Restore stash and switch back to original branch if we moved. # EXCEPTION: a parked feature branch we verified clean + fully # merged stays on the target — re-parking the checkout on the diff --git a/tests/hermes_cli/test_cmd_update.py b/tests/hermes_cli/test_cmd_update.py index 21f28a33ae..67fdf1b261 100644 --- a/tests/hermes_cli/test_cmd_update.py +++ b/tests/hermes_cli/test_cmd_update.py @@ -252,6 +252,42 @@ class TestCmdUpdateBranchFallback: captured = capsys.readouterr() assert "Already up to date!" in captured.out + @patch("shutil.which", return_value=None) + @patch("subprocess.run") + def test_fork_upstream_sync_that_moves_head_runs_post_update_steps( + self, mock_run, _mock_which, mock_args, capsys + ): + """A fork sync that pulls code must continue through post-update work.""" + from hermes_cli import main as hm + from hermes_cli import update_cmd + + mock_run.side_effect = _make_run_side_effect( + branch="main", verify_ok=True, commit_count="0" + ) + + # The first two reads bracket the upstream sync; later reads see the + # new HEAD while the normal update path finishes. + shas = iter(["aaaaaaa", "bbbbbbb"]) + + with patch.object( + hm, + "_get_origin_url", + return_value="https://github.com/example/hermes-agent.git", + ), patch.object( + update_cmd, + "_capture_head_sha", + side_effect=lambda *_args, **_kwargs: next(shas, "bbbbbbb"), + ), patch.object( + hm, "_sync_with_upstream_if_needed" + ), patch.object( + hm, "_reload_updated_runtime_modules" + ) as post_update_step: + cmd_update(mock_args) + + post_update_step.assert_called_once_with() + captured = capsys.readouterr() + assert "Already up to date!" not in captured.out + def test_update_non_interactive_runs_safe_config_migrations(self, mock_args, capsys): """Dashboard/web updates apply non-interactive migrations before restart.""" with patch("shutil.which", return_value=None), patch(