Skip to content

fix(install): support FQDN monorepo subpath on GHES hosts (closes #1673)#1684

Open
sergio-sisternes-epam wants to merge 2 commits into
mainfrom
sergio-sisternes-epam/fix-ghes-fqdn-monorepo-subpath
Open

fix(install): support FQDN monorepo subpath on GHES hosts (closes #1673)#1684
sergio-sisternes-epam wants to merge 2 commits into
mainfrom
sergio-sisternes-epam/fix-ghes-fqdn-monorepo-subpath

Conversation

@sergio-sisternes-epam
Copy link
Copy Markdown
Collaborator

TL;DR

Extend is_github_hostname() to recognise custom GitHub Enterprise Server hosts configured via GITHUB_HOST, so FQDN shorthand with monorepo subpaths (e.g. ghe.example.com/org/repo/packages/skill) correctly splits into git: org/repo + path: packages/skill instead of embedding the subpath into the git URL.

Problem

When installing a monorepo subpath package from a GHES host using FQDN shorthand:

apm install ghe.example.com/org/repo/packages/skill

The parser treats ALL path segments as the git repository URL, producing an invalid clone URL (https://ghe.example.com/org/repo.git/packages/skill/). This works on github.com because is_github_hostname() returns True for it, triggering a 2-segment owner/repo split. Custom GHES hosts fall through to the generic-host branch which treats all segments as repo path.

Root cause

Classification gap between two host-detection paths:

  • is_github_hostname() (parse-time) -- only matches github.com and *.ghe.com
  • AuthResolver.classify_host() (install-time) -- also checks GITHUB_HOST env var and correctly returns kind="ghes"

This means _detect_virtual_package() sets min_base_segments = len(path_segments) for GHES hosts, and _resolve_shorthand_to_parsed_url() includes all segments in user_repo.

Fix

Add a GITHUB_HOST env var check to is_github_hostname(), mirroring the GHES detection in classify_host(). Guards against false positives:

  • ADO hosts (dev.azure.com, *.visualstudio.com) excluded
  • gitlab.com excluded
  • *.ghe.com excluded (already matched by earlier check)
  • Invalid FQDNs rejected via is_valid_fqdn()
  • Only activates when GITHUB_HOST value matches the queried hostname

Before (with GITHUB_HOST=ghe.example.com):

ghe.example.com/org/repo/packages/skill
  -> repo_url='org/repo/packages/skill', virtual_path=None  [broken]

After:

ghe.example.com/org/repo/packages/skill
  -> repo_url='org/repo', virtual_path='packages/skill'  [correct]

Test evidence

  • 17 new tests across 2 files:
    • 10 tests for is_github_hostname() GHES classification (positive + negative guards)
    • 7 tests for end-to-end GHES FQDN subpath parsing
  • 16,647 existing tests pass -- zero regressions
  • All lint checks green (ruff, pylint R0801, auth-signals)

Files changed

File Change
src/apm_cli/utils/github_host.py Add GITHUB_HOST env var check to is_github_hostname()
tests/unit/test_github_host.py Add TestIsGitHubHostnameGHES class (10 tests)
tests/unit/test_generic_git_urls.py Add TestGHESFQDNSubpathParsing class (7 tests)

Out of scope

Generic boundary probe for non-GHES hosts (Gitea, Bitbucket, etc.) where GITHUB_HOST is not set. These hosts still require the dict format ({git: ..., path: ...}). A probe-based solution for truly generic hosts is a separate design effort.

Fixes: #1673

#1673)

Extend is_github_hostname() to check the GITHUB_HOST env var, mirroring
the GHES detection already present in AuthResolver.classify_host(). This
closes the classification gap that caused _detect_virtual_package() and
_resolve_shorthand_to_parsed_url() to treat configured GHES hosts as
generic hosts -- embedding subpaths into the git URL instead of splitting
into git: + path:.

Before: ghe.example.com/org/repo/packages/skill
  -> repo_url='org/repo/packages/skill', virtual_path=None (broken)

After (with GITHUB_HOST=ghe.example.com):
  -> repo_url='org/repo', virtual_path='packages/skill' (correct)

Guards against false positives:
- ADO hosts (dev.azure.com, *.visualstudio.com) excluded
- gitlab.com excluded
- Invalid FQDNs rejected
- Only activates when GITHUB_HOST matches the hostname

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 6, 2026 22:55
@sergio-sisternes-epam sergio-sisternes-epam added the panel-review Trigger the apm-review-panel gh-aw workflow label Jun 6, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes GHES monorepo-subpath shorthand parsing by extending is_github_hostname() to recognize a custom GitHub Enterprise Server host configured via GITHUB_HOST, so inputs like ghe.example.com/org/repo/packages/skill split into repo_url=org/repo and virtual_path=packages/skill rather than embedding the subpath into the clone URL.

Changes:

  • Extend is_github_hostname() to treat the GITHUB_HOST value as a GitHub-family hostname (with guards for ADO/GitLab/FQDN validity).
  • Add unit tests for GHES hostname classification via GITHUB_HOST.
  • Add end-to-end parsing tests for GHES FQDN shorthand with virtual subpaths (including #ref).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/apm_cli/utils/github_host.py Adds GITHUB_HOST-aware GHES detection to is_github_hostname() to enable owner/repo vs subpath splitting for GHES FQDN shorthand.
tests/unit/test_github_host.py Adds focused tests validating GHES hostname classification behavior via GITHUB_HOST (and negative guards).
tests/unit/test_generic_git_urls.py Adds parsing tests verifying GHES FQDN shorthand correctly yields repo_url + virtual_path when GITHUB_HOST matches.

Comment thread src/apm_cli/utils/github_host.py Outdated
Drop .strip().split('/')[0] from the GITHUB_HOST env var check in
is_github_hostname() and use .lower() only, matching the normalization
in AuthResolver.classify_host(). This prevents a misconfigured value
like GITHUB_HOST=ghe.example.com/extra from being accepted at parse
time but rejected at install time.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 6, 2026

APM Review Panel: ship_with_followups

GHES FQDN monorepo subpath install now splits correctly via GITHUB_HOST; ship after adding CHANGELOG entry and aligning classify_host() normalization to prevent silent auth misroute.

cc @sergio-sisternes-epam @danielmeppiel -- a fresh advisory pass is ready for your review.

This PR fixes a real enterprise production pain point: FQDN shorthand strings like ghe.example.com/org/repo/packages/skill were silently embedding the full path in the clone URL instead of splitting at the repo boundary. The fix is narrow, correct, and well-tested at the unit tier -- 17 new tests added, 16,647 existing tests passing. The contributor is an EPAM engineer reporting a production regression, which is the strongest adoption signal the project can receive. The PR ships.

Two cross-cutting themes require pre-merge action. First, the classify_host() normalization divergence, flagged independently by supply-chain-security, auth-expert, and cli-logging: is_github_hostname() now applies .strip().lower().split('/')[0] to GITHUB_HOST, but classify_host() in auth.py applies only .lower(). For a GITHUB_HOST value with accidental whitespace or an embedded path component, URL parsing classifies the host as GHES while token routing classifies it as generic -- silent auth failure after a fix that appeared to work. This is the highest-signal pre-merge ask because it creates a narrow regression window on the auth path and contradicts the PR body claim that the fix mirrors existing GHES detection in classify_host(). The repair is a one-line change. Second, the CHANGELOG and docs gap, flagged by oss-growth-hacker, doc-writer, and devx-ux: the fix is invisible at upgrade time without a CHANGELOG entry, and the GITHUB_HOST requirement for FQDN subpath parsing is entirely undocumented, leaving future GHES users with no recovery path when they encounter the same failure mode.

The test-coverage-expert surfaces a missing integration test (outcome: missing, tier: integration-with-fixtures) for the full install pipeline through APMPackage.from_apm_yml(). The 17 unit tests provide strong parse-boundary coverage; the integration gap parallels gaps in other install-path surfaces and is the right first follow-up issue after merge. The python-architect observation that GHES env detection is now duplicated five times with divergent guard sets is the correct cleanup target for a subsequent refactor PR.

Dissent. Supply-chain-security and auth-expert independently flagged the classify_host() normalization gap from slightly different angles -- supply-chain focused on whitespace stripping, auth-expert on path-component stripping -- but their fix recommendations are compatible and point at the same one-line change. I side with aligning classify_host() to the full .strip().lower().split('/')[0] normalization rather than removing normalization from is_github_hostname(), which would weaken the fix for the common case of a cleanly formatted GITHUB_HOST value.

Aligned with: Portable by manifest (FQDN shorthand in apm.yml correctly resolves for GHES monorepo subpaths, restoring manifest portability for enterprise teams with monorepo package layouts); Secure by default (ADO guard in is_github_hostname() prevents ADO host misclassification; the pre-merge classify_host() normalization fix closes the auth-misroute window for malformed GITHUB_HOST values); Multi-harness/multi-host (GHES is now a first-class host for FQDN dependency resolution on par with github.com and .ghe.com hosts); OSS community driven (external enterprise contributor filed and fixed a production regression -- shipping promptly with changelog credit is the correct community-first response).

Growth signal. The EPAM origin of this fix is the story. A major global IT services integrator filed and fixed a production pain point in the GHES enterprise segment -- evidence that APM is being evaluated in real enterprise environments and found worth contributing to. Release post angle: "APM now speaks fluent GHES monorepo -- any FQDN subpath, single shorthand, no workarounds." Pair the CHANGELOG entry with a social push crediting the contributor by name to maximize the conversion signal for GHES enterprise evaluators watching whether APM is production-ready for their environment.

Panel summary

Persona B R N Takeaway
Python Architect 0 1 1 Correct targeted fix; GHES env detection now duplicated 5 times with divergent guard sets -- extract _is_ghes_configured_host() helper as follow-up.
CLI Logging Expert 0 2 1 No user-facing output paths changed; two silent failure modes exposed by the new GITHUB_HOST lookup deserve diagnostic coverage.
DevX UX Expert 0 2 1 Fix works for GITHUB_HOST-aware users; silent misbehavior and missing docs leave unaware GHES users with no recovery path.
Supply Chain Security 0 2 2 PR is sound in scope; two cross-cutting normalization inconsistencies with classify_host() should be fixed before merge to prevent silent auth fallback on misconfigured GITHUB_HOST values.
OSS Growth Hacker 0 1 2 Solid GHES enterprise fix missing CHANGELOG entry; unlocks a high-value enterprise segment but the story stays invisible at upgrade time without a changelog line.
Auth Expert 0 2 2 Parse-time fix is correct for happy path; classify_host() diverges on strip/split normalization, creating a narrow misroute for malformed GITHUB_HOST values.
Doc Writer 0 3 1 CHANGELOG missing fix entry; GITHUB_HOST env-var doc and FQDN shorthand dep guide both omit new virtual-path-split behavior for GHES; docstring references are accurate.
Test Coverage Expert 0 1 1 17 unit-tier tests cover parse path well; install-pipeline integration floor not met; GITHUB_HOST scheme-prefix guard gap untested.

B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.

Top 5 follow-ups

  1. [Auth Expert] Align classify_host() GITHUB_HOST normalization to .strip().lower().split('/')[0] to match is_github_hostname() -- Flagged independently by supply-chain-security, auth-expert, and cli-logging: whitespace or path-component in GITHUB_HOST causes URL parsing to succeed as GHES while token routing fails as generic, producing silent auth breakage after a fix that appears to work. One-line change in auth.py:262. Highest-signal pre-merge ask.

  2. [OSS Growth Hacker] Add CHANGELOG entry under [Unreleased] ### Fixed crediting the contributor and closing apm install: FQDN monorepo subpath on GHES fails; hoping for CLI-only install without manual apm.yml edit #1673 -- Without a CHANGELOG entry the fix is invisible to enterprise GHES users at upgrade time -- the exact audience that would convert on this fix. Zero-effort, high-visibility pre-merge ask.

  3. [Test Coverage Expert] Add integration test for apm install GHES FQDN subpath through APMPackage.from_apm_yml() (outcome: missing, tier: integration-with-fixtures) -- No test pins the end-to-end contract dep.repo_url == 'org/repo' and dep.virtual_path == 'packages/skill' through the full install path. Future refactors can silently regress the fix with no automated catch.

  4. [DevX UX Expert] Emit an actionable error naming GITHUB_HOST when a GHES FQDN subpath install falls into the generic-host branch -- Without GITHUB_HOST set, the user receives a cryptic git clone failure with no mention of the required env var. APM already has scaffolding for env-var hints via unsupported_host_error().

  5. [Python Architect] Extract a module-private _is_ghes_configured_host() helper in utils/github_host.py to unify five diverging GHES env-detection copies -- The ADO guard added in this PR is correct but creates a detectable inconsistency across five call sites with divergent guard sets. The next change to GHES detection will again diverge without a single source of truth.

Architecture

classDiagram
    direction LR

    class GithubHostUtils {
        <<IOBoundary>>
        +is_github_hostname(hostname) bool
        +is_azure_devops_hostname(hostname) bool
        +is_gitlab_hostname(hostname) bool
        +is_valid_fqdn(hostname) bool
        +has_github_gitlab_host_env_conflict(hostname) bool
    }
    note for GithubHostUtils "utils/github_host.py -- GHES env check added to is_github_hostname (PR, has ADO guard). Pre-existing sibling copies in is_gitlab_hostname and has_github_gitlab_host_env_conflict lack the ADO guard. AuthResolver.classify_host and DownloadDelegate._is_configured_ghes are further diverging copies."

    class DependencyReference {
        <<ValueObject>>
        +host str
        +repo_url str
        +virtual_path str
        +is_virtual bool
        +parse(dep_str) DependencyReference
        +_detect_virtual_package(dep_str)
        +_resolve_virtual_shorthand_repo(repo_url, host, virtual_path)
    }

    class AuthResolver {
        <<Strategy>>
        +classify_host(host, port) HostInfo
    }

    class DownloadDelegate {
        +_is_configured_ghes(host) bool
    }

    DependencyReference ..> GithubHostUtils : is_github_hostname, is_azure_devops_hostname
    AuthResolver ..> GithubHostUtils : is_azure_devops_hostname, is_valid_fqdn
    DownloadDelegate ..> GithubHostUtils : is_github_hostname

    class GithubHostUtils:::touched
    classDef touched fill:#fff3b0,stroke:#d47600
Loading
flowchart TD
    A["apm install ghe.example.com/org/repo/packages/skill"] --> B
    B["DependencyReference.parse(dep_str)"] --> C
    C["_detect_virtual_package\nextracts hostname=ghe.example.com"] --> D
    D{"is_github_hostname(ghe.example.com)"}
    D -- "h==github.com? No\nh.endswith(.ghe.com)? No" --> E
    E["os.environ.get(GITHUB_HOST)"] --> F
    F{"ghes_host==h AND\nnot is_azure_devops_hostname AND\nis_valid_fqdn?"}
    F -- "env unset or mismatch: False\npre-PR behavior" --> G
    F -- "env matches + guards pass: True\npost-PR fix" --> H
    G["is_generic_host=True\nmin_base_segments=4"] --> J
    J["is_virtual=False\nBUG: repo_url embeds packages/skill"]
    H["is_generic_host=False\nmin_base_segments=2"] --> L
    L["is_virtual=True\nFIX: repo_url=org/repo, virtual_path=packages/skill"]
Loading

Recommendation

Approve and merge after the author completes two pre-merge asks: (1) add a CHANGELOG entry under [Unreleased] ### Fixed crediting @sergio-sisternes-epam and closing #1673, and (2) apply the one-line normalization fix in classify_host() at auth.py:262 (.strip().lower().split('/')[0]) to prevent the parse/auth misroute on malformed GITHUB_HOST values. All other findings -- integration test floor, actionable error UX, env-detection deduplication, docs expansion -- are tracked as follow-up issues rather than merge blockers. The unit-tier test coverage is strong, the fix is narrow and correct, and the contributor is an enterprise adopter whose PR should close promptly: a contributor kept waiting is a worse outcome than a follow-up issue left open.


Full per-persona findings

Python Architect

  • [recommended] GHES env detection pattern is duplicated 5 times with divergent guard sets at src/apm_cli/utils/github_host.py:190
    The pattern os.environ.get("GITHUB_HOST", "").strip().lower().split("/")[0] now appears in: (1) is_github_hostname() [PR, has ADO guard], (2) is_gitlab_hostname() [no ADO guard], (3) has_github_gitlab_host_env_conflict() [no ADO guard], (4) AuthResolver.classify_host() [no ADO guard, no .strip()/.split()], (5) DownloadDelegate._is_configured_ghes() [no guards at all]. The ADO guard added in this PR is correct but creates a detectable inconsistency. Future GHES detection changes need coordinated edits in 5 places.
    Suggested: Extract a module-private helper _is_ghes_configured_host(hostname: str) -> bool encapsulating the guard set. Call it from is_github_hostname(), and use the negation in is_gitlab_hostname() and has_github_gitlab_host_env_conflict() to also gain the ADO guard.

  • [nit] is_github_hostname() does not strip path segments from incoming hostname; is_gitlab_hostname() does at src/apm_cli/utils/github_host.py:184
    is_gitlab_hostname() normalises the incoming hostname with h = hostname.strip().lower().split("/")[0] but is_github_hostname() uses h = hostname.lower() without stripping. Quiet sibling inconsistency.
    Suggested: Change h = hostname.lower() to h = hostname.strip().lower().split("/")[0].

CLI Logging Expert

  • [recommended] Silent fallback when GITHUB_HOST fails is_valid_fqdn() with no user diagnostic at src/apm_cli/utils/github_host.py:190
    is_github_hostname() now checks GITHUB_HOST but returns False silently when the value fails is_valid_fqdn(). If a user sets GITHUB_HOST to a bare label (e.g. localhost) the FQDN guard rejects it, the host is treated as generic, and the user gets a cryptic git clone error with no indication that GITHUB_HOST was seen but rejected.
    Suggested: When ghes_host is non-empty but is_valid_fqdn(ghes_host) is False, emit a _rich_warning: '[!] GITHUB_HOST value is not a valid FQDN -- custom GHES host not recognised. Check your GITHUB_HOST setting.'

  • [recommended] _is_configured_ghes does not strip path from GITHUB_HOST, creating parse/auth inconsistency at src/apm_cli/deps/download_strategies.py:945
    The new is_github_hostname() applies .split('/')[0] to strip path segments from GITHUB_HOST. But _is_configured_ghes in download_strategies.py does not. If GITHUB_HOST=ghe.example.com/api/v3, is_github_hostname returns True (parse succeeds, subpath splits correctly) but _is_configured_ghes returns False (auth token not forwarded on download). Before this PR both returned False -- consistent. After this PR they diverge.
    Suggested: Apply the same .split('/')[0].strip() normalization to _is_configured_ghes.

  • [nit] No --verbose trace at the GHES recognition branch in _detect_virtual_package at src/apm_cli/models/dependency/reference.py:1192
    When is_github_hostname(validated_host) returns True because GITHUB_HOST matched, there is no verbose-mode log of that decision.
    Suggested: Add a --verbose log entry after is_github_hostname() returns True in _detect_virtual_package.

DevX UX Expert

  • [recommended] No actionable error emitted when GITHUB_HOST is missing for a GHES FQDN subpath install at src/apm_cli/models/dependency/reference.py:1227
    A user who types apm install ghe.example.com/org/repo/packages/skill without GITHUB_HOST set falls into the generic-host branch where all segments are embedded in the clone URL. The downstream git failure says "repository not found" with no mention of GITHUB_HOST. APM already has scaffolding for actionable env-var hints via unsupported_host_error().
    Suggested: In the is_generic_host branch, when len(path_segments) >= 4 and the host is a valid unclassified FQDN, raise a ValueError: 'FQDN shorthand with a monorepo subpath on a GHES host requires GITHUB_HOST. Set GITHUB_HOST=<host> so APM can split the repo boundary from the package subpath, then retry.'

  • [recommended] Docs not updated: FQDN subpath-splitting dependency on GITHUB_HOST is undocumented at docs/src/content/docs/reference/cli/install.md:20
    docs/src/content/docs/reference/cli/install.md describes FQDN shorthand as host/owner/repo with no mention that monorepo subpaths on GHES silently require GITHUB_HOST to split correctly.
    Suggested: Append a sentence to the PACKAGE_REF description covering GHES FQDN subpath with GITHUB_HOST requirement. Also add a parallel note to the GITHUB_HOST row in environment-variables.md.

  • [nit] GHES-precedence guard in is_gitlab_hostname missing is_azure_devops_hostname check at src/apm_cli/utils/github_host.py:69
    Asymmetry between is_github_hostname() (has ADO guard) and is_gitlab_hostname() (does not). Maintenance trap for future readers comparing the two functions.
    Suggested: Add and not is_azure_devops_hostname(ghes_host) in is_gitlab_hostname() GHES-precedence block.

Supply Chain Security Expert

  • [recommended] GITHUB_HOST whitespace not stripped in classify_host(), causing parse/auth split on GITHUB_HOST with leading or trailing spaces at src/apm_cli/core/auth.py:262
    is_github_hostname() applies .strip() before matching GITHUB_HOST. classify_host() reads os.environ.get('GITHUB_HOST', '').lower() with no strip. For GITHUB_HOST with accidental surrounding whitespace, is_github_hostname returns True but classify_host returns kind='generic'. Result: correct URL parsing, silent auth failure with no diagnostic.
    Suggested: Change to ghes_host = os.environ.get('GITHUB_HOST', '').strip().lower() at auth.py:262.

  • [recommended] Path-strip divergence between is_github_hostname() and classify_host() creates silent GHES parse/auth mismatch when GITHUB_HOST contains a path component at src/apm_cli/utils/github_host.py:190
    is_github_hostname() strips path components via .split('/')[0]. classify_host() does not. With GITHUB_HOST=ghe.corp.com/api/v3: parse sees GHES (correct), auth sees generic (wrong). Silent functional auth breakage with no error.
    Suggested: Remove .split('/')[0] from is_github_hostname() so both functions consistently reject path-component GITHUB_HOST values, OR add the same normalization to classify_host().

  • [nit] _supports_gh_cli_host() in token_manager.py now has unreachable code for GITHUB_HOST hosts after is_github_hostname() returns True early at src/apm_cli/core/token_manager.py:156
    Harmless dead code. Track as cleanup candidate in a follow-up.

  • [nit] Exclusion denylist incomplete: non-GitHub FQDN git hosts accidentally set as GITHUB_HOST receive GitHub tokens at src/apm_cli/utils/github_host.py:168
    Pre-existing design consequence, not introduced here. Docstring should warn explicitly.
    Suggested: Add WARNING block in docstring: GITHUB_HOST must be set only for actual GHES instances; setting it to any other git host will cause APM to route GitHub PATs to that host.

OSS Growth Hacker

Auth Expert

  • [recommended] classify_host() and is_github_hostname() normalize GITHUB_HOST differently; diverge on whitespace or embedded-path values at src/apm_cli/core/auth.py:262
    classify_host() at auth.py:262 does .lower() only. The new is_github_hostname() does .strip().lower().split('/')[0]. For GITHUB_HOST with leading/trailing whitespace or embedded path, is_github_hostname returns True while classify_host returns kind='generic'. Token routing goes entirely through classify_host(), so correct URL parsing and wrong auth is the result. Contradicts the PR body claim that this mirrors existing GHES detection in classify_host().
    Suggested: Apply the same normalization in classify_host(): ghes_host = os.environ.get('GITHUB_HOST','').strip().lower().split('/')[0]

  • [recommended] No test asserts is_github_hostname() and classify_host() agree for the same GHES host at tests/unit/test_github_host.py:484
    The fix's value depends on parse-time and auth-time routing reaching the same conclusion. No test pins this cross-module invariant. Future drift between the two paths would go undetected.
    Suggested: Add a test importing AuthResolver and asserting classify_host() returns kind='ghes' for the same GITHUB_HOST configuration.

  • [nit] DownloadDelegate._is_configured_ghes() is now largely superseded by is_github_hostname() but the OR remains at src/apm_cli/deps/download_strategies.py:725
    The OR on line 725 can still return True for an ADO host when _is_configured_ghes fires (weaker guards). Track as cleanup candidate after this PR lands.

  • [nit] GITHUB_HOST with a port (e.g. ghe.example.com:8080) silently fails the equality check at src/apm_cli/utils/github_host.py:190
    .split('/')[0] strips path separators but not colons. No test documents this known limitation.
    Suggested: Add a test asserting is_github_hostname('ghe.example.com') is False when GITHUB_HOST='ghe.example.com:8080'.

Doc Writer

  • [recommended] No CHANGELOG entry for the GHES FQDN monorepo subpath fix (closes apm install: FQDN monorepo subpath on GHES fails; hoping for CLI-only install without manual apm.yml edit #1673) at CHANGELOG.md
    The [Unreleased] Fixed section has two entries but nothing for apm install: FQDN monorepo subpath on GHES fails; hoping for CLI-only install without manual apm.yml edit #1673. This is a user-visible behavior change that GHES operators will look for in the changelog when assessing whether to upgrade.
    Suggested: Add under [Unreleased] ### Fixed: `apm install` now correctly splits FQDN shorthand with monorepo subpaths (e.g. `ghe.example.com/org/repo/packages/skill`) into `git: org/repo` + `path: packages/skill` for GHES hosts configured via `GITHUB_HOST`. Previously the subpath was embedded in the git URL, causing install failures. (by @sergio-sisternes-epam, closes #1673)

  • [recommended] GITHUB_HOST row in environment-variables.md does not mention FQDN dependency parsing at docs/src/content/docs/reference/environment-variables.md
    After this fix, GITHUB_HOST also gates whether FQDN shorthand dependency strings split into repo_url + virtual_path. A GHES admin who reads only the env-var reference will not discover this requirement.
    Suggested: Extend the Notes cell: "Process-wide; affects clone URLs, host detection, and FQDN shorthand dependency parsing (enables host/org/repo/virtual-path to split correctly into repo + virtual path for monorepo subpaths)."

  • [recommended] dependencies.md FQDN shorthand block omits GHES virtual-path behavior and may mislead with non-GitHub hosts comment at packages/apm-guide/.apm/skills/apm-usage/dependencies.md
    The comment # FQDN shorthand (non-GitHub hosts keep the domain) implies GHES (a GitHub host) cannot use FQDN form, which is now incorrect after this fix.
    Suggested: Add a GHES sub-block with examples (requires GITHUB_HOST set) and revise the comment to distinguish GHES from truly generic hosts.

  • [nit] GHES section in authentication.md shows owner/repo shorthand but not FQDN monorepo form at packages/apm-guide/.apm/skills/apm-usage/authentication.md
    Now that FQDN monorepo shorthand resolves correctly, a cross-reference here would help discoverability.
    Suggested: Append FQDN monorepo example to the GHES block with a note that GITHUB_HOST must be set for the virtual-path split to work.

Test Coverage Expert

  • [recommended] Install-pipeline integration floor missing: no fixture-backed test runs apm install for a GHES FQDN subpath dependency end-to-end at tests/integration/test_ghes_fqdn_subpath_install.py
    Evidence: missing, integration-with-fixtures -- proves: apm install ghe.example.com/org/repo/packages/skill installs packages/skill subpath, not the whole repo as a single path component.
    assert dep.repo_url == 'org/repo' and dep.virtual_path == 'packages/skill' and dep.is_virtual is True
    Probed tests/integration/ -- zero GHES FQDN subpath integration tests found. The user-facing promise lives in the full install pipeline, not only in DependencyReference.parse().
    Suggested: Add @pytest.mark.integration class TestGHESFQDNSubpathInstallPipeline that stubs the git-clone seam and verifies the end-to-end contract through APMPackage.from_apm_yml(). No PAT required.

  • [nit] No test covers GITHUB_HOST set with a URL scheme prefix ((ghe.example.com/redacted)), which silently fails GHES recognition at tests/unit/test_github_host.py
    Evidence: missing, unit -- GITHUB_HOST set with https:// prefix does not silently enable GHES for the wrong host value.
    monkeypatch.setenv('GITHUB_HOST', '(ghe.example.com/redacted)'); assert github_host.is_github_hostname('ghe.example.com') is False
    All 10 new unit tests verified present -- none cover scheme-prefixed GITHUB_HOST.
    Suggested: Add a test documenting current behavior; optionally add a guard detecting scheme prefix and raising ValueError with an actionable message.

This panel is advisory. It does not block merge. Re-apply the
panel-review label after addressing feedback to re-run.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • pypi.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "pypi.org"

See Network Configuration for more information.

Generated by PR Review Panel for issue #1684 · sonnet46 20.4M ·

@github-actions github-actions Bot removed the panel-review Trigger the apm-review-panel gh-aw workflow label Jun 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

apm install: FQDN monorepo subpath on GHES fails; hoping for CLI-only install without manual apm.yml edit

2 participants