fix: improve multi-host error guidance when GITHUB_HOST is set#130
Merged
danielmeppiel merged 3 commits intomainfrom Mar 3, 2026
Merged
fix: improve multi-host error guidance when GITHUB_HOST is set#130danielmeppiel merged 3 commits intomainfrom
danielmeppiel merged 3 commits intomainfrom
Conversation
- Remove stale self.github_host mutations that could leak host state across dependencies (dead code, but hazardous if reached) - Add actionable error message when GITHUB_HOST is set and a clone fails, guiding users to use FQDN syntax for deps on other hosts - Document multi-host behavior in getting-started.md Closes #113
Contributor
There was a problem hiding this comment.
Pull request overview
Improves user guidance for mixed GitHub host scenarios when GITHUB_HOST is set, reduces per-dependency state mutation in the GitHub downloader, and documents how to mix corporate and public GitHub dependencies safely.
Changes:
- Add an actionable clone-failure hint explaining how
GITHUB_HOSTaffects shorthand dependencies and how to use FQDN syntax. - Remove
self.github_host = dep_ref.hostassignments that mutate downloader instance state per dependency. - Update getting-started documentation with a callout and example for multi-host dependency configuration.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/apm_cli/deps/github_downloader.py |
Adds multi-host error guidance on clone failure; removes per-dependency host state mutations. |
docs/getting-started.md |
Documents that GITHUB_HOST applies to bare dependency names and shows how to mix hosts via FQDN. |
Comments suppressed due to low confidence (1)
src/apm_cli/deps/github_downloader.py:303
- New error-message behavior was added for the multi-host scenario (GITHUB_HOST set + dep host matches) but there isn’t a unit test asserting this branch. Please add a test around
_clone_with_fallbackthat forces all clone methods to fail and asserts (1) the guidance appears whenGITHUB_HOSTis set and dep_ref.host matches, and (2) it does not appear when the dependency explicitly targets a different host (e.g., github.com).
configured_host = os.environ.get("GITHUB_HOST", "")
dep_host = dep_ref.host if dep_ref else None
if is_ado and not self.has_ado_token:
error_msg += "For private Azure DevOps repositories, set ADO_APM_PAT environment variable."
elif configured_host and dep_host and dep_host == configured_host and configured_host != "github.com":
foutoucour
approved these changes
Mar 2, 2026
SebastienDegodez
approved these changes
Mar 3, 2026
danielmeppiel
added a commit
that referenced
this pull request
Mar 3, 2026
- Bump version to 0.7.4 - Rewrite Unreleased changelog as v0.7.4 with clean style (no bold labels), consolidated CI/governance entries, Added → Fixed → Changed ordering - Add missing PRs: #97 (hooks), #118 (governance), #119 (gh-aw upgrade), #130 (multi-host) - README: fix APM Packages section — accurate installable types, doc links, fix anthropics/courses → anthropics/skills, remove Add yours row, note hooks in apm.yml example comment
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #113 — when
GITHUB_HOSTis set to a corporate endpoint, shorthand dependencies (without an explicit hostname) resolve against that host. If a user mixes corporate and public GitHub deps without using FQDN, they get a confusing "Repository not found" error with no guidance.Changes
1. Actionable error message for multi-host scenarios
When a clone fails and
GITHUB_HOSTis set, APM now detects whether the failing dependency inherited its host fromGITHUB_HOSTand suggests using FQDN syntax:2. Remove stale host state mutations
Removed two
self.github_host = dep_ref.hostassignments inGitHubDownloaderthat mutate instance state per-dependency. These are dead code (every call path passesdep_refwith a non-nullhost, so_build_repo_urlalways usesdep_ref.hostdirectly), but they represent a state-pollution hazard if future refactoring introduced a code path withoutdep_ref.3. Documentation
Added a callout in
getting-started.mdclarifying thatGITHUB_HOSTaffects all bare package names, with a YAML example showing how to mix hosts.Testing
GITHUB_HOSTbehavior is preserved