From 00db7178ad35be990fe638ef5754f1ffab55f7f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=A8=B1=E5=85=83=E8=B1=AA?= <146086744+edenfunf@users.noreply.github.com> Date: Tue, 21 Apr 2026 19:24:38 +0800 Subject: [PATCH 1/2] fix(install): lowercase host in fallback-port-warned dedup key ``_fallback_port_warned`` keys on ``(dep_host, repo_url_base, dep_port)``. DNS hostnames are case-insensitive per RFC 4343, so two deps that differ only in host casing should collapse to one cross-protocol fallback warning, not two. Normalise the host axis on insertion, matching the ``host.lower() if host else host`` idiom already used by ``AuthResolver._cache`` at ``core/auth.py:224``. The inline NOTE at the construction site was a placeholder for this follow-up and is removed with the fix. Trigger surface is narrower than the issue's manifest-based repro suggests: ``DependencyReference.parse`` already lowercases the host on URL-string inputs, so manifest-driven deps are shielded by the parser. Code paths that build ``DependencyReference`` via the dataclass constructor (bypassing the parser) still leak casing into the dedup key, which is the surface the fix defends. The new regression test builds its deps that way to exercise the fix. Closes #800 --- src/apm_cli/deps/github_downloader.py | 10 +- tests/unit/test_protocol_fallback_warning.py | 114 +++++++++++++++++++ 2 files changed, 119 insertions(+), 5 deletions(-) diff --git a/src/apm_cli/deps/github_downloader.py b/src/apm_cli/deps/github_downloader.py index 65cd05bbc..a50826e17 100644 --- a/src/apm_cli/deps/github_downloader.py +++ b/src/apm_cli/deps/github_downloader.py @@ -768,11 +768,11 @@ def _env_for(use_token_attempt: bool) -> Dict[str, str]: and any(a.scheme == "ssh" for a in plan.attempts) and any(a.scheme == "https" for a in plan.attempts) ): - # NOTE: dedup key is case-sensitive. GitHub/Bitbucket hostnames - # are case-insensitive per RFC, so "Example.com" and - # "example.com" dedup as distinct identities -- worst case is a - # duplicate warning. Follow-up issue tracks normalization. - warn_key = (dep_host, repo_url_base, dep_port) + warn_key = ( + dep_host.lower() if dep_host else dep_host, + repo_url_base, + dep_port, + ) if warn_key not in self._fallback_port_warned: self._fallback_port_warned.add(warn_key) initial_scheme = plan.attempts[0].scheme.upper() diff --git a/tests/unit/test_protocol_fallback_warning.py b/tests/unit/test_protocol_fallback_warning.py index 79102e19f..026fc4a79 100644 --- a/tests/unit/test_protocol_fallback_warning.py +++ b/tests/unit/test_protocol_fallback_warning.py @@ -266,3 +266,117 @@ def _capture(message, symbol=None): ) assert any("7999" in m for m in port_warnings), port_warnings assert any("8443" in m for m in port_warnings), port_warnings + + def test_warning_dedup_normalises_hostname_casing(self): + """Issue #800: DNS hostnames are case-insensitive per RFC 4343, so + two deps that differ only in host casing denote the same identity. + The dedup key must lowercase the host axis to match the + ``AuthResolver._cache`` convention. + + Note: ``DependencyReference.parse`` already lowercases the host on + URL-string inputs, so this test must build the deps via the + dataclass constructor directly to reach the dedup key with casing + preserved -- that is the code path the dedup-key fix defends.""" + dep_lower = DependencyReference( + repo_url="project/repo", + host="bitbucket.example.com", + port=7999, + ) + dep_mixed = DependencyReference( + repo_url="project/repo", + host="Bitbucket.Example.com", + port=7999, + ) + assert dep_mixed.host == "Bitbucket.Example.com", ( + "direct constructor must preserve host casing, else this " + "test cannot reproduce the #800 regression" + ) + dl = _make_downloader() + dl.auth_resolver._cache.clear() + dl._allow_fallback = True + + def _fake_clone(url, *a, **kw): + raise GitCommandError("clone", "failed") + + captured = [] + + def _capture(message, symbol=None): + captured.append((message, symbol)) + + with patch.dict(os.environ, {}, clear=True), patch( + "apm_cli.core.token_manager.GitHubTokenManager.resolve_credential_from_git", + return_value=None, + ), patch( + "apm_cli.deps.github_downloader.Repo" + ) as MockRepo, patch( + "apm_cli.deps.github_downloader._rich_warning", side_effect=_capture + ): + MockRepo.clone_from.side_effect = _fake_clone + for dep in (dep_lower, dep_mixed): + target = Path(tempfile.mkdtemp()) + try: + dl._clone_with_fallback(dep.repo_url, target, dep_ref=dep) + except (RuntimeError, GitCommandError): + pass + finally: + import shutil + shutil.rmtree(target, ignore_errors=True) + + port_warnings = [m for m, _s in captured if "Custom port" in m] + assert len(port_warnings) == 1, ( + f"case-differing hostnames must dedup to one warning, " + f"got {len(port_warnings)}: {port_warnings!r}" + ) + + def test_host_normalisation_does_not_collapse_distinct_repos(self): + """Guard against over-normalisation: even when host casing is + folded, the dedup key must still discriminate on repo path, so + two deps sharing host+port but differing on repo still warn + twice. Built via the direct constructor for the same reason as + ``test_warning_dedup_normalises_hostname_casing``.""" + dep_a = DependencyReference( + repo_url="project/repo-a", + host="bitbucket.example.com", + port=7999, + ) + dep_b = DependencyReference( + repo_url="project/repo-b", + host="Bitbucket.Example.com", + port=7999, + ) + dl = _make_downloader() + dl.auth_resolver._cache.clear() + dl._allow_fallback = True + + def _fake_clone(url, *a, **kw): + raise GitCommandError("clone", "failed") + + captured = [] + + def _capture(message, symbol=None): + captured.append((message, symbol)) + + with patch.dict(os.environ, {}, clear=True), patch( + "apm_cli.core.token_manager.GitHubTokenManager.resolve_credential_from_git", + return_value=None, + ), patch( + "apm_cli.deps.github_downloader.Repo" + ) as MockRepo, patch( + "apm_cli.deps.github_downloader._rich_warning", side_effect=_capture + ): + MockRepo.clone_from.side_effect = _fake_clone + for dep in (dep_a, dep_b): + target = Path(tempfile.mkdtemp()) + try: + dl._clone_with_fallback(dep.repo_url, target, dep_ref=dep) + except (RuntimeError, GitCommandError): + pass + finally: + import shutil + shutil.rmtree(target, ignore_errors=True) + + port_warnings = [m for m, _s in captured if "Custom port" in m] + assert len(port_warnings) == 2, ( + f"same host+port with distinct repos must still warn " + f"separately, got {len(port_warnings)}: {port_warnings!r}" + ) From afcaf53fdacb8ff913496ee4c3d92603ab573b16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=A8=B1=E5=85=83=E8=B1=AA?= <146086744+edenfunf@users.noreply.github.com> Date: Wed, 22 Apr 2026 18:39:32 +0800 Subject: [PATCH 2/2] docs(changelog): add Fixed entry for #800 dedup-key lowercase fix --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index cb058ecfe..767cf636d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Legacy `.github/prompts/` and `.github/chatmodes/` that pre-dated the skill/agent primitive model (#823) +### Fixed + +- Lowercase the host axis of the `_fallback_port_warned` dedup key so deps that differ only in hostname casing collapse to one cross-protocol fallback warning, matching the `AuthResolver._cache` convention (RFC 4343). Closes #800 (#815) + ## [0.9.0] - 2026-04-21 ### Changed (BREAKING)