fix(update): the main channel follows the git branch without reading an R2 record
Every `hermes update` on the main channel first fetched hermes-assets.nousresearch.com/releases/channels/main.json. That record has never been published; a 404 fell through to the git branch. Some regional WAFs answer that same request 403 instead, which raised and stopped the update outright (#128295; patched around in #128323 and #128305). main IS the git branch, and the only thing its record could ever carry is a retirement notice nobody has published. resolve_source_target now returns the main branch target directly, the way stable reads GitHub releases directly. Preview channels (e.g. pm-preview) keep their R2 records.
This commit is contained in:
Teknium
@@ -80,7 +80,8 @@ def _resolve_channel(name: str, repository: str):
|
||||
|
||||
def resolve_source_target(channel: str, git_cmd=None, cwd=None, *, repository=None,
|
||||
forward_only: bool = False) -> SourceTarget:
|
||||
"""Resolve every subscription, including default labels, through R2.
|
||||
"""Resolve a subscription: stable from GitHub releases, main from the git branch,
|
||||
preview channels (e.g. ``pm-preview``) through their R2 records.
|
||||
|
||||
``forward_only`` (an unchosen default subscription) pins a checkout that already
|
||||
contains the release to its own HEAD, so the update is a no-op instead of a downgrade.
|
||||
@@ -122,20 +123,18 @@ def _head_containing(git_cmd, cwd, commit: str, repository: str) -> str | None:
|
||||
|
||||
|
||||
def _resolve_source_target(channel: str, git_cmd=None, cwd=None, *, repository=None) -> SourceTarget:
|
||||
from hermes_cli.release_channels import ChannelNotFound, validate_name
|
||||
from hermes_cli.release_channels import validate_name
|
||||
|
||||
validate_name(channel)
|
||||
repository = repository or source_repository(git_cmd, cwd)
|
||||
if channel == "stable":
|
||||
return _resolve_stable(repository, git_cmd, cwd)
|
||||
try:
|
||||
resolved = _resolve_channel(channel, repository)
|
||||
except ChannelNotFound:
|
||||
if channel != "main":
|
||||
raise
|
||||
# main IS the source branch; its record can only add a retirement.
|
||||
# Until one is published, a checkout keeps following the branch via git.
|
||||
if channel == "main":
|
||||
# main IS the git branch. No R2 record is read: none was ever published, and a
|
||||
# network that answers it with 403 instead of 404 (regional WAFs) must not stop
|
||||
# an install from following the branch it is on.
|
||||
return SourceTarget(channel, channel, repository, branch="main")
|
||||
resolved = _resolve_channel(channel, repository)
|
||||
terminal = resolved.terminal
|
||||
if terminal["repository"].lower() != repository.lower():
|
||||
raise ValueError("Channel repository does not match this source installation")
|
||||
|
||||
@@ -134,7 +134,7 @@ def test_retirement_adopts_destination_only_after_success(source, monkeypatch, o
|
||||
assert len(requests) == 1
|
||||
|
||||
|
||||
@pytest.mark.parametrize("channel", ["preview-not-registered", "stable", "canary", "main"])
|
||||
@pytest.mark.parametrize("channel", ["preview-not-registered", "stable", "canary"])
|
||||
def test_missing_channel_cannot_fall_back_to_main(source, monkeypatch, channel):
|
||||
from hermes_cli import source_check
|
||||
|
||||
@@ -151,23 +151,36 @@ def test_missing_channel_cannot_fall_back_to_main(source, monkeypatch, channel):
|
||||
assert git(source.root, "rev-parse", "HEAD") == before
|
||||
|
||||
|
||||
def test_unpublished_main_record_keeps_following_the_git_branch(source, monkeypatch):
|
||||
"""main IS the source branch: until R2 publishes its record, a checkout
|
||||
still updates via git instead of failing on a missing channel object."""
|
||||
@pytest.mark.parametrize("failure", ["not-found", "forbidden", "timeout"])
|
||||
def test_main_follows_the_git_branch_without_reading_a_channel_record(source, monkeypatch, failure):
|
||||
"""main IS the source branch, so its R2 record is never read: a 403 from a regional
|
||||
WAF or a timeout cannot stop an install following main. Preview channels still read theirs."""
|
||||
import urllib.error
|
||||
|
||||
from hermes_cli import source_check
|
||||
from hermes_cli.release_channels import ChannelNotFound
|
||||
|
||||
set_install_channel("main", source.root)
|
||||
def unpublished(name, repository):
|
||||
reads = []
|
||||
|
||||
def record(name, repository):
|
||||
reads.append(name)
|
||||
if failure == "forbidden":
|
||||
raise urllib.error.HTTPError(f"https://x/releases/channels/{name}.json", 403, "Forbidden", {}, None)
|
||||
if failure == "timeout":
|
||||
raise TimeoutError("timed out")
|
||||
raise ChannelNotFound(f"Channel object not found: releases/channels/{name}.json")
|
||||
monkeypatch.setattr(source_releases, "_resolve_channel", unpublished)
|
||||
|
||||
monkeypatch.setattr(source_releases, "_resolve_channel", record)
|
||||
target = source_releases.resolve_source_target("main", ["git"], source.root)
|
||||
assert target.branch == "main" and target.commit is None
|
||||
status = source_check.check_for_updates(install_root=source.root, home=source.home, force=True)
|
||||
assert "error" not in status, status
|
||||
assert status["targetSha"] == source.commits[2]
|
||||
with pytest.raises(ChannelNotFound):
|
||||
assert reads == []
|
||||
with pytest.raises((ChannelNotFound, urllib.error.HTTPError, TimeoutError)):
|
||||
source_releases.resolve_source_target("canary", ["git"], source.root)
|
||||
assert reads == ["canary"]
|
||||
|
||||
|
||||
def test_passive_check_reports_retirement_without_adopting_it(source, monkeypatch):
|
||||
|
||||
@@ -135,7 +135,7 @@ def test_target_worktree_owns_admission_and_fork_comparison(installation, monkey
|
||||
assert status["behind"] == 3
|
||||
assert status["hermesRoot"] == str(linked)
|
||||
assert check_for_updates(install_root=root, home=home, cache_path=cache)["supported"] is False
|
||||
assert requests == [MAIN_CHANNEL, "/repos/fixture/fork/commits/feature%2Fgui",
|
||||
assert requests == ["/repos/fixture/fork/commits/feature%2Fgui",
|
||||
f"/repos/fixture/fork/compare/{head}...{target}"]
|
||||
assert all(not any(arg in {"fetch", "checkout", "reset", "update-ref", "stash"} for arg in cmd) for cmd in commands)
|
||||
assert git("rev-parse", "HEAD", cwd=linked) == head
|
||||
@@ -158,7 +158,7 @@ def test_counts_are_honest_without_fetch(installation, tip_kind, compare, expect
|
||||
assert status["behind"] == expected
|
||||
assert status["updateAvailable"] is (expected != 0)
|
||||
if tip_kind != "unknown":
|
||||
assert requests == [MAIN_CHANNEL, "/repos/fixture/fork/commits/main"]
|
||||
assert requests == ["/repos/fixture/fork/commits/main"]
|
||||
|
||||
|
||||
def test_cache_force_expiry_and_passive_opt_out(installation, monkeypatch):
|
||||
@@ -177,26 +177,26 @@ def test_cache_force_expiry_and_passive_opt_out(installation, monkeypatch):
|
||||
responses[url] = (503, {})
|
||||
(root / "dirty.txt").write_text("carried work")
|
||||
assert check()["dirty"] is True
|
||||
assert requests.count(url) == requests.count(MAIN_CHANNEL) == 1
|
||||
assert requests.count(url) == 1
|
||||
assert check(force=True)["error"] == "fetch-failed"
|
||||
clock[0] += 3599
|
||||
assert check()["error"] == "fetch-failed"
|
||||
assert requests.count(url) == requests.count(MAIN_CHANNEL) == 2
|
||||
assert requests.count(url) == 2
|
||||
clock[0] += 2
|
||||
responses[url] = (200, head)
|
||||
assert check()["behind"] == 0
|
||||
assert requests.count(url) == requests.count(MAIN_CHANNEL) == 3
|
||||
assert requests.count(url) == 3
|
||||
clock[0] += 86401
|
||||
assert check()["behind"] == 0
|
||||
assert requests.count(url) == requests.count(MAIN_CHANNEL) == 4
|
||||
assert requests.count(url) == 4
|
||||
(home / "config.yaml").write_text("updates: {check: false}")
|
||||
assert check(passive=True)["behind"] is None
|
||||
assert check()["behind"] == 0
|
||||
assert requests.count(url) == requests.count(MAIN_CHANNEL) == 4
|
||||
assert requests.count(url) == 4
|
||||
git("commit", "--allow-empty", "-m", "moved")
|
||||
responses[url] = (200, git("rev-parse", "HEAD"))
|
||||
assert check()["behind"] == 0
|
||||
assert requests.count(url) == requests.count(MAIN_CHANNEL) == 5
|
||||
assert requests.count(url) == 5
|
||||
|
||||
|
||||
def test_explicit_and_current_branch_heal_only_after_confirmed_absence(installation, monkeypatch):
|
||||
@@ -212,7 +212,7 @@ def test_explicit_and_current_branch_heal_only_after_confirmed_absence(installat
|
||||
assert failed["branch"] == "deleted"
|
||||
assert failed["error"] == "fetch-failed"
|
||||
assert git("branch", "--show-current", cwd=linked) == "feature/gui"
|
||||
assert requests == [MAIN_CHANNEL]
|
||||
assert requests == []
|
||||
|
||||
|
||||
@pytest.mark.parametrize("selection", ["explicit", "configured", "current", "detached"])
|
||||
@@ -285,14 +285,14 @@ def test_channel_failure_never_probes_or_heals_a_branch(installation, name, fail
|
||||
assert json.loads(branch_file.read_text())["branch"] == "deleted"
|
||||
|
||||
|
||||
def test_unpublished_main_record_follows_the_branch(installation):
|
||||
"""A 404 for main.json keeps a checkout updating via git: the configured
|
||||
branch is probed exactly as for a published source-branch channel, and the
|
||||
Desktop branch setting is left alone."""
|
||||
def test_main_follows_the_branch_without_reading_a_channel_record(installation):
|
||||
"""main IS the git branch: no R2 record is requested (a regional WAF can answer
|
||||
it 403), the configured branch is probed, and the Desktop branch setting is left alone."""
|
||||
from hermes_cli.source_check import check_for_updates
|
||||
|
||||
_root, linked, home, _base, head, responses, requests, _git = installation
|
||||
responses[MAIN_CHANNEL] = (404, source_channel("main", "fixture/fork"))
|
||||
# Even a record that would answer is never read.
|
||||
responses[MAIN_CHANNEL] = (403, {"message": "blocked by WAF"})
|
||||
branch_path = "/repos/fixture/fork/commits/desktop-choice"
|
||||
responses[branch_path] = (200, head)
|
||||
branch_file = home / "desktop-update.json"
|
||||
@@ -303,7 +303,7 @@ def test_unpublished_main_record_follows_the_branch(installation):
|
||||
assert status.get("branch") == "desktop-choice", status
|
||||
assert status.get("targetSha") == head, status
|
||||
assert status["behind"] == 0
|
||||
assert requests == [MAIN_CHANNEL, branch_path]
|
||||
assert requests == [branch_path]
|
||||
assert json.loads(branch_file.read_text())["branch"] == "desktop-choice"
|
||||
|
||||
|
||||
@@ -379,7 +379,7 @@ def test_never_pushed_branch_keeps_its_pin(installation, pinned_by):
|
||||
assert json.loads(branch_file.read_text()) == {"branch": "local-work"}
|
||||
else:
|
||||
assert not branch_file.exists()
|
||||
assert requests == [MAIN_CHANNEL]
|
||||
assert requests == []
|
||||
|
||||
|
||||
@pytest.mark.parametrize("merge", ["fast-forward", "rebase", "unmerged"])
|
||||
@@ -436,7 +436,7 @@ def test_source_admission_is_stamp_owned_not_path_owned(installation, mechanism)
|
||||
responses["/repos/fixture/fork/commits/feature%2Fgui"] = (200, head)
|
||||
status = check_for_updates(install_root=linked, home=home)
|
||||
assert status["supported"] is (mechanism in ("self", None))
|
||||
assert requests == ([MAIN_CHANNEL, "/repos/fixture/fork/commits/feature%2Fgui"]
|
||||
assert requests == (["/repos/fixture/fork/commits/feature%2Fgui"]
|
||||
if status["supported"] else [])
|
||||
empty = home / "no-source"
|
||||
empty.mkdir()
|
||||
@@ -483,7 +483,7 @@ def test_malformed_optional_changelog_and_cache_do_not_hide_the_update(installat
|
||||
data["status"] = None
|
||||
cache.write_text(json.dumps(data))
|
||||
assert check_for_updates(install_root=root, home=home, cache_path=cache)["behind"] == 2
|
||||
assert requests == [MAIN_CHANNEL, "/repos/fixture/fork/commits/main",
|
||||
assert requests == ["/repos/fixture/fork/commits/main",
|
||||
f"/repos/fixture/fork/compare/{head}...{'a' * 40}"] * 2
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user