From 5d4071ec554ca811fb335c46ada68c7631f6cdc8 Mon Sep 17 00:00:00 2001 From: mkpoli Date: Wed, 22 Jul 2026 06:58:00 +0900 Subject: [PATCH 1/2] fix(update): don't flag canonical origin URL spellings as forks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _is_fork() compared the output of `git remote get-url origin` against four exact URL strings. Any other spelling of the official repository was announced as "Updating from fork" and walked the whole fork-sync flow: - ssh://git@github.com/NousResearch/hermes-agent.git — the same repo in ssh-URL form — was flagged as a fork. - `git remote get-url` applies `url..insteadOf` rewrites at read time, so installs whose users bind a specific SSH identity or route through a proxy via insteadOf showed the rewritten URL and were flagged too, even though the stored remote.origin.url was canonical. Parse the origin URL into a lowercased host/owner/repo identity (same shape as banner.py's _canonical_github_remote) and compare it exactly, accepting the scp-like, ssh:// and https:// spellings (with or without .git, user component, or port). Because `git remote get-url` alone cannot see through insteadOf rewrites or SSH host aliases, the stored, unrewritten remote.origin.url is read as well and the origin counts as the official repo if either reading parses as official — identity-binding alias configurations stay canonical, while genuine forks and mirrors are non-canonical under both readings. Hosts that are not literally github.com cannot be resolved to a physical host from here, so the update flow calls unrecognized origins "non-canonical" instead of asserting forkhood, every user-facing "Fork" message now says what is actually known, and the failed push-back message lists the read-only mirror / aliased-URL case alongside missing write access. The sync flow itself is unchanged: an origin strictly behind upstream is still fast-forwarded and pushed back exactly as before. --- hermes_cli/main.py | 8 +- hermes_cli/update_cmd.py | 171 +++++++++++++++----- tests/hermes_cli/test_cmd_update.py | 241 ++++++++++++++++++++++++++++ 3 files changed, 373 insertions(+), 47 deletions(-) diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 814fa7e8f327b..0f2604243eb54 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -5009,22 +5009,25 @@ def _clear_bytecode_cache(root: Path) -> int: _format_venv_python_holders_message, _gateway_prompt, _get_origin_url, + _get_stored_origin_url, _has_upstream_remote, _install_psutil_android_compat, _invalidate_update_cache, _is_android_python, - _is_fork, + _is_noncanonical_origin, _log_only_write, _mark_skip_upstream_prompt, _npm_bin_exists, _npm_lockfile_changed, _npm_manifest_paths, _npm_manifests_digest, + _origin_is_official_repo, _pause_windows_gateways_for_update, _print_curator_first_run_notice, _print_curator_recent_run_notice, _print_fts_optimize_available_notice, _print_stash_cleanup_guidance, + _push_main_to_origin, _record_npm_lockfile_hash, _refresh_active_lazy_features, _refresh_active_memory_provider_dependencies, @@ -5039,7 +5042,6 @@ def _clear_bytecode_cache(root: Path) -> int: _should_skip_upstream_prompt, _stash_apply_failed_only_on_existing_untracked, _stash_local_changes_if_needed, - _sync_fork_with_upstream, _sync_with_upstream_if_needed, _update_node_dependencies, _update_via_zip, @@ -5056,7 +5058,7 @@ def _clear_bytecode_cache(root: Path) -> int: _write_update_planned_stop_marker, _UPDATE_RUNTIME_RELOAD_MODULES, _UPDATE_CRITICAL_FILES, - OFFICIAL_REPO_URLS, + OFFICIAL_REPO_IDENTITY, OFFICIAL_REPO_URL, SKIP_UPSTREAM_PROMPT_FILE, _PRE_UPDATE_SNAPSHOT_KEEP, diff --git a/hermes_cli/update_cmd.py b/hermes_cli/update_cmd.py index 775a2b6a2901c..bd368c656d4dc 100644 --- a/hermes_cli/update_cmd.py +++ b/hermes_cli/update_cmd.py @@ -1134,12 +1134,10 @@ def _discard_stashed_changes( print("→ Discarded local source changes (updates.non_interactive_local_changes=discard).") return True -OFFICIAL_REPO_URLS = { - "https://github.com/NousResearch/hermes-agent.git", - "git@github.com:NousResearch/hermes-agent.git", - "https://github.com/NousResearch/hermes-agent", - "git@github.com:NousResearch/hermes-agent", -} +# Lowercased host/owner/repo identity of the official repository, compared +# against the parsed origin URL so every spelling (scp-like, ssh://, +# https://, any letter case) matches. +OFFICIAL_REPO_IDENTITY = "github.com/nousresearch/hermes-agent" OFFICIAL_REPO_URL = "https://github.com/NousResearch/hermes-agent.git" @@ -1160,21 +1158,89 @@ def _get_origin_url(git_cmd: list[str], cwd: Path) -> Optional[str]: pass return None -def _is_fork(origin_url: Optional[str]) -> bool: - """Check if the origin remote points to a fork (not the official repo).""" - if not origin_url: +def _get_stored_origin_url(git_cmd: list[str], cwd: Path) -> Optional[str]: + """Read remote.origin.url exactly as stored in .git/config. + + Unlike ``git remote get-url`` (used by ``_get_origin_url``), this does + NOT apply ``url..insteadOf`` rewrites, so it reflects the URL the + remote was actually configured with — the canonical value on installs + where the user's git config rewrites GitHub URLs to an SSH alias, + mirror, or proxy at transport time. + """ + try: + result = subprocess.run( + git_cmd + ["config", "--get", "remote.origin.url"], + cwd=cwd, + capture_output=True, + text=True, encoding="utf-8", errors="replace", + ) + if result.returncode == 0: + value = result.stdout.strip() + return value or None + except Exception: + pass + return None + +def _origin_is_official_repo(origin_url: str) -> bool: + """Return True if *origin_url* addresses the official repository. + + Parses the URL into a lowercased ``host/owner/repo`` identity and + compares it exactly, so the scp-like (``git@github.com:owner/repo``), + ``ssh://`` and ``https://`` spellings all match, regardless of letter + case, trailing ``.git``, user component or numeric port. Extra path + segments (``.../hermes-agent/tree/main``), non-git transport schemes + (``file://``, ``ftp://``), and non-numeric ports do not match — the + identity must be exactly the official repo over a git transport. + + Hosts that are not literally ``github.com`` — SSH config aliases such as + ``gh-work:owner/repo``, mirrors, proxies, and the effective URLs produced + by ``url..insteadOf`` rewrites — return False: they cannot be + resolved to a physical host from here, so they are conservatively treated + as non-canonical. + """ + url = origin_url.strip().rstrip("/") + if url.lower().endswith(".git"): + url = url[:-4] + + if "://" in url: + # https://github.com/owner/repo, ssh://git@github.com[:port]/owner/repo + scheme, rest = url.split("://", 1) + if scheme.lower() not in {"https", "http", "ssh", "git"}: + return False + host, sep, path = rest.partition("/") + if not sep: + return False + elif ":" in url: + # scp-like: git@github.com:owner/repo, gh-work:owner/repo + host, path = url.split(":", 1) + else: return False - # Normalize URL for comparison (strip trailing .git if present) - normalized = origin_url.rstrip("/") - if normalized.endswith(".git"): - normalized = normalized[:-4] - for official in OFFICIAL_REPO_URLS: - official_normalized = official.rstrip("/") - if official_normalized.endswith(".git"): - official_normalized = official_normalized[:-4] - if normalized == official_normalized: + + host = host.rsplit("@", 1)[-1] # drop any user component + if ":" in host: + host, port = host.rsplit(":", 1) + if port and not port.isdigit(): return False - return True + + segments = [s for s in path.split("/") if s] + if len(segments) != 2: + return False + + identity = f"{host}/{segments[0]}/{segments[1]}".casefold() + return identity == OFFICIAL_REPO_IDENTITY + +def _is_noncanonical_origin(origin_url: Optional[str]) -> bool: + """Check if the origin remote is not the official repository. + + Returns True for anything that does not parse as the official repo under + a recognized URL spelling: genuine forks, mirrors, and origins reached + through SSH aliases or ``url.insteadOf`` rewrites. The update flow uses + this as "origin is not confirmed canonical" — the host behind an aliased + URL cannot be verified from here, so this is never proof of forkhood. + """ + if not origin_url: + return False + return not _origin_is_official_repo(origin_url) def _has_upstream_remote(git_cmd: list[str], cwd: Path) -> bool: """Check if an 'upstream' remote already exists.""" @@ -1232,8 +1298,8 @@ def _mark_skip_upstream_prompt(): except Exception: pass -def _sync_fork_with_upstream(git_cmd: list[str], cwd: Path) -> bool: - """Attempt to push updated main to origin (sync fork). +def _push_main_to_origin(git_cmd: list[str], cwd: Path) -> bool: + """Attempt to push the updated local main to origin. Returns True if push succeeded, False otherwise. """ @@ -1249,13 +1315,12 @@ def _sync_fork_with_upstream(git_cmd: list[str], cwd: Path) -> bool: return False def _sync_with_upstream_if_needed(git_cmd: list[str], cwd: Path) -> None: - """Check if fork is behind upstream and sync if safe. + """Check if origin is behind upstream and sync if safe. - This implements the fork upstream sync logic: - If upstream remote doesn't exist, ask user if they want to add it - Compare origin/main with upstream/main - If origin/main is strictly behind upstream/main, pull from upstream - - Try to sync fork back to origin if possible + - Try to push the updated main back to origin if possible """ has_upstream = _has_upstream_remote(git_cmd, cwd) @@ -1266,7 +1331,7 @@ def _sync_with_upstream_if_needed(git_cmd: list[str], cwd: Path) -> None: # Ask user if they want to add upstream print() - print("ℹ Your fork is not tracking the official Hermes repository.") + print("ℹ This origin is not the canonical Hermes repository URL.") print(" This means you may miss updates from NousResearch/hermes-agent.") print() try: @@ -1323,20 +1388,20 @@ def _sync_with_upstream_if_needed(git_cmd: list[str], cwd: Path) -> None: # If origin/main has commits not on upstream, don't trample if origin_ahead > 0: print() - print(f"ℹ Your fork has {origin_ahead} commit(s) not on upstream.") + print(f"ℹ Origin has {origin_ahead} commit(s) not on upstream.") print(" Skipping upstream sync to preserve your changes.") print(" If you want to merge upstream changes, run:") print(" git pull upstream main") return - # If upstream is not ahead, fork is up to date + # If upstream is not ahead, origin is up to date if upstream_ahead == 0: - print(" ✓ Fork is up to date with upstream") + print(" ✓ Origin is up to date with upstream") return # origin/main is strictly behind upstream/main (can fast-forward) print() - print(f"→ Fork is {upstream_ahead} commit(s) behind upstream") + print(f"→ Origin is {upstream_ahead} commit(s) behind upstream") print("→ Pulling from upstream...") try: @@ -1353,15 +1418,22 @@ def _sync_with_upstream_if_needed(git_cmd: list[str], cwd: Path) -> None: print(" ✓ Updated from upstream") - # Try to sync fork back to origin - print("→ Syncing fork...") - if _sync_fork_with_upstream(git_cmd, cwd): - print(" ✓ Fork synced with upstream") + # Try to push the updated main back to origin. For a fork on GitHub this + # fast-forwards its main; for a read-only mirror or an aliased URL of the + # official repo the push fails harmlessly — the local repo is already + # updated. + print("→ Syncing origin with upstream...") + if _push_main_to_origin(git_cmd, cwd): + print(" ✓ Origin synced with upstream") else: + print(" ℹ Got updates from upstream but couldn't push to origin.") + print( + " No write access? Origin may also be a read-only mirror or an" + ) print( - " ℹ Got updates from upstream but couldn't push to fork (no write access?)" + " aliased URL of the official repository (SSH alias / url.insteadOf)." ) - print(" Your local repo is updated, but your fork on GitHub may be behind.") + print(" Your local repo is updated, but origin's main may be behind.") def _invalidate_update_cache(): """Delete the update-check cache for ALL profiles so no banner @@ -3163,7 +3235,7 @@ def _cmd_update_impl(args, gateway_mode: bool): capture_output=True, ) - # Build git command once — reused for fork detection and the update itself. + # Build git command once — reused for origin classification and the update itself. git_cmd = ["git"] if sys.platform == "win32": git_cmd = ["git", "-c", "windows.appendAtomically=false"] @@ -3177,12 +3249,23 @@ def _cmd_update_impl(args, gateway_mode: bool): # lockfile churn) update with a clean tree. _discard_lockfile_churn(git_cmd, _m().PROJECT_ROOT) - # Detect if we're updating from a fork (before any branch logic) + # Classify the origin before any branch logic. + # `git remote get-url` applies url..insteadOf rewrites, so also + # read the stored, unrewritten remote.origin.url: an origin counts as + # the official repo if EITHER spelling parses as official. Users who + # bind a specific SSH identity or proxy via insteadOf keep a canonical + # stored URL and must not be treated as a fork; genuine forks and + # mirrors are non-canonical under both readings. origin_url = _m()._get_origin_url(git_cmd, _m().PROJECT_ROOT) - is_fork = _is_fork(origin_url) + stored_origin_url = _m()._get_stored_origin_url(git_cmd, _m().PROJECT_ROOT) + noncanonical_origin = _is_noncanonical_origin(origin_url) and ( + stored_origin_url is None or _is_noncanonical_origin(stored_origin_url) + ) - if is_fork: - print("⚠ Updating from fork:") + if noncanonical_origin: + print( + "⚠ Updating from a non-canonical origin (fork, mirror, or rewritten URL):" + ) print(f" {origin_url}") print() @@ -3306,8 +3389,8 @@ def _cmd_update_impl(args, gateway_mode: bool): 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": + # Even if we're up to date with origin, origin may be behind upstream + if noncanonical_origin 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 @@ -3531,8 +3614,8 @@ def _cmd_update_impl(args, gateway_mode: bool): ) _m()._record_bytecode_fingerprint() - # Fork upstream sync logic (only for main branch on forks) - if is_fork and branch == "main": + # Upstream sync for non-canonical origins (main branch only) + if noncanonical_origin and branch == "main": _m()._sync_with_upstream_if_needed(git_cmd, _m().PROJECT_ROOT) # Reinstall Python dependencies. Prefer .[all], but if one optional extra diff --git a/tests/hermes_cli/test_cmd_update.py b/tests/hermes_cli/test_cmd_update.py index e2a546da587ca..378c9fba287c4 100644 --- a/tests/hermes_cli/test_cmd_update.py +++ b/tests/hermes_cli/test_cmd_update.py @@ -753,3 +753,244 @@ def test_wsl_update_skips_windows_npm_build_paths(self, mock_args, monkeypatch): not call.args or not call.args[0] or call.args[0][0] != windows_npm for call in mock_run.call_args_list ) + + +class TestOriginOfficialUrlRecognition: + """_is_noncanonical_origin must recognize the official repo across URL spellings. + + The old string-equality check false-positived on any non-listed spelling + of the canonical URL — e.g. ssh://git@github.com/... or the effective URL + produced by a git url.insteadOf rewrite — announcing "Updating from fork" + for stock canonical installs. + """ + + @pytest.mark.parametrize( + "url", + [ + "https://github.com/NousResearch/hermes-agent.git", + "https://github.com/NousResearch/hermes-agent", + "https://github.com/NousResearch/hermes-agent/", + "git@github.com:NousResearch/hermes-agent.git", + "git@github.com:NousResearch/hermes-agent", + "ssh://git@github.com/NousResearch/hermes-agent.git", + "ssh://git@github.com/NousResearch/hermes-agent", + "ssh://git@github.com:22/NousResearch/hermes-agent.git", + "HTTPS://GITHUB.COM/NousResearch/hermes-agent.git", + "https://GitHub.com/nousresearch/HERMES-AGENT", + "https://github.com/NousResearch/hermes-agent.GIT", + "https://TOKEN@github.com/NousResearch/hermes-agent.git", + ], + ) + def test_official_url_spellings_are_recognized(self, url): + from hermes_cli import main as hm + + assert hm._is_noncanonical_origin(url) is False + + @pytest.mark.parametrize( + "url", + [ + "https://github.com/someone/hermes-agent.git", + "git@github.com:someone/hermes-agent.git", + "https://gitlab.com/NousResearch/hermes-agent.git", + "gh-work:NousResearch/hermes-agent.git", # SSH config alias host + "https://mirror.example.com/NousResearch/hermes-agent.git", + "https://github.com.evil.com/NousResearch/hermes-agent.git", + "https://github.com@evil.com/NousResearch/hermes-agent.git", + "git@github.com:NousResearch/hermes-agent2", + "https://github.com/NousResearch/hermes-agent/tree/main", + "https://github.com/NousResearch", + "file://github.com/NousResearch/hermes-agent.git", + "ftp://github.com/NousResearch/hermes-agent.git", + "https://github.com:evil.com/NousResearch/hermes-agent", + ], + ) + def test_non_official_urls_are_flagged(self, url): + from hermes_cli import main as hm + + assert hm._is_noncanonical_origin(url) is True + + def test_missing_origin_is_not_flagged(self): + from hermes_cli import main as hm + + assert hm._is_noncanonical_origin(None) is False + + +class TestUpdateNonCanonicalOriginBanner: + """The update banner must not assert forkhood for unrecognized origins.""" + + @patch("shutil.which", return_value=None) + @patch("subprocess.run") + def test_banner_does_not_assert_fork_for_aliased_origin( + self, mock_run, _mock_which, mock_args, capsys + ): + """An origin that is the official repo under an SSH alias or a + url.insteadOf rewrite must be called non-canonical, never a fork: + the host an alias resolves to cannot be verified from here. + """ + from hermes_cli import main as hm + + mock_run.side_effect = _make_run_side_effect( + branch="main", verify_ok=True, commit_count="0" + ) + + with patch.object( + hm, + "_get_origin_url", + return_value="gh-work:NousResearch/hermes-agent.git", + ), patch.object( + hm, "_get_stored_origin_url", return_value=None + ), patch.object(hm, "_sync_with_upstream_if_needed") as sync_mock: + cmd_update(mock_args) + + out = capsys.readouterr().out + assert "non-canonical origin" in out + assert "Updating from fork" not in out + sync_mock.assert_called_once() + + @patch("shutil.which", return_value=None) + @patch("subprocess.run") + def test_ssh_url_spelling_is_recognized_as_canonical( + self, mock_run, _mock_which, mock_args, capsys + ): + """ssh://git@github.com/... is the official repo — no banner and no + fork-sync flow.""" + from hermes_cli import main as hm + + mock_run.side_effect = _make_run_side_effect( + branch="main", verify_ok=True, commit_count="0" + ) + + with patch.object( + hm, + "_get_origin_url", + return_value="ssh://git@github.com/NousResearch/hermes-agent.git", + ), patch.object(hm, "_sync_with_upstream_if_needed") as sync_mock: + cmd_update(mock_args) + + out = capsys.readouterr().out + assert "non-canonical origin" not in out + sync_mock.assert_not_called() + + @patch("shutil.which", return_value=None) + @patch("subprocess.run") + def test_aliased_origin_with_canonical_stored_url_is_not_a_fork( + self, mock_run, _mock_which, mock_args, capsys + ): + """url.insteadOf rewrites only what `git remote get-url` reports; + the stored remote.origin.url stays canonical. An origin that parses + as official under either reading must not enter the fork flow — + this is the identity-binding SSH-alias configuration. + """ + from hermes_cli import main as hm + + mock_run.side_effect = _make_run_side_effect( + branch="main", verify_ok=True, commit_count="0" + ) + + with patch.object( + hm, + "_get_origin_url", + return_value="gh-work:NousResearch/hermes-agent.git", + ), patch.object( + hm, + "_get_stored_origin_url", + return_value="git@github.com:NousResearch/hermes-agent.git", + ), patch.object(hm, "_sync_with_upstream_if_needed") as sync_mock: + cmd_update(mock_args) + + out = capsys.readouterr().out + assert "non-canonical origin" not in out + assert "Updating from fork" not in out + sync_mock.assert_not_called() + + @patch("shutil.which", return_value=None) + @patch("subprocess.run") + def test_official_effective_url_wins_regardless_of_stored_url( + self, mock_run, _mock_which, mock_args, capsys + ): + """An effective URL that parses as official settles the question on + its own — a non-canonical stored URL (insteadOf rewriting the other + way) must not drag the update into the sync flow. + """ + from hermes_cli import main as hm + + mock_run.side_effect = _make_run_side_effect( + branch="main", verify_ok=True, commit_count="0" + ) + + with patch.object( + hm, + "_get_origin_url", + return_value="ssh://git@github.com/NousResearch/hermes-agent.git", + ), patch.object( + hm, + "_get_stored_origin_url", + return_value="gh-work:NousResearch/hermes-agent.git", + ), patch.object(hm, "_sync_with_upstream_if_needed") as sync_mock: + cmd_update(mock_args) + + out = capsys.readouterr().out + assert "non-canonical origin" not in out + sync_mock.assert_not_called() + + @patch("shutil.which", return_value=None) + @patch("subprocess.run") + def test_origin_non_canonical_under_both_readings_keeps_fork_flow( + self, mock_run, _mock_which, mock_args, capsys + ): + """A genuine fork or mirror is non-canonical in both the effective + and the stored URL — the banner and sync flow still run.""" + from hermes_cli import main as hm + + mock_run.side_effect = _make_run_side_effect( + branch="main", verify_ok=True, commit_count="0" + ) + + with patch.object( + hm, + "_get_origin_url", + return_value="https://github.com/someone/hermes-agent.git", + ), patch.object( + hm, + "_get_stored_origin_url", + return_value="https://github.com/someone/hermes-agent.git", + ), patch.object(hm, "_sync_with_upstream_if_needed") as sync_mock: + cmd_update(mock_args) + + out = capsys.readouterr().out + assert "non-canonical origin" in out + sync_mock.assert_called_once() + + +class TestSyncPushFailureMessage: + """The failed push-back message must cover mirrors/aliased URLs.""" + + @patch("subprocess.run") + def test_push_failure_mentions_mirror_and_alias_case( + self, mock_run, capsys + ): + from hermes_cli import main as hm + from hermes_cli import update_cmd + + def side_effect(cmd, **kwargs): + joined = " ".join(str(c) for c in cmd) + if "rev-list" in joined: + # origin_ahead (upstream/main..origin/main) → 0 + if "upstream/main..origin/main" in joined: + return subprocess.CompletedProcess(cmd, 0, stdout="0\n", stderr="") + # upstream_ahead (origin/main..upstream/main) → 1 + return subprocess.CompletedProcess(cmd, 0, stdout="1\n", stderr="") + if "push" in joined: + return subprocess.CompletedProcess(cmd, 1, stdout="", stderr="denied") + return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="") + + mock_run.side_effect = side_effect + # _sync_with_upstream_if_needed calls its module-local helper, so the + # patch must target update_cmd, not the main-module re-export. + with patch.object(update_cmd, "_has_upstream_remote", return_value=True): + hm._sync_with_upstream_if_needed(["git"], hm.PROJECT_ROOT) + + out = capsys.readouterr().out + assert "couldn't push to origin" in out + assert "mirror" in out + assert "insteadOf" in out From 5bf2bfbcc1f3826dbcad561d08355a9004c5b7b4 Mon Sep 17 00:00:00 2001 From: mkpoli Date: Wed, 22 Jul 2026 06:58:00 +0900 Subject: [PATCH 2/2] chore(contributors): map commit-author email --- contributors/emails/git@mkpo.li | 1 + 1 file changed, 1 insertion(+) create mode 100644 contributors/emails/git@mkpo.li diff --git a/contributors/emails/git@mkpo.li b/contributors/emails/git@mkpo.li new file mode 100644 index 0000000000000..21c90083056ec --- /dev/null +++ b/contributors/emails/git@mkpo.li @@ -0,0 +1 @@ +mkpoli