Skip to content

fix(update): fork updates that pull upstream code now run the full post-update path (#73108, salvage #73679) - #92769

Merged
teknium1 merged 2 commits into
mainfrom
hermes/hermes-83bfdb1e
Aug 23, 2026
Merged

teknium1 merged 2 commits into
mainfrom
hermes/hermes-83bfdb1e

Conversation

@teknium1

Copy link
Copy Markdown
Collaborator

Summary

A fork's hermes update that pulls hundreds of upstream commits no longer prints "Already up to date!" and skips everything — the upstream sync now runs BEFORE the no-update decision, so a HEAD-moving sync flows through the full post-update path: dependency sync, gateway restart, and the fleet version matrix (#73108's remaining leg; salvage of #73679 by @francip).

Root cause: _sync_with_upstream_if_needed lived INSIDE the commit_count == 0 branch, which returns immediately after — an update that pulled real code verified nothing (and on fork installs, bypassed the Phase-1 fleet matrix entirely).

Changes

  • hermes_cli/update_cmd.py (@francip): hoist the fork sync above the no-update decision; if HEAD moved across the sync, set commit_count from the moved range (min 1) so the normal update path runs. Reconciled with the shallow-repo commit-count block and the parked-branch stash handling that landed since his branch.
  • tests/hermes_cli/test_cmd_update.py (@francip + ours): his regression test, hardened for host isolation — pinned fleet/gateway discovery (real dev-box gateways read STALE / trip the live-system guard) and the pipeline aborts at _reload_updated_runtime_modules, the exact proof point the bug never reached.

Validation

Check Result
test_cmd_update.py new test passes; failure set byte-identical to origin/main on this host (6 pre-existing env-shaped reds, CI is arbiter)
Sabotage run (hoist disabled = old behavior) regression test FAILS — proves the fix
Attribution @francip's commit cherry-picked with authorship intact

Closes the fork leg of #73108; part of the Phase-1 completion pass on #91277 (the matrix now runs on fork updates too).

Infographic

The update that lied

francip and others added 2 commits August 22, 2026 23:59
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
… the reload proof point

The salvaged test drove the FULL post-update pipeline against the real
dev box: real fleet probes read live gateways as STALE (exit 1) and the
restart phase tripped the live-system guard on a real gateway PID. Pin
empty fleet/gateway discovery and make _reload_updated_runtime_modules
(the proof the post-update path ran — the bug returned before it) abort
the pipeline. Sabotage-verified: disabling the hoisted sync fails the
test.
@teknium1

Copy link
Copy Markdown
Collaborator Author

Live E2E (zero mocks) added to the validation record: built a real fork topology in /tmp — bare upstream 3 commits ahead, bare origin in sync with local, local checkout with both remotes. Confirmed the trigger state (HEAD..origin/main == 0, where pre-fix code returned early), then ran the REAL _capture_head_sha → REAL _sync_with_upstream_if_needed (actual fetch + FF merge, 74fea4bf→59ff3637) → hoist arithmetic → commit_count=3 → post-update path runs. No-op leg also proven live: upstream not ahead → HEAD unmoved → commit_count stays 0 → 'Already up to date!' remains correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/install-update Installer, updater, packaging, wheels, doctor comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants