plugin/scripts: Windows (Git Bash) support#9
Open
Nedlitex wants to merge 1 commit intoReflexioAI:mainfrom
Open
plugin/scripts: Windows (Git Bash) support#9Nedlitex wants to merge 1 commit intoReflexioAI:mainfrom
Nedlitex wants to merge 1 commit intoReflexioAI:mainfrom
Conversation
Five Git-Bash-on-Windows bugs prevented the plugin install + service auto-start from working. None of them affect Linux/macOS — every change is gated on `claude_smart_is_windows` (uname matches MINGW*/MSYS*/CYGWIN*). 1. uv install: the astral.sh bash installer downloads a zip and unzips it, but Git Bash's bundled unzip corrupts the Windows uv.exe (bad CRC on the inflated binary). On Windows, run the official PowerShell installer (install.ps1) instead — same destination (~/.local/bin/uv.exe), so the post-install PATH prepend works uniformly. 2. python3 App Execution Alias stub: on Windows, %LocalAppData%\Microsoft\WindowsApps\python3.exe is a Microsoft Store launcher stub. `command -v python3` returns truthy but invoking it prints "Python was not found" and exits non-zero. Add claude_smart_resolve_python which probes interpreters with `-V` to filter the stub out, preferring `python` (the real installed one) over `python3` on Windows. Used by smart-install.sh's settings.json patcher and by the spawn-detached fallback. 3. spawn_detached on Windows: Git Bash has no setsid, no process groups, and os.setsid() is POSIX-only — the existing python3-based fallback would never work even if a real Python were on PATH. Use plain nohup on Windows: ignoring SIGHUP is sufficient for the child to survive the parent console closing, and Windows doesn't have the zombie-on-parent-exit problem POSIX has. 4. kill_tree on Windows: kill -TERM -- "-pgid" (negative pid = process group) is a POSIX construct with no Windows equivalent. Use taskkill /T /F /PID, which walks the child-process tree via the parent-pid/job-object relationships. Subtlety: in Git Bash, $! for a backgrounded job returns the MSYS pid (an internal counter), not the native Windows pid taskkill needs. ps's default output exposes the WINPID column; awk extracts it. Service scripts now delegate kill_group to claude_smart_kill_tree so the logic stays in one place. 5. Service start scripts (backend-service.sh, dashboard-service.sh): the inline setsid/python3/nohup detach blocks would land in the broken python3 branch on Windows. Replace with the unified claude_smart_spawn_detached helper. PID-file recording is split: Windows records the spawned MSYS pid (translated to WINPID by kill_tree at signal time); POSIX still records the pgid where setsid guarantees pid==pgid, and falls back to ps -o pgid otherwise. Tested on Windows 11 / Git Bash (MINGW64): smart-install.sh now completes (uv install + uv sync + dashboard build), and claude_smart_spawn_detached + kill_tree round-trip a `ping -n 100` through start-alive-kill-gone end-to-end. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughIntroduces cross-platform process lifecycle management abstractions for shell scripts. Adds Windows/POSIX-aware spawning, killing, and Python resolution primitives in a shared library, then refactors service scripts to use these helpers instead of direct system calls. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugin/scripts/_lib.sh`:
- Around line 42-59: The POSIX branch of claude_smart_resolve_python currently
only checks discoverability with command -v but must also verify the interpreter
actually runs like the Windows branch; update the for-loop for cand in python3
python to, after command -v "$cand", attempt a lightweight runtime check (e.g.,
"$cand" -V or --version redirected to /dev/null) and only return that candidate
if the check succeeds so that claude_smart_spawn_detached() will receive a
working interpreter; keep the same candidate order and error handling (return 1
if none pass).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 18d623bb-c99a-403a-a30b-f6680f6fd22a
📒 Files selected for processing (4)
plugin/scripts/_lib.shplugin/scripts/backend-service.shplugin/scripts/dashboard-service.shplugin/scripts/smart-install.sh
Comment on lines
+42
to
+59
| claude_smart_resolve_python() { | ||
| if claude_smart_is_windows; then | ||
| for cand in python python3; do | ||
| if command -v "$cand" >/dev/null 2>&1 && "$cand" -V >/dev/null 2>&1; then | ||
| command -v "$cand" | ||
| return 0 | ||
| fi | ||
| done | ||
| return 1 | ||
| fi | ||
| for cand in python3 python; do | ||
| if command -v "$cand" >/dev/null 2>&1; then | ||
| command -v "$cand" | ||
| return 0 | ||
| fi | ||
| done | ||
| return 1 | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Resolver implementation:"
sed -n '35,59p' plugin/scripts/_lib.sh
echo
echo "Detach path that executes the resolved interpreter:"
sed -n '71,83p' plugin/scripts/_lib.sh
echo
echo "Other runtime call sites:"
rg -n 'claude_smart_resolve_python|claude_smart_spawn_detached' plugin/scriptsRepository: ReflexioAI/claude-smart
Length of output: 2565
Probe POSIX candidates before returning them.
The function's comment explicitly promises a working interpreter, but the POSIX branch only checks discoverability via command -v, whereas the Windows branch validates with -V. A broken shim (e.g., pyenv stub) can pass the POSIX check and then fail at runtime when executed directly in claude_smart_spawn_detached() at line 78. This is inconsistent with the documented contract and the Windows logic.
Suggested change
claude_smart_resolve_python() {
if claude_smart_is_windows; then
for cand in python python3; do
if command -v "$cand" >/dev/null 2>&1 && "$cand" -V >/dev/null 2>&1; then
command -v "$cand"
return 0
fi
done
return 1
fi
for cand in python3 python; do
- if command -v "$cand" >/dev/null 2>&1; then
+ if command -v "$cand" >/dev/null 2>&1 && "$cand" -V >/dev/null 2>&1; then
command -v "$cand"
return 0
fi
done
return 1
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugin/scripts/_lib.sh` around lines 42 - 59, The POSIX branch of
claude_smart_resolve_python currently only checks discoverability with command
-v but must also verify the interpreter actually runs like the Windows branch;
update the for-loop for cand in python3 python to, after command -v "$cand",
attempt a lightweight runtime check (e.g., "$cand" -V or --version redirected to
/dev/null) and only return that candidate if the check succeeds so that
claude_smart_spawn_detached() will receive a working interpreter; keep the same
candidate order and error handling (return 1 if none pass).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes five Windows-specific bugs in the plugin shell scripts so
claude-smartinstalls and auto-starts cleanly under Git Bash (MINGW64) on Windows 10/11. Every change is gated onclaude_smart_is_windows(uname matchesMINGW*/MSYS*/CYGWIN*), so Linux and macOS behaviour is byte-for-byte unchanged.Hit while installing the plugin on Windows 11 with
claude-smart0.1.17. The dashboard's/claude-smart:dashboardslash command failed withbackend: not runninganddashboard: .next missing, because the install never completed and the service-start scripts couldn't detach.What was broken
smart-install.shuv installuvinstall reports success thenuv syncfails becauseuv.exeis corruptinstall.sh) downloads a zip and unzips it; Git Bash's bundledunzipcorrupts the Windowsuv.exe(bad CRC on inflate).smart-install.shcs-cite heredocpython3is the Microsoft Store App Execution Alias stub atWindowsApps\python3.exe.command -v python3returns truthy but invoking it prints "Python was not found" and exits non-zero._lib.shclaude_smart_spawn_detachedpython3 -c '...os.setsid()...'— same App Execution stub fires; even with a real Python,os.setsid()is POSIX-only and wouldAttributeError.backend-service.sh/dashboard-service.shkill_groupstopdoesn't kill the servicekill -TERM -- "-pgid"(negative pid = process group) is POSIX-only. Windows has no process groups, so the signal goes nowhere.What changes
_lib.sh— three new helpers, one rewritten:claude_smart_is_windows—uname -stest forMINGW*|MSYS*|CYGWIN*.claude_smart_resolve_python— probes interpreters with-Vto filter the App Execution Alias stub. Preferspythonoverpython3on Windows; preservespython3-first ordering on POSIX.claude_smart_kill_tree— POSIX: signal the process group (TERM, grace, KILL). Windows:taskkill //T //F //PID. Walks Git Bash's MSYS-pid to Windows-pid translation viaps(column 4 in default output isWINPID);$!for a backgrounded job returns the MSYS pid, buttaskkillneeds the native Windows pid.claude_smart_spawn_detached— Windows path: plainnohup(no setsid, no process groups; nohup ignores SIGHUP, which is sufficient for the child to survive the parent console closing). POSIX path: setsid → resolved-pythonos.setsid()→ nohup, with the python branch now gated on-Vso the App Execution stub never fires.smart-install.sh— Windows branch in the uv-install block uses the official PowerShell installer (irm https://astral.sh/uv/install.ps1 | iex); same destination (~/.local/bin/uv.exe), so the post-installPATHprepend works uniformly. The cs-cite settings.json patcher usesclaude_smart_resolve_pythoninstead of barepython3. Also adds~/.local/bin/uv.exeto the post-install fallback search.backend-service.sh/dashboard-service.sh— drop the inline setsid/python3/nohup chains; callclaude_smart_spawn_detachedand record$!. PID-file logic split per-OS:claude_smart_kill_treetranslates to WINPID at signal time.ps -o pgidas before.kill_groupbecomes a one-line wrapper aroundclaude_smart_kill_treeso the cross-OS logic lives in one place.What's intentionally out of scope
reap_port_listeners(backend-service.sh) and the dashboardstopfallback both uselsof, which isn't on Git Bash by default. Existingcommand -v lsofguards already make these no-ops on Windows. Replacing them withnetstat -ano | findstr LISTENINGparsing would work but expands surface area. The recorded-pid path withtaskkill /T /Falready covers the normal stop case (killsnpmplus itsnodechildren).uv sync --python <ver>: while building this PR I hit a separate failure whereuvdefaulted to Python 3.14 andhdbscan(transitive viareflexio-ai) has no 3.14 wheel for Windows, so the source build fired and failed on a 3.14 incompatibility. That's a transitive-dep issue, not a Windows-bash issue, and would also bite Linux/macOS users who install Python 3.14. Worth fixing upstream (probably by pinning inpyproject.tomlor via a--pythonflag), but I left it out so this PR stays focused on the bash-vs-Windows angle..envfile encoding: reflexio's.envscaffolding wrote a CP1252-encoded em-dash (byte0x97) into a comment, whichpython-dotenvthen rejected on the backend at startup. Repository for that fix isReflexioAI/reflexio, not this one.Test plan
bash -nclean on all four edited scripts.claude_smart_is_windowsreturns 0 on Git Bash MINGW64 (Windows 11), 1 on Linux/macOS shells.claude_smart_resolve_pythonreturns the realpythoninterpreter on Windows when both real-Python and the WindowsApps stub are on PATH; returnspython3on POSIX.claude_smart_spawn_detached+claude_smart_kill_treeround-trip: spawnping -n 100 127.0.0.1, verifykill -0reports alive, verifytasklistshowsPING.EXEwith the matching WINPID, kill via helper, confirm process gone fromtasklistandkill -0reports dead.smart-install.shend-to-end on Windows 11 / Git Bash:uvinstalls via PowerShell (no unzip corruption),uv synccompletes (with Python 3.12 — see the "out of scope" note above), dashboardnpm install+npm run buildproduce a working.next/.reflexio services start --only backend --no-reload) and dashboard (npm run start) reach/healthafter manual launch via the helpers;/claude-smart:dashboardopenshttp://localhost:3001and renders.claude_smart_is_windowsso behaviour should be unchanged, but a maintainer with those environments should re-run the existing service start/stop tests to confirm.🤖 Generated with Claude Code
Summary by CodeRabbit