Honor GH host resolution in gh aw trial clone paths (host + --clone-repo)#41822
Honor GH host resolution in gh aw trial clone paths (host + --clone-repo)#41822Copilot wants to merge 1 commit into
gh aw trial clone paths (host + --clone-repo)#41822Conversation
gh aw trial clone paths (host + --clone-repo)
🤖 PR Triage
Score breakdown: Impact 25 · Urgency 10 · Quality 5 Fixes
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Hey That said, the PR currently carries zero code changes — the only commit is an empty "Initial plan" placeholder. Two things needed to get this across the finish line:
If you'd like a hand, you can assign this prompt to your coding agent:
|
|
✅ Test Quality Sentinel completed test quality analysis. No test files were added or modified in this PR (#41822). PR currently contains only an 'Initial plan' commit with no code changes (0 additions, 0 deletions, 0 files). Test Quality Sentinel skipped. |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #41822 does not have the 'implementation' label and has 0 new lines of code in business logic directories (threshold: 100). |
|
✅ PR Code Quality Reviewer completed the code quality review. No code to review: PR #41822 has 0 changed files and 0 additions/deletions. The only commit is an empty 'Initial plan' placeholder. The described implementation (host-aware trial URL helpers and clone path updates) has not been committed yet. |
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnose and /tdd — submitting as COMMENT because the PR currently contains zero changed files (only an Initial plan commit).
📋 Key Themes & Highlights
Situation
This PR was opened as a planning stub by a Copilot agent. The described fix — introducing trialRepositoryURL, trialRepositoryGitURL, and trialRepositoryActionsSettingsURL helpers in pkg/cli/trial_repository.go — already exists in the base branch (landed in commit f414fc7). Likewise, TestTrialRepositoryURLHelpers in trial_repository_test.go already covers the three new helpers.
The PR needs either:
- Closure — if the implementation is already complete on
main, this PR is redundant. - Implementation commit — if there are remaining hardcoded
github.lovev.sitesites not yet covered by the helpers, those changes still need to be committed here.
/diagnose observations
- ✅ Root cause correctly identified: hardcoded
https://github.lovev.siteliterals in clone/push/display paths. - ✅ Fix approach is sound: centralized helpers delegating to
getGitHubHost()rather than inline duplication. - ✅ An ADR (
docs/adr/41159-honor-gh-host-in-trial-repository-urls.md) documents the decision with alternatives considered — excellent practice. ⚠️ No regression-test commit is present in this PR to prove the bug is gone (the tests in the base may cover it, but a failing-then-passing cycle in this branch is missing).
/tdd observations
- ✅
TestTrialRepositoryURLHelperscovers the three URL helpers with three table-driven cases. ⚠️ Coverage gap:GITHUB_ENTERPRISE_HOSTandGITHUB_HOSTare set int.Setenvbut no table-driven case exercises them as the primary host. Their precedence vsGH_HOSTis untested.⚠️ No end-to-end test verifies that clone operations in both direct mode and--clone-repomode actually use the host-aware URL rather than a hardcoded literal — consider adding a test that stubsgetGitHubHost()and asserts the git-clone command string.
Positive Highlights
- ✅
trialRepositoryGitURLcomposing ontrialRepositoryURLavoids the trailing-slash edge case that often bites URL-building code. - ✅ Trailing-slash stripping (when
GITHUB_SERVER_URLends with/) is handled transparently by the existinggetGitHubHost()implementation. - ✅
trialRepositoryActionsSettingsURLensures user-facing settings links are host-aware, not just git clone paths.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 62.1 AIC · ⌖ 7.71 AIC · ⊞ 6.6K
gh aw trialwas resolving repository existence via the configured GHES host, but clone operations still targetedgithub.lovev.site, making trial mode unusable outside public GitHub. This change aligns all trial-mode repository URLs with the active host resolution path (GH_HOST/ enterprise envs).Host-aware trial URL construction
github.lovev.site.Clone path fixes in both trial modes
--clone-repomode to use host-aware git URLs.Consistency across user-facing output