"""Tests for issue #1579: What's new link can open a 404 GitHub compare page. Bug shape: api/updates.py shipped current_sha=local-HEAD-short. When the local HEAD is not present upstream (unpushed work, dirty stage, fork, in-flight rebase, release-time merge commit), the resulting compare URL https://github.com//compare/... returns GitHub's 404 page because is not a public commit. Fix: Use `git merge-base HEAD ` instead of `git rev-parse HEAD`. merge-base is the most recent commit both local and upstream agree on, and (since `git fetch` succeeded just before) it is guaranteed to exist in the upstream GitHub repo. If merge-base fails (shallow clone with divergent histories), fall back to current_sha=None — the JS link guard suppresses the link rather than emitting a known-broken URL. """ import os import re import subprocess import sys from pathlib import Path from unittest.mock import patch REPO_ROOT = Path(__file__).resolve().parent.parent sys.path.insert(0, str(REPO_ROOT)) # ── 1. Server-side: api.updates._check_repo uses merge-base, not HEAD ── def _make_throwaway_repo(tmp_path, *, local_only_commits=0, upstream_advanced=0): """Create a tiny git repo with a fake 'origin' remote. Returns the local clone path. Set local_only_commits>0 to put commits on local HEAD that don't exist on origin (the #1579 trigger). Set upstream_advanced>0 to make the remote ahead. """ upstream = tmp_path / 'upstream.git' subprocess.run(['git', 'init', '--quiet', '--bare', str(upstream)], check=True) subprocess.run( ['git', '--git-dir', str(upstream), 'symbolic-ref', 'HEAD', 'refs/heads/master'], check=True, ) seed = tmp_path / 'seed' subprocess.run(['git', 'init', '--quiet', '--initial-branch=master', str(seed)], check=True) for cmd in [ ['git', '-C', str(seed), 'config', 'user.email', 'test@test.test'], ['git', '-C', str(seed), 'config', 'user.name', 'test'], ['git', '-C', str(seed), 'commit', '--allow-empty', '-m', 'initial', '--quiet'], ['git', '-C', str(seed), 'remote', 'add', 'origin', str(upstream)], ['git', '-C', str(seed), 'push', '--quiet', '-u', 'origin', 'master'], ]: subprocess.run(cmd, check=True) # Clone FIRST — local and upstream share the initial commit only. local = tmp_path / 'local' subprocess.run(['git', 'clone', '--quiet', str(upstream), str(local)], check=True) subprocess.run(['git', '-C', str(local), 'config', 'user.email', 'test@test.test'], check=True) subprocess.run(['git', '-C', str(local), 'config', 'user.name', 'test'], check=True) # Add local-only commits to the local clone (the #1579 trigger). These never # get pushed — they exist only on the local clone's master branch. for i in range(local_only_commits): subprocess.run(['git', '-C', str(local), 'commit', '--allow-empty', '-m', f'local-only commit {i}', '--quiet'], check=True) # Advance upstream by committing on the seed and pushing — so local clone # is now `upstream_advanced` commits behind on the remote-tracking branch. for i in range(upstream_advanced): subprocess.run(['git', '-C', str(seed), 'commit', '--allow-empty', '-m', f'upstream commit {i}', '--quiet'], check=True) if upstream_advanced: subprocess.run(['git', '-C', str(seed), 'push', '--quiet'], check=True) return local def _short_sha(repo, ref): out = subprocess.run(['git', '-C', str(repo), 'rev-parse', '--short', ref], capture_output=True, text=True, check=True) return out.stdout.strip() def test_current_sha_is_merge_base_not_local_HEAD(tmp_path, monkeypatch): """Reporter's exact scenario: local has unpushed commits, upstream advanced. Before #1579 fix: current_sha = local HEAD = unpublished SHA → URL 404s. After fix: current_sha = merge-base = the public ancestor commit → URL resolves. """ # Clear cached config (api.updates may import HERMES_HOME at import time) repo = _make_throwaway_repo( tmp_path, local_only_commits=2, upstream_advanced=3, ) head_sha = _short_sha(repo, 'HEAD') expected_base = _short_sha(repo, 'HEAD~2') # merge-base in this scenario # Import updates with a stable CWD if 'api.updates' in sys.modules: del sys.modules['api.updates'] from api import updates as upd result = upd._check_repo(repo, 'webui') assert result is not None, "non-bare repo with origin should return a result" assert result['behind'] == 3, f"expected 3 commits behind, got {result['behind']}" # The core fix: current_sha must be the merge-base, not local HEAD. # merge-base = HEAD~2 in this scenario (local has 2 unpushed commits, # so the most recent shared point with upstream is 2 commits before HEAD). assert result['current_sha'] == expected_base, ( f"current_sha should be merge-base ({expected_base}), got {result['current_sha']} " f"(local HEAD is {head_sha}). Old #1579 bug regressed." ) assert result['current_sha'] != head_sha, ( f"current_sha must NOT be local HEAD ({head_sha}) — that's the #1579 bug." ) # latest_sha is what _check_repo's own fetch+rev-parse returns assert result['latest_sha'], "latest_sha must be populated" # Critical compare-URL property: current_sha and latest_sha both correspond # to commits the upstream knows about (one by being upstream tip, the other # by being a shared ancestor). The merge-base is verifiable via the local # clone's remote-tracking branch: upstream_history = subprocess.run( ['git', '-C', str(repo), 'log', '--format=%h', 'origin/master'], capture_output=True, text=True, check=True, ).stdout.split() assert result['current_sha'] in upstream_history or any( h.startswith(result['current_sha']) for h in upstream_history ), ( f"current_sha ({result['current_sha']}) must be present in upstream history " f"— that's what guarantees the GitHub /compare/ URL won't 404." ) def test_current_sha_equals_HEAD_when_no_local_commits(tmp_path): """Backward-compat: pure-behind clone (no local-only commits) is unchanged. merge-base equals HEAD in this case — so the URL is identical to what we shipped before #1579. """ repo = _make_throwaway_repo(tmp_path, local_only_commits=0, upstream_advanced=4) if 'api.updates' in sys.modules: del sys.modules['api.updates'] from api import updates as upd result = upd._check_repo(repo, 'webui') head_sha = _short_sha(repo, 'HEAD') assert result['current_sha'] == head_sha, ( "Pure-behind clone: merge-base equals HEAD; URL should be unchanged " "from pre-#1579 behavior." ) assert result['behind'] == 4 def test_current_sha_falls_back_to_None_when_merge_base_fails(tmp_path): """Defensive: if merge-base errors (shallow clone, no shared history), return current_sha=None so the JS link guard suppresses the bad link rather than emitting one that 404s. """ repo = _make_throwaway_repo(tmp_path, local_only_commits=0, upstream_advanced=1) if 'api.updates' in sys.modules: del sys.modules['api.updates'] from api import updates as upd # Patch _run_git so any 'merge-base' call returns failure real_run = upd._run_git def fake_run(args, *a, **kw): if args and args[0] == 'merge-base': return ('', False) return real_run(args, *a, **kw) with patch.object(upd, '_run_git', side_effect=fake_run): result = upd._check_repo(repo, 'webui') assert result is not None assert result['current_sha'] is None, ( "merge-base failure must fall back to None so JS suppresses the link " "(emitting a known-broken URL is worse than no link)." ) # latest_sha should still be populated — that path doesn't depend on merge-base assert result['latest_sha'] # ── 2. Client-side: ui.js link guard suppresses URL on null current_sha ── def _read_ui_js(): return (REPO_ROOT / 'static' / 'ui.js').read_text(encoding='utf-8') def test_whats_new_link_resets_display_and_contents_on_every_render(): """Without reset, a stale link from a prior banner can stay visible after a re-render where the new payload has current_sha=None. """ src = _read_ui_js() idx = src.find("function _renderUpdateWhatsNewLinks(data)") assert idx != -1, "What's-new link renderer not found" block = src[idx:idx + 1200] clear_idx = block.find("container.replaceChildren()") hide_idx = block.find("container.style.display='none'") show_idx = block.find("container.style.display='block'") assert clear_idx != -1, "Missing container contents reset on every render" assert hide_idx != -1, "Missing display='none' reset when no safe links exist" assert clear_idx < show_idx, "contents reset must precede link rendering" assert hide_idx < show_idx, "hidden state must be handled before visible rendering" def test_whats_new_link_suppressed_when_current_sha_falsy(): """The legacy fallback must guard on all three of repo_url/current_sha/latest_sha.""" src = _read_ui_js() idx = src.find("function _updateCompareUrl(info)") assert idx != -1, "Compare URL helper not found" block = src[idx:idx + 500] compact = re.sub(r"\s+", "", block) assert "if(!(repo_url&¤tSha&&latestSha))returnnull;" in compact assert "constfallbackUrl=repo_url+'/compare/'+currentSha+'...'+latestSha;" in compact assert "return_isSafeUpdateCompareUrl(fallbackUrl)?fallbackUrl:null;" in compact # ── 3. End-to-end: simulate the exact reporter URL shape ── def test_reporter_url_shape_no_longer_produces_invalid_compare_url(tmp_path): """Reporter saw https://github.com/.../compare/c660c7f...86cb22e where c660c7f was an unpublished local SHA. After fix, the URL should use a SHA that exists upstream. """ repo = _make_throwaway_repo(tmp_path, local_only_commits=2, upstream_advanced=5) if 'api.updates' in sys.modules: del sys.modules['api.updates'] from api import updates as upd result = upd._check_repo(repo, 'webui') head_sha = _short_sha(repo, 'HEAD') base_sha = _short_sha(repo, 'HEAD~2') # the merge-base # The compare URL the JS would build cur, latest = result['current_sha'], result['latest_sha'] # In a real run repo_url is converted from origin's URL; in this test the # value will be a file:// path, but that's fine — what we care about is # the cur and latest shas. assert cur == base_sha assert cur != head_sha, "Must not use local HEAD (the #1579 reporter URL bug)" # The "merge-base...upstream-tip" URL is by construction valid because # both endpoints exist on the upstream (one by being the upstream tip, # the other by being a shared ancestor of upstream and local).