Fix Real Findings from the Promotion PR's Fresh Review Pass by ptr727 · Pull Request #1047 · ptr727/ProjectTemplate

ptr727 · 2026-08-28T01:22:49Z

Summary

Fixes three real bugs the promotion PR's (#1046) fresh review pass over the full menu.ps1/menu.sh diff surfaced, in both files:

A failed non-default -Ref fetch or checkout left HUB_FETCHED unset, so final cleanup skipped the cloned hub and its ownership marker entirely, the same false-ownership hazard already fixed for the initial clone failure on PR Add menu.ps1, the Windows Counterpart to menu.sh #1045. HUB_FETCHED is now set the moment the initial clone lands, before -Ref's own fetch/checkout can fail.
An unrecognized menu choice returned exit code 2, colliding with a driven tool's own legitimate exit code 2 (scripts/carry.py's "repository is not uniquely registered", reproduced from earlier testing on PR Add menu.ps1, the Windows Counterpart to menu.sh #1045): Invoke-InteractiveMenu would silently swallow that tool's real failure as if the menu choice itself had been invalid. Replaced with a dedicated BAD_CHOICE flag.
Windows PowerShell 5.1 reading the script through Invoke-Expression (or a similar pipe) has an empty $PSCommandPath, and the PowerShell-7 handoff ran unconditionally before ever checking for a console, so a piped-in run could fail handing off to pwsh instead of reaching the "download and run it" message. Guarded in both menu.ps1 and bootstrap.ps1, which shares the same startup order.

Two other findings on PR #1046 were declined with evidence in the review thread rather than fixed here: the file header comment (same shape as menu.sh/bootstrap.ps1, already merged) and the mutex's release timing (same accepted trade-off as menu.sh's flock, already documented in the code).

Verification

menu.ps1/bootstrap.ps1 parse clean under both pwsh 7 and real Windows PowerShell 5.1, and lint clean against PSScriptAnalyzerSettings.psd1.
menu.sh passes bash -n.
Verified the $PSCommandPath fix against a real Invoke-Expression on Windows PowerShell 5.1: prints the download remedy instead of crashing the handoff. Confirmed no regression to the normal -File invocation.
Verified the HUB_FETCHED/cleanup fix and the BAD_CHOICE fix functionally in both menu.ps1 and menu.sh.
.husky/pre-commit (ruff, mypy, prose_lint, eol) passes clean.

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes
- Improved menu handling so invalid selections no longer conflict with legitimate task exit codes.
- Improved cleanup after setup failures during repository ref retrieval or checkout.
- Added clearer instructions when scripts are run directly from a pipeline or without an action.
- Improved compatibility handling for older PowerShell versions.

qodo-code-review and coderabbitai on PR #1046 (develop -> main), against the full diff menu.ps1 first introduced: - A failed non-default -Ref fetch or checkout left HUB_FETCHED unset, so final cleanup skipped the cloned hub and its ownership marker entirely, the same false-ownership hazard already fixed for the initial clone failure. HUB_FETCHED is now set the moment the initial clone lands, before -Ref's own fetch/checkout can fail. Fixed in both menu.ps1 and menu.sh. - An unrecognized menu choice returned exit code 2, which collided with a driven tool's own legitimate exit code 2 (scripts/carry.py's "repository is not uniquely registered", reproduced from this session's own earlier testing): Invoke-InteractiveMenu would silently swallow that tool's real failure as if the menu choice itself had been invalid. Replaced with a dedicated BAD_CHOICE flag, matching how QUIT is already kept apart from a return code. Fixed in both files. - Windows PowerShell 5.1 reading the script through Invoke-Expression (or a similar pipe) has an empty $PSCommandPath, and the PowerShell-7 handoff ran unconditionally before ever checking for a console, so a piped-in run could fail handing off to pwsh instead of reaching the "download and run it" message. Verified against a real Invoke-Expression on Windows PowerShell 5.1. Guarded in both menu.ps1 and bootstrap.ps1, which shares the same startup order; the repeated remedy text is now the shared Show-DownloadAndRunRemedy in each file. Declined on the same PR, with evidence in the thread: the header comment (same shape as menu.sh/bootstrap.ps1, already merged), and the mutex's release timing (same accepted trade-off as menu.sh's flock, already documented in the code).

qodo-code-review · 2026-08-28T01:23:41Z

PR Summary by Qodo

Fix Host Menu Cleanup, Exit-Code Handling, and PowerShell Handoff

🐞 Bug fix🕐 20-40 Minutes

AI Description

• Ensure failed non-default ref operations still clean up owned hub clones.
• Preserve task exit codes by tracking invalid menu choices separately.
• Guide piped Windows PowerShell runs instead of attempting an impossible pwsh handoff.

Diagram

graph TD
A["PowerShell startup"] --> B{"Script path exists?"} -->|yes| C["Pwsh handoff"]
B -->|no| D["Download remedy"]
E["Hub clone"] --> F["Owned cleanup"]
G{"Menu choice valid?"} -->|yes| H["Task status"]
G -->|no| G

High-Level Assessment

The targeted state changes are the best fit for the existing cross-shell design. A reserved dispatch exit code would continue colliding with tool results, and globally reordering interactivity checks could alter bootstrap's noninteractive action modes; dedicated choice state, immediate clone ownership state, and a narrow PSCommandPath guard preserve current contracts.

Files changed (3) +45 / -14

Bug fix (3) +45 / -14

bootstrap.ps1Guard piped Windows PowerShell startup before pwsh handoff+15/-3

Guard piped Windows PowerShell startup before pwsh handoff

• Detects an empty PSCommandPath before attempting the PowerShell 7 handoff and prints download-and-run guidance instead. Extracts that guidance into a helper reused by the noninteractive report path.

host-setup/bootstrap.ps1

menu.ps1Fix Windows menu cleanup, dispatch state, and piped startup+23/-7

Fix Windows menu cleanup, dispatch state, and piped startup

• Marks a cloned hub as fetched before ref-specific operations so cleanup still runs after fetch or checkout failures. Separates invalid menu input from task exit codes and guards fileless Windows PowerShell runs with shared download guidance.

host-setup/menu.ps1

menu.shPreserve Bash task statuses and clean failed ref clones+7/-4

Preserve Bash task statuses and clean failed ref clones

• Marks successful clones for cleanup before resolving non-default refs. Replaces exit code 2 as the invalid-choice sentinel with dedicated state so legitimate task failures remain visible.

host-setup/menu.sh

qodo-code-review · 2026-08-28T01:23:55Z

Code Review by Qodo

🐞 Bugs (0)📘 Rule violations (0)📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Tip of the day

💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

coderabbitai · 2026-08-28T01:23:55Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 4c42296e-338c-411b-b915-f1778e754a03

📥 Commits

Reviewing files that changed from the base of the PR and between f3f4673 and c48c441.

📒 Files selected for processing (3)

host-setup/bootstrap.ps1
host-setup/menu.ps1
host-setup/menu.sh

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.

📝 Walkthrough

Walkthrough

The host setup scripts now preserve cloned-hub cleanup state, separate invalid menu choices from task exit codes, and share download-and-run instructions for unsupported piped-in PowerShell execution.

Changes

Host setup reliability

Layer / File(s)	Summary
Hub cleanup state `host-setup/menu.ps1`, `host-setup/menu.sh`	Both menu implementations mark the hub as fetched immediately after cloning, before ref fetch or checkout operations.
Menu exit-state handling `host-setup/menu.ps1`, `host-setup/menu.sh`	Both menu implementations use `BAD_CHOICE` for invalid selections while preserving dispatched task exit codes, including code `2`.
PowerShell download remedy `host-setup/bootstrap.ps1`, `host-setup/menu.ps1`	PowerShell scripts centralize download-and-run instructions and stop piped-in execution when no real script path exists.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk:⚪ Minimal · up to c48c4

The PR makes targeted fixes to cleanup, exit-code handling, and PowerShell startup behavior; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. (2 skipped: 2 …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately identifies the pull request as fixing findings from a review of the promotion PR. It is concise and related to the three bug fixes in the changeset, although it does not name each…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title accurately identifies the pull request as fixing findings from a review of the promotion PR. It is concise and related to the three bug fixes in the changeset, although it does not name each specific fix.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. (2 skipped: 2 unsupported.)

Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch menu-review-fixes

_{Comment @coderabbitai help to get the list of available commands.}

ptr727 merged commit 1863f3c into developAug 28, 2026
8 checks passed

ptr727 deleted the menu-review-fixes branch August 28, 2026 01:33

ptr727 mentioned this pull request Aug 28, 2026
New fleet skill proposal: local-strict-review — adversarial pre-PR self-review on a stronger model #1056
Closed

Navigation Menu

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

Fix Real Findings from the Promotion PR's Fresh Review Pass#1047
ptr727 merged 1 commit into
developfrom
menu-review-fixes

ptr727 commented Aug 28, 2026•
edited by coderabbitai Bot
Loading

Uh oh!

qodo-code-reviewBot commented Aug 28, 2026

Uh oh!

qodo-code-reviewBot commented Aug 28, 2026

Uh oh!

coderabbitaiBot commented Aug 28, 2026•
edited
Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

ptr727 commented Aug 28, 2026• edited by coderabbitai BotLoading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Verification

Summary by CodeRabbit

Uh oh!

qodo-code-reviewBot commented Aug 28, 2026

PR Summary by Qodo

Uh oh!

qodo-code-reviewBot commented Aug 28, 2026

Code Review by Qodo

Great, no issues found!

Uh oh!

coderabbitaiBot commented Aug 28, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

ptr727 commented Aug 28, 2026•
edited by coderabbitai Bot
Loading

coderabbitaiBot commented Aug 28, 2026•
edited
Loading