From de69bfd604e3b3388a3e695ec0b34ea656aa51c4 Mon Sep 17 00:00:00 2001 From: Yuriy Kirillov Date: Thu, 25 Jun 2026 01:15:04 +0200 Subject: [PATCH] feat: remove pinned/always_refresh logic and require explicit ref tags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All GitHub refs now re-fetch on every update run; idempotency is preserved by the existing body-equality check. Tagless refs are rejected at parse time — a tag is always required. Removes: is_pinned, always_refresh, _latest_tag, tagless auto-latest. Co-Authored-By: Claude Sonnet 4.6 --- src/embedder/providers/__init__.py | 2 -- src/embedder/providers/github.py | 30 ++++------------------- src/embedder/providers/local.py | 3 --- src/embedder/updater.py | 4 +--- tests/test_cli.py | 2 +- tests/test_refs.py | 15 ++++++++---- tests/test_updater.py | 38 +++++++----------------------- 7 files changed, 24 insertions(+), 70 deletions(-) diff --git a/src/embedder/providers/__init__.py b/src/embedder/providers/__init__.py index 87266d4..5bb164f 100644 --- a/src/embedder/providers/__init__.py +++ b/src/embedder/providers/__init__.py @@ -18,8 +18,6 @@ def parse_ref(self, raw: str) -> AnyRef: ... def resolve(self, ref: AnyRef) -> AnyRef: ... - def always_refresh(self, ref: AnyRef) -> bool: ... - def fetch(self, ref: AnyRef, base_dir: Path) -> str: ... diff --git a/src/embedder/providers/github.py b/src/embedder/providers/github.py index 26f53a5..33d88a1 100644 --- a/src/embedder/providers/github.py +++ b/src/embedder/providers/github.py @@ -14,34 +14,26 @@ r"^github\.com/" r"(?P[A-Za-z0-9_.-]+)/" r"(?P[A-Za-z0-9_.-]+)" - r"(?:@(?P[^:\s]+))?" + r"@(?P[^:\s]+)" r":(?P[^\s]+)$" ) -_SEMVER_RE = re.compile(r"^v?\d+(\.\d+)*$") - @dataclass(frozen=True) class GitHubAssetRef: owner: str repo: str asset: str - tag: str | None = None + tag: str @property def repository(self) -> str: return f"{self.owner}/{self.repo}" - @property - def is_pinned(self) -> bool: - return self.tag is not None and bool(_SEMVER_RE.match(self.tag)) - def with_tag(self, tag: str) -> GitHubAssetRef: return GitHubAssetRef(owner=self.owner, repo=self.repo, asset=self.asset, tag=tag) def render(self) -> str: - if self.tag is None: - return f"github.com/{self.repository}:{self.asset}" return f"github.com/{self.repository}@{self.tag}:{self.asset}" @@ -59,7 +51,7 @@ def parse_github_ref(raw: str) -> GitHubAssetRef: owner=match["owner"], repo=match["repo"], asset=asset, - tag=match.group("tag"), + tag=match["tag"], ) @@ -78,14 +70,9 @@ def parse_ref(self, raw: str) -> GitHubAssetRef: return parse_github_ref(raw) def resolve(self, ref: GitHubAssetRef) -> GitHubAssetRef: - # Validate gh is available for any GitHub ref, then return as-is. - # Tagless and branch refs are handled via always_refresh + fetch. self.require() return ref - def always_refresh(self, ref: GitHubAssetRef) -> bool: - return not ref.is_pinned - def fetch(self, ref: GitHubAssetRef, base_dir: Path) -> str: return self._fetch_file(ref) @@ -116,18 +103,9 @@ def run(self, args: list[str], *, check: bool = True) -> CommandResult: def auth_ok(self) -> bool: return self.run(["auth", "status"], check=False).returncode == 0 - def _latest_tag(self, ref: GitHubAssetRef) -> str: - result = self.run( - ["api", f"repos/{ref.repository}/releases/latest", "--jq", ".tag_name"] - ) - if not result.stdout or result.stdout == "null": - raise EmbedderError(f"Could not resolve latest release for {ref.repository}") - return result.stdout - def _fetch_file(self, ref: GitHubAssetRef) -> str: - tag = ref.tag if ref.tag is not None else self._latest_tag(ref) encoded_asset = quote(ref.asset, safe="/") - encoded_tag = quote(tag, safe="") + encoded_tag = quote(ref.tag, safe="") result = self.run( [ "api", diff --git a/src/embedder/providers/local.py b/src/embedder/providers/local.py index 03ec9aa..59dc830 100644 --- a/src/embedder/providers/local.py +++ b/src/embedder/providers/local.py @@ -27,9 +27,6 @@ def parse_ref(self, raw: str) -> LocalRef: def resolve(self, ref: LocalRef) -> LocalRef: return ref - def always_refresh(self, ref: LocalRef) -> bool: - return True - def fetch(self, ref: LocalRef, base_dir: Path) -> str: resolved_base = base_dir.resolve() target = (base_dir / ref.path).resolve() diff --git a/src/embedder/updater.py b/src/embedder/updater.py index dc577a2..894348f 100644 --- a/src/embedder/updater.py +++ b/src/embedder/updater.py @@ -75,10 +75,8 @@ def update_files( provider = get_provider(check.block.ref.render(), _providers) if local_only and not isinstance(provider, LocalProvider): continue - if not check.update_available and not provider.always_refresh(check.block.ref): - continue new_body = provider.fetch(check.latest_ref, _base_dir) - if new_body == check.block.body and not check.update_available: + if new_body == check.block.body: continue updates.append( BlockUpdate(block=check.block, new_ref=check.block.ref, new_body=new_body) diff --git a/tests/test_cli.py b/tests/test_cli.py index 56d5aaa..4d6c941 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -56,7 +56,7 @@ def test_check_missing_gh_returns_environment_error( target.write_text( "\n".join( [ - marker("github.com/rubykatzen/embedder:fragment.md"), + marker("github.com/rubykatzen/embedder@v0.1.0:fragment.md"), "managed", close_marker(), "", diff --git a/tests/test_refs.py b/tests/test_refs.py index 2da52e2..d27da20 100644 --- a/tests/test_refs.py +++ b/tests/test_refs.py @@ -28,11 +28,9 @@ def test_reject_invalid_github_ref() -> None: parse_github_ref("rubykatzen/embedder@v0.1.0:fragment.md") -def test_parse_github_ref_without_tag() -> None: - ref = parse_github_ref("github.com/OWNER/repo-name:file.md") - - assert ref.tag is None - assert ref.render() == "github.com/OWNER/repo-name:file.md" +def test_reject_tagless_github_ref() -> None: + with pytest.raises(RefError): + parse_github_ref("github.com/OWNER/repo-name:file.md") def test_parse_github_ref_branch() -> None: @@ -43,6 +41,13 @@ def test_parse_github_ref_branch() -> None: assert ref.render() == "github.com/rubykatzen/embedder@main:docs/fragment.md" +def test_parse_github_ref_floating_tag() -> None: + ref = parse_github_ref("github.com/rubykatzen/embedder@v0.2:fragments/file.md") + + assert ref.tag == "v0.2" + assert ref.render() == "github.com/rubykatzen/embedder@v0.2:fragments/file.md" + + def test_asset_path_with_subdirectory() -> None: ref = parse_github_ref("github.com/rubykatzen/embedder@v0.1.0:docs/fragments/file.md") diff --git a/tests/test_updater.py b/tests/test_updater.py index 3131f53..ca7795f 100644 --- a/tests/test_updater.py +++ b/tests/test_updater.py @@ -30,9 +30,6 @@ def resolve(self, ref: GitHubAssetRef) -> GitHubAssetRef: self.resolve_calls += 1 return ref - def always_refresh(self, ref: GitHubAssetRef) -> bool: - return not ref.is_pinned - def fetch(self, ref: GitHubAssetRef, base_dir: Path) -> str: return self.contents[(ref.repository, ref.asset)] @@ -41,32 +38,13 @@ def fake_providers() -> list[Provider]: return [FakeGitHubProvider(), LocalProvider()] -def test_check_blocks_tagless_ref_not_update_pending() -> None: - """Auto-latest refs are always-refresh; check() doesn't report them as pending updates.""" - text = "\n".join( - [ - marker("github.com/rubykatzen/embedder:fragment.md"), - "old", - close_marker(), - "", - ] - ) - blocks = parse_blocks(Path("AGENTS.md"), text) - - results = check_blocks(blocks, fake_providers()) - - assert len(results) == 1 - assert not results[0].update_available - assert results[0].latest_ref.render() == "github.com/rubykatzen/embedder:fragment.md" - - def test_update_files_replaces_only_managed_body(tmp_path: Path) -> None: target = tmp_path / "AGENTS.md" target.write_text( "\n".join( [ "before", - marker("github.com/rubykatzen/embedder:fragment.md"), + marker("github.com/rubykatzen/embedder@v0.1.0:fragment.md"), "old managed text", close_marker(), "after", @@ -82,7 +60,7 @@ def test_update_files_replaces_only_managed_body(tmp_path: Path) -> None: assert target.read_text(encoding="utf-8") == "\n".join( [ "before", - marker("github.com/rubykatzen/embedder:fragment.md"), + marker("github.com/rubykatzen/embedder@v0.1.0:fragment.md"), "new managed text", close_marker(), "after", @@ -125,10 +103,10 @@ def test_check_blocks_calls_resolve_per_block() -> None: """resolve() is called once per block (no caching); it's a no-op for all ref types.""" text = "\n".join( [ - marker("github.com/rubykatzen/embedder:first.md"), + marker("github.com/rubykatzen/embedder@v0.1.0:first.md"), "old", close_marker(), - marker("github.com/rubykatzen/embedder:second.md"), + marker("github.com/rubykatzen/embedder@v0.1.0:second.md"), "old", close_marker(), "", @@ -179,10 +157,10 @@ def test_cache_uses_correct_asset_per_block() -> None: """Two blocks from the same repo must not share each other's asset.""" text = "\n".join( [ - marker("github.com/rubykatzen/embedder:first.md"), + marker("github.com/rubykatzen/embedder@v0.1.0:first.md"), "old", close_marker(), - marker("github.com/rubykatzen/embedder:second.md"), + marker("github.com/rubykatzen/embedder@v0.1.0:second.md"), "old", close_marker(), "", @@ -193,8 +171,8 @@ def test_cache_uses_correct_asset_per_block() -> None: results = check_blocks(blocks, registry) - assert results[0].latest_ref.render() == "github.com/rubykatzen/embedder:first.md" - assert results[1].latest_ref.render() == "github.com/rubykatzen/embedder:second.md" + assert results[0].latest_ref.render() == "github.com/rubykatzen/embedder@v0.1.0:first.md" + assert results[1].latest_ref.render() == "github.com/rubykatzen/embedder@v0.1.0:second.md" def test_local_ref_body_refreshed_on_update(tmp_path: Path) -> None: