fix(cli): treat a fork's upstream sync as an update
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
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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(
|
||||
|
||||
Reference in New Issue
Block a user