Skip to content

Fix Real Findings from the Promotion PR's Fresh Review Pass #1047

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ptr727 merged 1 commit into from
Aug 28, 2026
Merged

Fix Real Findings from the Promotion PR's Fresh Review Pass #1047

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions host-setup/bootstrap.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,10 +381,24 @@ function Resolve-Mode {
return $given[0]
}

# The lines a piped-in run is told to paste instead, shared by both places that print them: the handoff below needs a real file to hand off to pwsh, and Test-Interactive needs a real console to ask on, and a run with neither reaches this the same way.
function Show-DownloadAndRunRemedy {
info ' [Net.ServicePointManager]::SecurityProtocol = [Net.ServicePointManager]::SecurityProtocol -bor [Net.SecurityProtocolType]::Tls12'
info " Invoke-WebRequest -UseBasicParsing -Uri https://raw.githubusercontent.com/$script:REPO/$script:DEFAULT_REF/host-setup/bootstrap.ps1 -OutFile bootstrap.ps1"
info ' powershell -ExecutionPolicy Bypass -File bootstrap.ps1'
}

function main {
if ($script:WANT_HELP) { usage; exit 0 }

if ($PSVersionTable.PSVersion.Major -lt 7) {
# $PSCommandPath is empty for a script read through Invoke-Expression or a similar pipe, and the handoff below needs a real path to re-invoke under pwsh, so this cannot wait for Test-Interactive to say the same thing for a different reason.
if (-not $PSCommandPath) {
warn 'This needs a real file to hand off to PowerShell 7, and a piped-in script has none.'
info 'Download the file and run it, rather than piping it:'
Show-DownloadAndRunRemedy
exit 0
}
Invoke-PwshHandoff
}

Expand All @@ -403,9 +417,7 @@ function main {
$script:MODE = 'report'
warn 'No action given and no console to ask on, so this is a report'
info 'Download the file and run it, rather than piping it, to reach the menu:'
info ' [Net.ServicePointManager]::SecurityProtocol = [Net.ServicePointManager]::SecurityProtocol -bor [Net.SecurityProtocolType]::Tls12'
info " Invoke-WebRequest -UseBasicParsing -Uri https://raw.githubusercontent.com/$script:REPO/$script:DEFAULT_REF/host-setup/bootstrap.ps1 -OutFile bootstrap.ps1"
info ' powershell -ExecutionPolicy Bypass -File bootstrap.ps1'
Show-DownloadAndRunRemedy
}
}

Expand Down
30 changes: 23 additions & 7 deletions host-setup/menu.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ $DOWNSTREAM_ROOT = ''
$DOWNSTREAM_NAME = ''
$PWSH_PATH = ''
$QUIT = $false
$BAD_CHOICE = $false

# --- Output ---

Expand Down Expand Up @@ -208,6 +209,8 @@ function Invoke-FetchHubLocked {
Remove-Item -Force (Get-MarkerPath) -ErrorAction SilentlyContinue
return $false
}
# Set the moment the clone lands, whatever -Ref still has to do: Invoke-Cleanup is gated on this, and a later ref-specific failure below must still remove the tree this step already created.
$script:HUB_FETCHED = $true
if ($script:REF -ne $script:DEFAULT_REF) {
& git -C $hubPath fetch --quiet origin $script:REF | Out-Host
if ($LASTEXITCODE -ne 0) {
Expand All @@ -221,7 +224,6 @@ function Invoke-FetchHubLocked {
}
}
$script:HUB_ROOT = $hubPath
$script:HUB_FETCHED = $true
info "Cloned to $script:HUB_ROOT"
return $true
}
Expand Down Expand Up @@ -462,7 +464,8 @@ function Invoke-Dispatch {
{ $_ -in @('q', 'Q') } { $script:QUIT = $true; return 0 }
default {
warn 'Not one of the choices'
return 2
$script:BAD_CHOICE = $true
return 0
}
}
}
Expand All @@ -472,10 +475,11 @@ function Invoke-InteractiveMenu {
Show-Menu
$choice = Read-Host 'Choose'
$script:QUIT = $false
$script:BAD_CHOICE = $false
$rc = Invoke-Dispatch $choice
if ($script:QUIT) { break }
# An unrecognized choice is rc 2, already warned by Invoke-Dispatch, so this loops straight back rather than reading a pointless confirmation.
if ($rc -eq 2) { continue }
# A dedicated flag rather than a reserved return code: every dispatched task's own exit code passes through unchanged, and a tool that happens to exit 2 for its own reason (scripts/carry.py's "not uniquely registered", for one) must not be misread as an unrecognized choice.
if ($script:BAD_CHOICE) { continue }
if ($rc -eq 0) {
step 'Done'
} else {
Expand Down Expand Up @@ -506,10 +510,24 @@ function Resolve-Directory {
return $trimmed
}

# The lines a piped-in run is told to paste instead, shared by both places that print them: the handoff below needs a real file to hand off to pwsh, and Test-Interactive needs a real console to ask on, and a run with neither reaches this the same way.
function Show-DownloadAndRunRemedy {
info ' [Net.ServicePointManager]::SecurityProtocol = [Net.ServicePointManager]::SecurityProtocol -bor [Net.SecurityProtocolType]::Tls12'
info " Invoke-WebRequest -UseBasicParsing -Uri https://raw.githubusercontent.com/$script:HUB_REPO/$script:DEFAULT_REF/host-setup/menu.ps1 -OutFile menu.ps1"
info ' powershell -ExecutionPolicy Bypass -File menu.ps1'
}

function main {
if ($script:WANT_HELP) { usage; exit 0 }

if ($PSVersionTable.PSVersion.Major -lt 7) {
# $PSCommandPath is empty for a script read through Invoke-Expression or a similar pipe, and the handoff below needs a real path to re-invoke under pwsh, so this cannot wait for Test-Interactive to say the same thing for a different reason.
if (-not $PSCommandPath) {
warn 'This needs a real file to hand off to PowerShell 7, and a piped-in script has none.'
info 'Download the file and run it, rather than piping it:'
Show-DownloadAndRunRemedy
exit 0
}
Invoke-PwshHandoff
}

Expand All @@ -520,9 +538,7 @@ function main {
if (-not (Test-Interactive)) {
warn 'No console to ask on, so there is no menu to show'
info 'Download the file and run it, rather than piping it, to reach the menu:'
info ' [Net.ServicePointManager]::SecurityProtocol = [Net.ServicePointManager]::SecurityProtocol -bor [Net.SecurityProtocolType]::Tls12'
info " Invoke-WebRequest -UseBasicParsing -Uri https://raw.githubusercontent.com/$script:HUB_REPO/$script:DEFAULT_REF/host-setup/menu.ps1 -OutFile menu.ps1"
info ' powershell -ExecutionPolicy Bypass -File menu.ps1'
Show-DownloadAndRunRemedy
exit 0
}

Expand Down
11 changes: 7 additions & 4 deletions host-setup/menu.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ fetch_hub_locked() {
rm -rf "$DIR/hub" "$(marker_path)"
return 1
}
# Set the moment the clone lands, whatever $REF still has to do: cleanup is gated on this, and a later ref-specific failure below must still remove the tree this step already created.
HUB_FETCHED=true
# A branch name is already checked out by the clone above.
# A tag, a pull request ref, or a commit needs an explicit fetch and checkout, since "git clone --branch" only takes a branch or a tag, not an arbitrary commit.
if [[ $REF != "$DEFAULT_REF" ]]; then
Expand All @@ -137,7 +139,6 @@ fetch_hub_locked() {
}
fi
HUB_ROOT="$DIR/hub"
HUB_FETCHED=true
info "Cloned to $HUB_ROOT"
}

Expand Down Expand Up @@ -340,6 +341,7 @@ print_menu() {

# A failing task (a real install error, a network hiccup) is reported and returns to the menu rather than ending the session, so dispatch's own exit status cannot double as "quit": QUIT is a separate flag the q/Q case sets, read by the loop after every dispatch regardless of whether the task it ran succeeded.
QUIT=false
BAD_CHOICE=false

dispatch() {
case "$1" in
Expand All @@ -366,7 +368,7 @@ dispatch() {
q | Q) QUIT=true ;;
*)
warn "Not one of the choices"
return 2
BAD_CHOICE=true
;;
esac
}
Expand All @@ -377,11 +379,12 @@ interactive_menu() {
print_menu
read -r -p "Choose: " choice
QUIT=false
BAD_CHOICE=false
rc=0
dispatch "$choice" || rc=$?
[[ $QUIT == true ]] && break
# An unrecognized choice is rc 2, already warned by dispatch, so this loops straight back rather than reading a pointless confirmation.
((rc == 2)) && continue
# A dedicated flag rather than a reserved return code: every dispatched task's own exit code passes through unchanged, and a tool that happens to exit 2 for its own reason (scripts/carry.py's "not uniquely registered", for one) must not be misread as an unrecognized choice.
[[ $BAD_CHOICE == true ]] && continue
if ((rc == 0)); then
step "Done"
else
Expand Down