Skip to content

Fix GitHub Enterprise host handling#70

Open
Mariangeles-2025 wants to merge 2 commits intoAzure-Samples:mainfrom
Mariangeles-2025:main
Open

Fix GitHub Enterprise host handling#70
Mariangeles-2025 wants to merge 2 commits intoAzure-Samples:mainfrom
Mariangeles-2025:main

Conversation

@Mariangeles-2025
Copy link
Copy Markdown

No description provided.

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

Updates the setup/authentication flow in doctor.sh to better support GitHub Enterprise by making the GitHub host configurable.

Changes:

  • Introduces a GITHUB_HOST environment variable (defaulting to github.com).
  • Passes the configured host into copilot login via --host.
Show a summary per file
File Description
doctor.sh Adds GITHUB_HOST defaulting and uses it when invoking copilot login.

Copilot's findings

  • Files reviewed: 1/1 changed files
  • Comments generated: 2

Comment thread doctor.sh Outdated
MAGENTA='\033[0;35m'
BOLD='\033[1m'
NC='\033[0m' # No Color
#Variable for the GitHub host, defaults to 'github.com
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

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

The new comment has a typo/incomplete quoting (missing closing ' after github.com) and is missing a space after #, which makes it harder to read and can be confusing about the expected value format for GITHUB_HOST.

Suggested change
#Variable for the GitHub host, defaults to 'github.com
# Variable for the GitHub host, defaults to 'github.com'

Copilot uses AI. Check for mistakes.
Comment thread doctor.sh
echo -e "${BLUE}🔐 Authenticating with GitHub Copilot...${NC}"
echo ""
if ! copilot login; then
if ! copilot login --host "https://${GITHUB_HOST}"; then
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

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

copilot login --host "https://${GITHUB_HOST}" will produce an invalid URL if GITHUB_HOST is provided with a scheme already (e.g., https://ghe.example.comhttps://https://...). Consider normalizing GITHUB_HOST (accept either hostname or full URL) before building the --host argument, and reuse the same host for the PAT instructions URL in this setup flow so GitHub Enterprise users aren’t directed to github.com.

Copilot uses AI. Check for mistakes.
@jkordick
Copy link
Copy Markdown
Contributor

Review Summary for PR #70

Overall

  • Groups reviewed: 1/1
  • LGTM: 0
  • Guidance applied (code changes): 2
  • PR comments posted: 0
  • NACK: 0

Build & Test Result

  • ✅ Shell syntax check passed

Code Changes Applied

  1. doctor.sh (lines 16-19) — Fixed comment typo (space after #, closing quote). Added scheme-stripping (${GITHUB_HOST#https://}, ${GITHUB_HOST#http://}) to prevent double-scheme bug when GITHUB_HOST is set to a full URL like https://ghe.example.com. Removed extra blank line.
  2. doctor.sh (line 1175) — Updated PAT creation URL to use https://${GITHUB_HOST}/settings/tokens so GitHub Enterprise users are directed to their own instance instead of github.com.

Acceptance Criteria Coverage

  • ℹ️ No linked issue — no acceptance criteria to evaluate.

Observations

  • ⚠️ No PR description or linked issue — traceability gap. Consider linking an issue before merging.

- Fix comment typo (space after #, closing quote)
- Add scheme-stripping for GITHUB_HOST to prevent double-scheme bug
- Update PAT URL to use GITHUB_HOST for GHE support
- Remove extra blank line
@jkordick
Copy link
Copy Markdown
Contributor

@Mariangeles-2025 please test it again with the changes in your environment (as I cannot reproduce it)

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.

3 participants